akiomik / akiomik/mado

The development scripts report results without having run anything

オープン
#400 コメント 6 件 リアクション 0 件 担当者 0 名 GitHub で見る
ci
主要言語
Rust
スター
407
フォーク
12
平均マージ
5時間 45分
マージ済み PR(30日)
30

説明

Split out of #396, which was narrowed to the bugs that stop the scripts from
doing what they say. This collects the other half: every way
`scripts/acceptance` and `scripts/benchmarks` produce an answer when the thing
being measured never ran. None of them fails loudly; each looks like a result.

## The cases

| where | what is missing | what you get |
| --- | --- | --- |
| `acceptance/test.sh` | `$PROJECT_ROOT/tmp`, which is gitignored and which nothing creates | both redirects fail, the second only after paying for a `cargo run` |
| `acceptance/test.sh` | `mdl` (a Ruby gem, vendored nowhere) | empty `mdl.txt`; two empty files compare equal, so mado and mdl "agree" |
| `acceptance/test.sh` | `$DOC_PATH`, until `setup.sh` has run | both files empty, same false agreement |
| `acceptance/test.sh` | a build that fails, or no `cargo` | `cargo run` writes only to stderr, `mado.txt` empty, exit 0 |
| `benchmarks/comparison.sh` | `node_modules`, which is gitignored and which nothing installs | `hyperfine --ignore-failure` publishes `markdownlint` and `markdownlint-cli2` as roughly a thousand times faster than mado |
| `benchmarks/comparison.sh` | mado's binary, when `CARGO_TARGET_DIR` or `build.target-dir` sends it elsewhere | `cargo build` succeeds, hyperfine times a command that cannot start, mado "wins" |
| both | the repository, when run from another directory | `cargo` cannot find the manifest |

The `node_modules` one is the common case, not a corner: `scripts/benchmarks/`
tracks `package.json` and `package-lock.json`, `node_modules` is gitignored,
nothing runs `npm ci`, and README.md documents the benchmark as `setup.sh` then
`comparison.sh`. A fresh clone following the README publishes wrong numbers.

## What a fix has to decide

Not just "add a check". `hyperfine --ignore-failure` is there deliberately,
because the linters legitimately exit non-zero when they find violations —
`mado check` on a file with violations returns 1 — so the scripts cannot simply
gate on exit status. The distinction that has to be drawn is *present and
built* versus *what it reported*, and it has to be drawn for every external
tool the scripts invoke.

Whether `setup.sh` should also run `npm ci`, or the README should gain the
step, is part of the same decision.

## Why the linter would not have caught it

None of these is visible to `shellcheck`. They are facts about
`hyperfine --ignore-failure`'s semantics, about cargo's output path, about
which files `git` restores, and about what an empty file means when two are
compared — not about shell syntax. #398 is worth doing on its own terms, but it
would not have found a single row of the table above.

That is also the argument in #397, applied to these scripts rather than to
`action/entrypoint.sh`: the failure mode here is not that the shell is written
badly, it is that this much conditional external-tool orchestration has no
tests and no type system, and every review round finds another unguarded
dependency. Related: #399, where the same class turned destructive.

コントリビューションガイド

コントリビューションガイドを開く

評価

この issue はまだ評価されていません。

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。