diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e4d51add..d4a03153 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -52,7 +52,7 @@ jobs: # assembly still exited 0 (improvise-w8ih). It guards the assemble step # below, so it fails under this job's name rather than another's. - name: The stamp gate refuses what it claims to - run: ./scripts/checkers.sh assemble-stamp + run: ./scripts/checkers.sh --ci build # Built by Nix (packages.web and packages.site in nix/ci.nix), which runs # the same harness/build.sh and web/assemble.sh a developer runs, so there # is still one answer to "how is the bundle built". `web` is the three @@ -184,7 +184,7 @@ jobs: # The guard above is only worth its line if its refusals are themselves # checked -- the step it replaces was green while asserting nothing. - name: The node test runner refuses what it claims to - run: ./scripts/checkers.sh node-test + run: ./scripts/checkers.sh --ci js kb: name: Knowledge base consistency @@ -199,27 +199,12 @@ jobs: run: | tpl prolog-sessions/kb.pl \ -g "(kb_consistent -> halt(0) ; (kb_report, halt(1)))" - # scripts/kb is the only thing that writes kb.pl, so its refusals are as - # load-bearing as kb_consistent itself. Without this the replacement - # writer could regress green -- the review that found the directive - # bypass also found that nothing ran this suite. - - name: The kb tool refuses what it claims to - run: ./scripts/checkers.sh kb # Every suite goes through scripts/checkers.sh, which scrubs the - # environment, so the variables a suite passes under here are the ones it - # passes under locally (improvise-f9qf). Its bash is the dev shell's. - - name: The checker runner refuses what it claims to - run: ./scripts/checkers.sh --self-test - # pr-watch is driven entirely from fixtures. The release preflight is the - # one guard whose absence is invisible until a tag is public and - # immutable. pr-reviews stands between a merge and an unread review, and - # was blind to inline comments for as long as it existed (improvise-xgra). - # fmt-workspaces is here because its own failure mode is the bug it - # exists for: a usage error wearing a formatting failure's exit code. - # bd-audit-ids is the dead-ID audit, whose recipe once read a bd that - # never ran as a clean audit (improvise-52eq). - - name: The remaining checkers refuse what they claim to - run: ./scripts/checkers.sh pr-watch release-preflight pr-reviews fmt-workspaces bd-audit-ids + # environment (improvise-f9qf); its bash is the dev shell's. Which suites + # this job runs is the `kb` row of the CI table in checkers.sh, deliberately + # not listed here. check-ci-graph.py holds that table to `checkers.sh --list`. + - name: Every checker in scripts/ refuses what it claims to + run: ./scripts/checkers.sh --ci kb # --------------------------------------------------------------------------- # Lints the one crate the `test` job cannot reach. diff --git a/context/code/tooling.md b/context/code/tooling.md index 0edc9366..4e508534 100644 --- a/context/code/tooling.md +++ b/context/code/tooling.md @@ -259,6 +259,245 @@ the deploy's and pass. `README.md` therefore states the rule and deliberately **does not list the jobs**: a second copy with nothing holding it to the first is what went stale, twice. +It holds a second rule of the same kind: every suite `scripts/checkers.sh +--list` names must run in some CI step (improvise-dak5). `ci.yml` names its +suites by hand, because they need different jobs and environments (`kb` runs +under a nix trealla, `node-test` in the `js` job), so a single bare +`checkers.sh` was not an option. The check does not re-derive what a suite is. +It asks `checkers.sh --list`, so the naming rule keeps its one home. + +**What CI runs is a table in `checkers.sh`, not something read out of shell** +(improvise-st8q, improvise-tvte). Two rounds of review each found the gate +passing while a suite never ran, and every finding was the gate +*interpreting* `ci.yml`. It read `--list` as "run everything". It counted the +names after `--self-test`. Then it read a bare `scripts/checkers.sh` inside a +heredoc, a folded `>` value or a multi-line string as a command, and it ignored +`if:`. Each fix added parsing, and shell and YAML always had another form. So +the parsing went. `checkers.sh` holds a job→suites table, `ci_suites`, and each +CI job has exactly one step whose whole `run:` value is +`./scripts/checkers.sh --ci `. A `nix shell --command` prefix is +allowed. `--ci` runs that row and nothing else, and `--ci` beside a suite name +or `--self-test` is a usage error. `checkers.sh --would-run --ci ` reports +the row without running it, so the table has one home. + +The gate reads no shell. It counts a step only when all of these hold: +- the step sits in a job's `steps:` sequence, at step-key indentation; +- its `run:` value is exactly that call, on one line. A plain scalar folds a + deeper-indented next line into the value, so `|| true` there would become part + of the command. A `run:` that continues onto another line is not counted + (improvise-1be2); +- the call names its own job; +- neither the step nor the job carries `if:`, `continue-on-error:` or `shell:`; +- no job it `needs:`, directly or through other jobs, carries a job-level + `if:`. GitHub skips a job whose need was skipped, so a false `if:` two jobs + away skips the call as surely as one on its own job (improvise-bbwa). + +The only YAML it models is what makes a line structure rather than content. A +key with an inline value owns every following line indented deeper. So a +heredoc, a folded or plain continuation, or a whole fake job inside a +top-level block scalar is skipped as content. Every other line's value must be +one of a few shapes that end on their own line, or the line is refused +(improvise-0wn3, below). Any line outside a YAML comment that mentions +`scripts/checkers.sh` and is not a counted step is **reported**, not ignored. +The union of the counted jobs' rows must cover `checkers.sh --list`. + +**A key the gate cannot read plainly is refused, not parsed** (improvise-w0j4). +YAML has many spellings of one key or one list. `"if":`, `'continue-on-error':`, +`if :`, and `? if` then `: false` all mean the same key. `needs: ["gate"]`, +`needs: 'gate'`, `- "gate"`, `needs: &d [gate]`, and a flow list split over +two lines all name `gate`. The gate read none of them. Each one skipped `kb` +on a PR while the gate reported every suite run. + +Teaching it each spelling is the treadmill st8q stepped off. So every +structural line inside `jobs:` must take one plain form, or the gate names the +line and fails: +- a top-level key; +- a job name; +- a job key; +- a step key; +- a `needs:` value that is empty (a block list follows), one bare name, or a + one-line flow list of bare names; +- a block-list item that is a bare name. + +Content under a job key other than `steps:` and `needs:`, such as `env:` or +`strategy:`, is not structure the gate reads, so it is not checked. + +**But a line is content only once its job has a 4-space key** (improvise-rjaa). +The first version accepted any line deeper than 4 outside `steps:` and +`needs:`. So a *needed* job whose own keys sat at indent 5 or 6, which is valid +YAML, had its `if:` and `needs:` read as content. The gate exited 0 with `kb` +skipped. Now a job's first key must be at indent 4. After that, a deeper line +can only be the content of an empty-valued key, because a key with an inline +value already owns every deeper line. + +Three valid styles are refused loudly rather than read: +- an indentless block list (`needs:` then `- gate` at indent 4); +- a block list at indent 8; +- a quoted top-level key such as `"on":`. + +Each exits 1 and names its line. Spell it the plain way. + +**A ` #` on any line the gate reads is refused, and every matcher reads the +same string** (improvise-j49r). The gate used to strip a trailing comment for +the readability check and match the raw line everywhere else. So +` gate: # skipped on purpose` was accepted as a plain job header by one and +never opened as a job by the other. Its `if: false` was credited to the job +before it, and `read_graph` did not list `gate` at all. Both rules passed with +`kb` skipped and the deploy ungated by `gate`. An audit of every match found +the stripper wrong in a second way: it cut `A: "x # y"` to `A: "x`, the quote +tracker then held the next lines open, and a job-level `if:` after it vanished. +That also exited 0 on the real `ci.yml`. + +There were two ways to make the matchers agree: +- **strip everywhere.** A correct stripper has to know where a quoted value + starts and ends, because `"x # y"` has no comment and `"x" # y` has one. That + is parsing again, the thing st8q and w0j4 gave up. +- **Refuse the comment.** This was chosen. Once every line containing ` #` is + refused, the stripped and raw forms of every accepted line are the same + string, so no two matchers can read different versions of it. So the + stripper is deleted, not kept consistent. + +This refuses some valid styles, on any line that is not block-scalar content: +- a trailing comment on a step, such as `- uses: actions/checkout@v7 # pin`; +- a one-line `run: echo "a # b"`; +- a quoted `with:` value containing ` #`. + +This costs future readability, not correctness. The real `ci.yml` has none of +them. A full-line comment is still skipped, and a `#` with no space before it, +as in `nixpkgs#trealla`, is not a comment. After a refused line only the exit +is guaranteed: what the gate reads next may be garbled, and the exit is 1 +either way. + +**A quoted value that does not close on its own line is refused, and the quote +tracker is deleted** (improvise-6zf1). The tracker held a multi-line quoted +value open until the next line containing its quote character. YAML does not +close on an escaped `\"` inside double quotes, or on a doubled `''` inside +single quotes. So the tracker closed early, a later content line such as +`b: "` opened a phantom string, and that string swallowed the job's real +`if:`. Both quote styles exited 0 on the real `ci.yml`. This was the fourth +review round whose fail-open lived in quote or comment handling. Closing the +tracker correctly would have meant modelling YAML's quote rules one more time, +so the gate stops reading multi-line quotes at all. The real `ci.yml` has +none. + +The opening line is still judged by `_opens_quote`, and that check is now +load-bearing: a value it wrongly calls closed is not refused, and its +continuation lines are read as structure. A `name: "a\"` followed by +`needs: ok` would then overwrite a job's real `needs: gate`. So `_opens_quote` +drops `\\.` before looking for the closing `"`, and `''` before looking for +the closing `'`, and a case covers each. + +**Every value the gate reads must be one of a few shapes, each ending on its +own line** (improvise-0wn3). The 6zf1 fix refused a multi-line *quote* and +left a multi-line *flow* value (`[`, `{`) alone. The argument for leaving it +was that a flow continuation line is read as a key, so it fails closed. That +argument was wrong. It covered the closing line it imagined, not one written +to look like a step key: + +``` + - name: [a, "b + run: ./scripts/checkers.sh --ci kb + c: x"] +``` + +go-yaml, PyYAML and Psych all read this step as one `name` string with no +`run:`. The gate read the middle line as the step's `run:` and counted it. So +did the single-quoted form, a flow map, a flow opened on a line of its own, and +an anchored or tagged flow (`&a [x, "b`, `!!seq [x, "b`). All exited 0. + +This was the fifth review round in a row, and each fail-open was a construct +the gate did not *recognise* but also did not *refuse*. So the check is now an +allowlist over values, not a list of refused constructs. `_line_value` takes the +value a line carries: a key's, a list item's, or a whole line of its own. That +value must be one of: +- empty (a block follows); +- a block-scalar header (`|`, `>`, with chomping and indentation indicators), + whose body the owner rule skips; +- a quoted scalar that closes on the same line (`_opens_quote`); +- a one-line flow with no quotes, no nesting and no `#` inside it, such as + `needs: [test, wasm]`; +- a plain scalar that does not start with a YAML indicator (`[ ] { } & * ! | > + % @ \` ? ,`). + +Anything else is refused, whatever it is. The real `ci.yml` needs nothing +more: 153 plain values, 41 empty, 2 one-line flows and 5 block headers, with 0 +refused. + +The obvious narrower fix, "a flow value must end in its closer", is not enough. +`[a, "b]` ends in `]` while its quote is still open, and a case pins that. + +This is an allowlist over *values*, and it applies on every line the gate +reads, at any depth. The *keys* at indent 8 or less inside `jobs:` were already +allowlisted by `_readable` (w0j4, rjaa). A deeper key is not shape-checked, +because its value is. So a quoted key deep in a job, such as `"FOO": bar` under +`env:`, is now refused, because the gate cannot tell it from an unclosed quote. +That is a valid style refused loudly, beside the others in this section. + +**A line holding a line break YAML knows and the gate does not, or a control +character, is refused** (improvise-xpzh). The gate splits the text on `\n` +only. YAML also breaks lines at U+0085 (NEL), U+2028 and U+2029, so +`runs-on: x if: false` was one line to the gate and two to go-yaml. +The job's `if:` was invisible, and the gate exited 0. The same break inside a +full-line comment, or inside a `run: |` body, ends that line for YAML too. A +body line `echo a if: false` makes the `if:` a real job key. + +So the check runs on **every** line, before the comment and owner skips, and +refuses: +- C0 controls other than tab (CR included); +- DEL; +- the whole C1 block, which contains U+0085; +- U+2028 and U+2029. + +CR is the one break the read folds. `read_workflow` opens with Python's +universal newlines, which turn CR and CRLF into `\n`, and a lone CR is a line +break to YAML too, so the two agree. A CR that survives to the split can only +come from a read that did not fold it, and it is refused. Two self-test +controls write real files, one CRLF and one lone CR, and read them back +through `read_workflow`. The code path is the point: a string handed straight +to `suite_verdict` never meets the fold. + +**Why not refuse all non-ASCII,** which would be the simpler allowlist. The +real `ci.yml` has 27 non-ASCII characters (em dash, `§`, arrows) in its +comments, so it would have to be rewritten to pass. The set of characters that +changes line structure is not open-ended, though. It is YAML's own `b-char`: +LF and CR, plus NEL, LS and PS in YAML 1.1. So refusing that set, and the +controls around it, is complete by the spec's definition, not a list that +grows with each review. + +`read_graph` refuses nothing, so it cannot rely on refusal. It opens a job on +any ` name:` line followed by whitespace or the end of the line, which is +wider than the exact header `ci_steps` accepts. Reading too wide only adds jobs +`pages-deploy` must need, which fails closed. Reading too narrow is what lost +`gate`. + +**A `defaults: run: shell:` does not disqualify a step** (the user's ruling, +PR #228). An earlier version refused to count any step under a workflow- or +job-level `defaults:`, because a default shell can change what the call runs +under. The user called that "a silly check". It also broke on the day +`ci.yml` gained a real one: PR #287 set a workflow-level +`shell: nix develop .#ci --command bash …`, and every suite then read as run by +no CI step. A default shell is how the workflow chooses its environment, not a +way to skip a call. A shell that failed to run the call would fail the step, red +in CI. A step's own `shell:` still disqualifies it. That rule was not part of +the ruling. + +**What this does not close, stated rather than guarded.** Once a counted step +is reached, the gate trusts that it runs the script it names. An earlier step +in the same job that rewrote `scripts/checkers.sh` or its table would defeat +that. So would an earlier `actions/checkout` step with a `ref:`, which +replaces the tree, so the counted step runs another revision's `checkers.sh` +and its table. Shell control flow *around* the call cannot hide in the counted +step, because its value is exactly the call. A needed job that *fails* also +skips the job, and the gate does not model that. The failure is itself red in +CI, and it is why `pages-deploy` must still `needs:` every job. + +**Two claims earlier versions of this section made were wrong.** "`--would-run` +anywhere but first is a usage error" was false: `checkers.sh --list +--would-run` exits 0 at `--list` before it reaches the second flag. And "a line +`shlex` cannot parse fails closed" was true of that one line, but read as a +property of the gate, which at that point still passed on heredocs and folded +values. + ### pr-watch and mergeability (improvise-edpl, improvise-zlj2) `scripts/pr-watch` is what the orchestrator's merge procedure stands on, and diff --git a/context/test-evidence/tooling.md b/context/test-evidence/tooling.md index c7b8d46c..bdfdf7e8 100644 --- a/context/test-evidence/tooling.md +++ b/context/test-evidence/tooling.md @@ -977,6 +977,533 @@ So the pass needs both the exclusions and git's file list. After each rival the original was restored (`cmp` against a saved copy) and the suite re-run at 8 passed. +### Every checkers.sh suite runs in CI — `scripts/check-ci-graph.py` (improvise-dak5) + +**What was unguarded.** `ci.yml` names its `checkers.sh` suites by hand. A new +`scripts/X-test.sh` beside `scripts/X` is picked up by `checkers.sh` locally +and never runs in CI unless someone adds it to a step. Found on #226, where +`bd-audit-ids` had to be added by hand. + +**Planted in the real tree.** `scripts/zz-planted.sh` plus +`scripts/zz-planted-test.sh` appear in `checkers.sh --list`, and +`check-ci-graph.py` then exits 1 with *`zz-planted` is a checkers.sh suite that +no CI step runs; add it to ci.yml*. As a control, appending `zz-planted` to the +named step clears it (exit 0, "all 9 checkers.sh suites run in CI"). Both files +and the `ci.yml` edit were then removed, and the real run is back to 8 suites, +exit 0. + +**Seven self-test suite lists.** They cover: a suite no step runs; suites named +across several steps (one inside `nix shell … --command`); a bare run covering +everything; `--self-test` covering `checkers` and nothing else; a bare mention +in a YAML comment, which must *not* count as a bare run; names continued onto +the next line with a backslash; and a step naming a suite that does not exist. + +**Rivals, each run against `--self-test`:** +- **YAML comments no longer skipped:** exactly the comment case fails, because + the bare mention there reads as "everything runs" and the missing suite + passes. +- **`suites_run` always answering "everything"** (the vacuous shape): five + cases fail, every one that expects a suite to be named. + +The script was restored after each (`cmp` against a saved copy), and the +self-test is back to "5 graphs and 7 suite lists judged". + +### The suite check asks checkers.sh what a step runs — `scripts/check-ci-graph.py` (improvise-maax) + +**What broke.** The adversarial review of #228 at `cee61b94`, and ChatGPT +Codex's P1 on it, found the first version failing open in four shapes. Each let +the check pass while a suite never ran: +- a bare `scripts/checkers.sh` outside a command read as "every suite runs": + after an inline `#` (Codex), in a step `name:`, or as `echo`'s argument; +- `checkers.sh --list` alone read as every suite, though `--list` runs nothing; +- `--self-test zz-new kb` counted `zz-new` and `kb`, though `--self-test` + discards them; +- `--list kb zz-new` counted both. + +Two more were loud but wrong: only the first invocation on a line counted, and +quoted names kept their quotes. + +**The fix moves the argument rules back into `checkers.sh`.** It gains +`--would-run`, which is honoured only as the first argument and reports rather +than runs. `check-ci-graph.py` finds invocations: `run:` text only, comments +stripped by bash's rule, command position only. It then asks `--would-run` +about each one. + +**Every new case run against the OLD parser** (`/tmp/ccg-old.py`, the file at +`cee61b94`): eight of the sixteen suite cases give the wrong answer there. The +five false passes are `--list` alone, the inline comment (Codex), the non-run +block scalar, the step `name:` and `echo`'s argument. Wrong in a noisier way: +`--self-test zz-new kb` (the old parser counts `kb`, so it reports only a +bogus "zz-new is not a suite"), `--list kb zz-new` (the same), and `a && b` +(the second suite is missed). + +**A vacuity fixed before the rivals ran.** As first written, the bare-mention +cases required `zz-new`, which no real `checkers.sh` ever runs. A false bare +run then still left exactly `zz-new` unrun, the expected answer, so the +any-word-position rival passed all sixteen. The cases now require `kb` and +`pr-watch`, and the real step runs only `kb`. A false bare run is then the only +way `pr-watch` counts as run. + +**Rivals, one function swapped in at a time and each run against the real +`--self-test`:** +- **no comment stripping:** fails only "a suite named inside a trailing shell + comment is not run"; +- **every line scanned** (run: prefixes still recognised): fails "a + command-shaped line in a non-run block scalar" and the block-run case; +- **`checkers.sh` accepted in any word position:** fails only "a bare mention + as echo's argument"; +- **argv modelled naively** (names, else everything): fails the `--self-test` + case and both `--list` cases; +- **first invocation per line only:** fails "every invocation on a line + counts" and the block-run case; +- **`--would-run` honoured in any position**, in `checkers.sh` itself: fails + `checkers-test.sh`'s "--would-run anywhere but first is a usage error" (exit + 0, wanted 4) and the parser's "a step spelling --would-run runs nothing". + +One more rival crashed instead: an early every-line rival hit `shlex`'s +"No escaped character" on a lone trailing backslash. So a line that cannot be +parsed now contributes no invocation, which fails closed. Everything was +restored (`cmp` for `checkers.sh`) and re-run: `--self-test` reports 16 suite +lists and `checkers-test.sh` 17/17. A planted `zz-planted` suite still fails +the real run, naming it. + +### The suite gate reads a table, not shell — `scripts/check-ci-graph.py` (improvise-st8q, improvise-tvte) + +**What broke, twice over.** The #228 re-review at `c1417359` showed the maax +parser still failing open. On the real `ci.yml`, two changes each turned +exit 1 into exit 0: +- a `GITHUB_STEP_SUMMARY` heredoc holding a bare `./scripts/checkers.sh`; +- folding the kb step as `run: >`. + +That is because `run:` values were read one line at a time. `"--command" in +words` also matched any command at all. And step and job `if:` were ignored +(tvte). Each round of parsing had been answered by another shell or YAML form. + +**The structural fix.** `checkers.sh` owns a job→suites table, and each CI job +makes one call, `checkers.sh --ci `. The gate parses no shell. It counts a +step only when all of these hold: +- the step sits at step-key indentation inside `steps:`; +- its whole `run:` value is that call; +- the call names its own job; +- the step carries no `if:`, `continue-on-error:`, `shell:` or covering + `defaults:`. + +Every other line mentioning `scripts/checkers.sh` is reported. A key with an +inline value owns deeper lines, and an open quote owns lines until it closes, +so content is never read as structure. + +**The reviewer's attacks replayed on the real `ci.yml`** (kb step mutated, +restored after each): +- the heredoc: exit 1, naming the line and the seven unrun suites; +- `run: >`: exit 1, the same; +- `if:` on the kb step: exit 1, "conditional (if)". + +**Self-test: 19 suite lists.** Every case asks the real `checkers.sh` table. + +**Rivals, each a source edit loaded and run against the real `--self-test`, +each failing a case only it can:** +- **No quote tracking:** fails "an exact run: line inside a multi-line quoted + value does not count" and nothing else. The first quoted case could not + isolate this rule, because its content line was not `run:`-shaped anyway. + It was replaced. +- **No owner rule:** fails "a whole job inside a top-level block scalar does not + count". +- **Disqualifiers ignored:** fails the step `if:`, job `if:`, job `if:` after + its steps, `continue-on-error` and both `defaults:` cases. +- **Any job may run any row:** fails "a job running another job's row is + refused". +- **The call may be embedded in other shell:** fails "shell control flow around + the call does not count". +- **Stray mentions not reported:** fails the nine cases that expect a line to + be named. +- **Steps counted outside `steps:`:** fails "a step-shaped item under a job key + other than steps: does not count". The first form of this rival passed, + because it changed only where a step starts while the step-key match still + required `steps:`. The rival was rewritten to treat every job key with an + empty value as opening steps. + +**`checkers-test.sh` gains five `--ci` cases.** The rival that drops "`--ci` +takes a job and nothing else" fails two of them, `--ci` beside a suite name +and beside `--self-test`, each exit 0 where 4 was wanted. + +**Stated, not guarded:** a step earlier in the same job that rewrites +`scripts/checkers.sh` or its table (`context/code/tooling.md`). + +### A continued run: and a skipped need do not count — `scripts/check-ci-graph.py` (improvise-1be2, improvise-bbwa) + +**What broke (1be2, P2, fail-open).** The gate matched `run:` against its own +physical line only. A plain YAML scalar folds a deeper-indented next line into +the value, so a kb step whose `run:` continued onto a line reading `|| true` +ran `./scripts/checkers.sh --ci kb || true`. It counted as the exact call, and +the gate reported "all 9 suites run". A failing suite could then never fail CI. + +**What broke (bbwa, P3).** The gate disqualified `if:` on the step and on its +own job, but not on a job it `needs:`. GitHub skips a job whose need was +skipped, so `if: false` on a needed job meant the call never ran, while the +gate counted it. + +**Fix.** A `run:` whose value owns a following non-blank line is not counted, +and the step is named. `needs:` is read in all three forms (`[a, b]`, a +scalar, a block list) and walked transitively. A job-level `if:` anywhere in +that closure disqualifies the step as "conditional (needs K, which has if)". +The "no CI step runs" message now names the fix site: a row of `ci_suites` in +`scripts/checkers.sh`, not `ci.yml`. + +**Red first.** The three new cases (a continuation line, `needs:` inline and +as a block list) each failed with `saw []`, which is the false pass. + +**Self-test: 23 suite lists.** One of the 23 is the two-hop case, added after +the rival below survived the first 22. + +**Rivals, each a source edit run against the real `--self-test`:** +- **The continuation is not marked:** fails the 1be2 case only. +- **No needs flag:** fails the three needs cases. +- **Block-list `needs:` not read:** fails "the same, with needs: written as a + list" only. +- **No transitive walk:** passed all 22 cases at first, because every needs + case was one hop. "The same, two needs: away" (`kb` needs `mid`, `mid` needs + `gate`) was added, and the rival now fails it alone. + +**Stated, not guarded:** a needed job that *fails* also skips the job, and an +earlier `actions/checkout` with `ref:` replaces the tree +(`context/code/tooling.md`). + +### The gate refuses a key or needs: it cannot read plainly — `scripts/check-ci-graph.py` (improvise-w0j4) + +**What broke (P2, fail-open).** Found in the delta review of #228 at +`2aafff3b`. The bbwa fix read `needs:` names as raw text, and it recognised job +and step keys only in the bare `key:` form. The review put a `gate` job with +`if: false` in the real `ci.yml` and gave `kb` a `needs:` on it. Every one of +these spellings still exited 0 with "all 9 suites run": +- `needs: ["gate"]`, `needs: 'gate'`, and a block item `- "gate"`; +- `needs: &d [gate]`, and `needs: &d` followed by a block list; +- a flow list split over two lines. + +yq read `kb` as needing `gate` in every case. The disqualifier keys had the +same gap: `"if":`, `'continue-on-error':`, `if :`, and `? if` then `: false`. + +**Fix: the review's suggestion, refuse rather than parse.** Every structural +line inside `jobs:` must be one plain form. That means a job name, a job key, a +step key, a `needs:` value that is empty, one bare name or a one-line flow list +of bare names, or a bare block item. Top-level keys must be plain everywhere. +Anything else is reported, naming the line, and the gate fails. + +**Red first.** Eleven refusal cases were added. All eleven failed with `saw []` +against `2aafff3b`, which is the false pass. Two controls, `needs: [ gate ] # +c` and `needs: gate`, still read `gate` and are conditional. Two more cases, +a quoted job name and a quoted top-level key, were added with the fix. + +**Replayed on the real `ci.yml`.** Each attack was mutated in, run, and +restored, with a byte comparison after the last. All eleven spellings now exit +1, and the plain `needs: [gate]` control exits 1 as conditional. + +**The 1be2 boundary.** The earlier case put the continuation two columns past +the key. So a rival that marks multiline only past `owner + 1` passed all 23 +cases. On the real `ci.yml`, a 9-space `|| true` under the kb step's `run:` at +column 8 then exited 0. The new case continues exactly one column past the +key. The review's rival fails it and nothing else. On the real `ci.yml` the +9-space continuation now exits 1. + +**Rivals, each a source edit run against the real `--self-test` (39 cases):** +- `needs:` value accepted as anything: fails the five inline or anchored or + split `needs:` cases. +- Block items accepted as anything: fails the quoted-item case. +- Step keys not checked: fails the quoted step key. +- Job keys not checked: fails `"if":`, `'continue-on-error':`, `if :`, and `?`. +- Job names not checked: fails the quoted job name. +- Top-level keys not checked: fails the quoted top-level key. +- Refusals collected but not reported: fails all thirteen. +- The review's `owner + 1` rival: fails the boundary case alone. + +**Stated, not guarded.** Content under job keys other than `steps:` and +`needs:` is not checked. Neither is anything deeper than a step's own keys. +The gate reads no structure there. + +### A job's keys start at indent 4, or its lines are refused — `scripts/check-ci-graph.py` (improvise-rjaa) + +**What broke (P2, fail-open).** Found in the delta review of #228 at +`3eafd4de`. w0j4's `_readable` accepted any line deeper than 4 outside `steps:` +and `needs:` as content. A *needed* job whose own keys sat at indent 5 or 6, +which is valid YAML, therefore had its `if:` and `needs:` read as content. The +review's three plants each exited 0 with `kb` skipped: +- a 6-space `gate` job, with `kb` given `needs: gate`; +- a 5-space `gate` job, with `kb` given `needs: [gate]`; +- a 6-space `mid` job holding `needs: gate`, with `kb` given `needs: mid`. + +**Fix, as the review suggested.** A job's first key must be at indent 4. After +that, a line at indent 5 to 8 outside `steps:` and `needs:` is content only +once its job has a 4-space key. Such a line can only be the content of an +empty-valued key, because a key with an inline value already owns every deeper +line. + +**Red first.** All three plants, as self-test cases, failed with `saw []`. A +control, `env:` then `FOO: bar` at indent 6, stays accepted as content. + +**Replayed on the real `ci.yml`.** Each plant was mutated in, run, and +restored. All three now exit 1, naming the first misplaced line. + +**Rivals, each run against the real `--self-test` (43 cases):** +- The pre-fix rule (any line past 4 is content): fails the three plants. +- `job_keyed` never reset at a new job: fails the same three. The previous + job's key would otherwise license the next job's misplaced lines. + +**Refused loudly, not read:** an indentless block list, a block list at indent +8, and a quoted top-level key such as `"on":`. Each is valid YAML, and each is +named by line. + +### A comment on a read line is refused, and the matchers share one string — `scripts/check-ci-graph.py` (improvise-j49r) + +**What broke (P2, fail-open).** Found in the delta review of #228 at +`5738e8e0`. `_readable` checked the comment-stripped line, while `_JOB.match` +in both `ci_steps` and `read_graph` ran on the raw line. A header +` gate: # skipped on purpose` was therefore accepted and never opened. Its +`if: false` went to the previous job, and `read_graph` did not list `gate`. +Planted before ` wasm:` in the real `ci.yml`, with `kb` given `needs: gate`, +the gate exited 0: "pages-deploy needs all 7 other jobs; all 9 suites run". + +**The audit found a second instance.** The stripper cut `A: "x # y"` to +`A: "x`, so `_opens_quote` held the following lines open, and a job-level +`if: false` after them was never read. Planted under `kb` in the real +`ci.yml`, this also exited 0 at `5738e8e0`. + +**Fix.** A line containing ` #` is refused, with its own message, and the +stripper is deleted, so every matcher reads the raw line. `read_graph` opens a +job on `_JOB_OPEN` (` name:` followed by whitespace or the end of the line). +That is wider than `_JOB`, because it refuses nothing. The argument is in +`context/code/tooling.md`. + +**Red first** against `5738e8e0`: +- the suite case for the header: `saw []`; +- the `read_graph` case for the header: `saw []`; +- the `"x # y"` case at indent 10: `saw []`. + +The `"x" # note` case read correctly at `5738e8e0`, because the stripper cut it +cleanly. It guards the alternative of reading raw lines without refusing: under +that alternative the raw value `"x" # note` opens the quote tracker. Two old +cases were changed. The positive "own row" case lost its trailing comment, and +that form is now its own refusal case. The w0j4 padded-list control lost its +`# c` and is still a control. + +**Replayed on the real `ci.yml`.** Each plant was mutated in, run and +restored, and the file was compared byte for byte (sha1 `a00288a8…`). Four +plants were run: j49r as reviewed, j49r with `gate` also in pages-deploy's +`needs`, `"x # y"` under `kb`, and `"x" # note` under `kb`. At `5738e8e0` the +first and third exited 0. Now all four exit 1 and name the line. + +**Rivals, each a source edit run against the real `--self-test` (47 cases).** +The review's eleven still fail where the review thread said they would. The +continuation rival fails two cases, the needs flag five, block-list one, +transitive one, flow-only two, `owner + 1` one, `if_jobs` kb-only five, +`_readable` always True fifteen, quoted `_NEEDS_VALUE` one, and each rjaa rival +three. Four rivals are new: +- Raw lines, no refusal: fails the run-line, `"x" # note` and `"x # y"` cases. +- The stripped line everywhere, no refusal (the other option): fails all four + suite cases. +- Refusal only inside `jobs:` at indent 8 or less: fails the indent-10 + `"x # y"` case alone. +- `read_graph` opening on the exact `_JOB`: fails the `read_graph` case alone. + +The header suite case does not separate the raw-without-refusal rival. There +the exact `_JOB` on the raw line refuses the header anyway, and the complaint +has the same text. + +### A multi-line quoted value is refused, and the quote tracker is gone — `scripts/check-ci-graph.py` (improvise-6zf1) + +**What broke (P2, fail-open, predates j49r).** Found in the delta review of #228 +at `588094b7`. The tracker closed a multi-line quoted value at the first line +containing its quote character, including an escaped `\"` or a doubled `''`, +which YAML does not treat as a close. A later content line such as `b: "` then +opened a phantom string that swallowed the job's real `if: false`. The review +planted a `gate` job whose `name:` spans lines, with `kb` given `needs: gate` +and `gate` added to pages-deploy's `needs`. yq-go read `gate.if == false`, and +the gate exited 0 in both quote styles. + +**Fix.** A line whose quoted value does not close on its own line is refused, +with its own message. The tracker is deleted. The real `ci.yml` has no +multi-line quoted value (scanned with `_ANY_KEY` and `_opens_quote`: 0). + +**Red first** against `588094b7`: both plant cases (double and single) gave +`saw []`. + +**New cases, and why each is there.** +- The two plants. Each now draws the refusal, and also "conditional (needs + gate, which has if)", because with the tracker gone the job's `if:` is read. +- An opening line ending in an escaped `\"`, and one ending in a doubled `''`, + each followed by `needs: ok` and a readable closing line (`x: y"`). yq-go + reads each as one string, with `kb.needs == "gate"`. If `_opens_quote` + called either value closed, nothing would be refused, and the gate would + read `needs: ok` in place of kb's real need. These cases are what make + `_opens_quote`'s escape handling load-bearing and tested. +- Two refused styles the review listed: a trailing comment on a step, and + `run: echo "a # b"`. Both were already refused by the j49r rule, so these + cases document behaviour rather than fix a bug. The third style, a quoted + `with:` value containing ` #`, was already the j49r indent-10 case. + +**Changed cases.** +- "an exact run: line inside a multi-line quoted value does not count" is now + "a run: line inside a multi-line quoted value: the quote is refused". It + draws the refusal of the opening line and a plain refusal of the closing + `b"`. Only the exit is guaranteed after a refused line. +- "a closed quote followed by a comment is refused (j49r)" now reads the + job-level `if:` correctly. Its expectation is the refusal plus "conditional + (job if)", which is what the j49r evidence wanted but could not have while + the tracker existed. + +**Replayed on the real `ci.yml`.** I planted the review's three shapes: double +quote, single quote, and double quote with the reopening line at indent 4. At +`588094b7` each exited 0. Now each exits 1, naming the opening line. All seven +plants from both rounds were restored byte for byte (sha1 `a00288a8…`). + +**Rivals, each a source edit run against the real `--self-test` (53 cases).** +All 18 fail: +- the review's eleven; +- the four from j49r; +- three new ones: + - a multi-line quote is not refused: fails the five 6zf1 cases; + - `_opens_quote` ignores `\"`: fails the escaped-`\"` case alone; + - `_opens_quote` ignores `''`: fails the doubled-`''` case alone. + +The j49r rival "no comment refusal" now fails four cases, not three: the +`"x" # note` case is also caught by the quote refusal, because the raw value +`"x" # note` does not end in its quote. + +**Checked, not a finding.** A flow value (`{…}` or `[…]`) that continues onto +another line is not refused. yq-go rejects a continuation at a lesser +indentation (`did not find expected ',' or '}'`), which is loud. At the key's +own indentation the continuation is read as a key, which fails closed: a +spurious `if:`, or a `needs:` value the exact `_NEEDS_VALUE` refuses. + +### Every value is one of a few one-line shapes — `scripts/check-ci-graph.py` (improvise-0wn3) + +**What broke (P2, fail-open).** Found in the delta review of #228 at +`9dc8a643`. A multi-line flow value was not refused: + +``` + - name: [a, "b + run: nix shell nixpkgs#trealla --command ./scripts/checkers.sh --ci kb + c: x"] +``` + +go-yaml, PyYAML 6.0.3 and Psych 5.2.2 read this step as a single `name` string +with no `run:`. The gate read the middle line as the step's `run:` and counted +it. The single-quoted form and a flow map did the same. All three exited 0 on +the real `ci.yml`. + +**This retracts the 6zf1 entry's "Checked, not a finding" note above.** That +note argued a flow continuation "fails closed", but it only considered closing +lines that are not spelled like a step key. + +**Fix: an allowlist over values, not one more refused construct.** Every value +the gate reads must be one of: empty; a block-scalar header; a quoted scalar +closed on its line; a one-line flow with no quotes, nesting or `#`; or a plain +scalar not starting with a YAML indicator. That covers a key's value, a list +item's, or a line of its own. Anything else is refused. The argument is in +`context/code/tooling.md`. + +The real `ci.yml` has 153 plain values, 41 empty, 2 one-line flows and 5 block +headers, and none is refused. When a line is both an unreadable key and an +unreadable value, the key refusal's message wins, so w0j4's cases keep their +diagnosis. + +**Red first** against `9dc8a643`. All seven new cases gave `saw []`: +- the review's three: `[` double-quoted, `[` single-quoted, `{`; +- `[a, "b]`, which ends in its closer with its quote still open. This is what + the review's proposed fix ("does not close on the same line") would pass if + it were read as "ends in `]`"; +- a flow opened on a line of its own, under an empty `name:`. yq-go reads it + as one string too; +- an anchored flow (`&a [x, "b`) and a tagged one (`!!seq [x, "b`), each + confirmed with yq-go to swallow the `run:`. + +**Replayed on the real `ci.yml`.** I replaced kb's checkers step with each of +the review's three plants. At `9dc8a643` all three exit 0. Now all three exit +1, naming the opening line. All ten plants from the four rounds exit 1, and +the file was restored byte for byte (sha1 `a00288a8…`). + +**Rivals, each a source edit run against the real `--self-test` (60 cases).** +All 22 fail. The review's sixteen are my 01–11, the comment rival, the +`read_graph` rival, the old quote-refusal rival (now "value allowlist removed", +which fails 12 cases), and both `_opens_quote` rivals. The four new ones: +- a flow need only end in its closer: fails `[a, "b]` alone; +- any flow start allowed: fails the five flow cases; +- only a key's value is checked: fails the own-line flow case alone; +- anchors and tags read as plain: fails the anchored and tagged cases. + +**A valid style newly refused.** A quoted key below the `_readable` depth, such +as `"FOO": bar` under `env:`, is refused, because the gate cannot tell it from +an unclosed quote. Checked directly: refused, and the plain `FOO: bar` control +is accepted. + +### A line break YAML knows and the gate does not is refused — `scripts/check-ci-graph.py` (improvise-xpzh) + +**What broke (P2, fail-open).** Found in the delta review of #228 at +`7ae871cb`. The gate splits on `\n` only, but YAML also breaks lines at U+0085, +U+2028 and U+2029. `runs-on: ubuntu-latest if: false` on the real +`kb` job is one line to the gate and two to go-yaml. yq-go reads +`kb.if == false` with each of the three characters, and the gate exited 0. + +**Fix.** `_UNSAFE_CHAR` is checked on every line, before the comment and owner +skips. It covers C0 except tab, DEL, all of C1 (which holds U+0085), U+2028 and +U+2029. The workflow is read through `read_workflow`, whose universal newlines +fold CR and CRLF. The argument, including why all non-ASCII is not refused, is +in `context/code/tooling.md`. The real `ci.yml` has 0 CR bytes, no BOM and 27 +printable non-ASCII characters, none of them in the refused set. + +**Red first** against `7ae871cb`. All five new suite cases gave `saw []`: +- one per break character, each on a `runs-on:` line; +- a U+2028 inside a full-line comment; +- a U+2028 inside a `run: |` body, on the job's last step. yq-go ends the + block there and reads the `if:` as a job key. + +**Controls.** Two cases write a real file and read it back through +`read_workflow`: +- CRLF line ends are accepted with no complaint; +- a lone CR is a line break, so the following `if:` makes kb conditional, as + YAML reads it. + +**Replayed on the real `ci.yml`.** The three plants, one per character, on +kb's `runs-on:`, exit 0 at `7ae871cb`. Now all three exit 1, naming line 255. +All 13 plants from the five rounds exit 1. The file was written with +`newline=""` so the characters landed literally, and it was restored byte for +byte (sha1 `a00288a8…`). + +**Rivals, each a source edit run against the real `--self-test` (65 suite +lists, 2 files).** All 27 fail. The 22 from earlier rounds fail as before. +Five are new: + +| Rival | Fails | +|---|---| +| The character check removed | the five new cases | +| C1 allowed | the U+0085 case alone | +| Comments not checked | the comment case alone | +| Owned content not checked | the block-scalar case, and the comment case | +| The read does not fold CR (`newline=""`) | both file controls | + +The owned-content rival also fails the comment case because `owner` still +holds the preceding `runs-on:` key's column when that line is checked. + +### A `defaults: run: shell:` no longer disqualifies a step — `scripts/check-ci-graph.py` (the user's ruling on #228) + +**What changed, and why.** The gate refused to count any step under a +workflow- or job-level `defaults:` block. The user ruled it "a silly check". +It also broke on contact with main: PR #287 added a workflow-level +`defaults: run: shell: nix develop .#ci --command bash …`, and on the merged +tree every suite read as run by no CI step. The refusal is removed. + +**Cases.** Two cases pinned the refusal: "a job's defaults: can change the +shell, so it does not count" and "workflow-level defaults: disqualifies every +step". Both are deleted. The new case "a step under a defaults: run: shell: is +counted" carries both a workflow-level default in #287's shape and a +job-level one, and expects no complaint. + +**Red first.** Against the code before the removal, the new case drew both +steps as "conditional (workflow defaults)" and kb's as "(job defaults, workflow +defaults)", so both rows went unrun. + +**Kept.** A step's own `shell:` still disqualifies it; the ruling did not cover +that. The w0j4 case refusing a quoted `"defaults":` key is about key spelling, +and it stands. + ### The audit tells grep's no-match from its read error — `scripts/bd-audit-ids-test.sh` (improvise-lqs0) **What broke.** `bd-audit-ids` treated xargs exit 123 as "a grep batch matched diff --git a/scripts/check-ci-graph.py b/scripts/check-ci-graph.py index c3fc9911..88ef1510 100755 --- a/scripts/check-ci-graph.py +++ b/scripts/check-ci-graph.py @@ -6,16 +6,26 @@ README name three of seven while the rest read as ungated (improvise-x7cq). Stating it was not enough -- a new job was added during review of PR #83 and nothing noticed, so the rule is a gate. See context/code/tooling.md. +Second rule: every suite `scripts/checkers.sh --list` knows must run in some +CI step. ci.yml names its suites by hand, so a new `*-test.sh` ran locally and +never in CI until someone remembered to add it (improvise-dak5). Each job runs +`checkers.sh --ci `, whose table lives in checkers.sh; no shell is parsed +here, and what a row runs is asked of `--would-run` (improvise-maax, st8q). + `--self-test` checks the checker: each case is a graph this must refuse. """ +import os import re +import subprocess import sys +import tempfile WORKFLOW = ".github/workflows/ci.yml" DEPLOY = "pages-deploy" _JOB = re.compile(r"^ ([A-Za-z0-9_-]+):$") +_JOB_OPEN = re.compile(r"^ ([A-Za-z0-9_-]+):(\s|$)") _NEEDS = re.compile(r"^ needs:\s*\[(.*)\]\s*$") @@ -24,7 +34,9 @@ def read_graph(text): Both come off indentation, which is what a workflow's own shape is. A job key is two spaces; `needs:` is four INSIDE the job it belongs to, so the - current job is tracked rather than the first `needs:` taken. + current job is tracked rather than the first `needs:` taken. A job opens on + `_JOB_OPEN`, wider than `_JOB`: this reader refuses nothing, so it reads wide + (improvise-j49r). """ jobs, needs, current = [], None, None in_jobs = False @@ -34,7 +46,7 @@ def read_graph(text): continue if not in_jobs: continue - m = _JOB.match(raw) + m = _JOB_OPEN.match(raw) if m: current = m.group(1) jobs.append(current) @@ -60,6 +72,611 @@ def verdict(jobs, needs): return out +_JOB_KEY = re.compile(r"^ ([A-Za-z_][\w-]*):(.*)$") +_STEP_START = re.compile(r"^ - ") +_STEP_KEY = re.compile(r"^ (?:- | )([A-Za-z_][\w-]*):(.*)$") +_ANY_KEY = re.compile(r"^(\s*)(?:- )?([A-Za-z_][\w-]*):(.*)$") +_CI_CALL = re.compile(r"^(?:nix shell \S+ --command )?\./scripts/checkers\.sh --ci ([\w-]+)$") +_DISQUALIFY = ("if", "continue-on-error", "shell") + + +_NAME = r"[A-Za-z_][\w-]*" +_TOP_KEY = re.compile(rf"^{_NAME}:(\s|$)") +_NEEDS_VALUE = re.compile(rf"^(|{_NAME}|\[\s*{_NAME}(\s*,\s*{_NAME})*\s*\])$") +_NEEDS_ITEM = re.compile(rf"^ - {_NAME}$") +_COMMENT = re.compile(r"\s#") +_UNSAFE_CHAR = re.compile("[\x00-\x08\x0b-\x1f\x7f-\x9f\u2028\u2029]") + + +def _readable(line, indent, in_steps, in_needs, job_keyed): + """Whether a structural line inside `jobs:` is one this gate can read. Anything + else is refused, not parsed: context/code/tooling.md (improvise-w0j4, rjaa).""" + if indent == 2: + return bool(_JOB.match(line)) + if indent == 4: + m = _JOB_KEY.match(line) + return bool(m) and (m.group(1) != "needs" + or bool(_NEEDS_VALUE.match(_inline_value(m.group(2))))) + if in_needs: + return bool(_NEEDS_ITEM.match(line)) + if in_steps: + return indent in (6, 8) and bool(_STEP_KEY.match(line)) + return indent > 4 and job_keyed + + +def _inline_value(rest): + """A key's inline value. No comment is stripped: a line carrying one is + refused, so every matcher reads the same string (improvise-j49r).""" + return rest.strip() + + +_FLOW_LINE = re.compile(r"""^(\[[^\[\]{}"'#]*\]|\{[^\[\]{}"'#]*\})$""") +_BLOCK_HEADER = re.compile(r"^[|>][-+0-9]*$") + + +def _plain_value(value): + """Whether a value is one of the few shapes the gate reads, all ending on their own + line; anything else is refused: context/code/tooling.md (improvise-0wn3).""" + if value == "" or _BLOCK_HEADER.match(value) or _FLOW_LINE.match(value): + return True + if value[0] in "\"'": + return not _opens_quote(value) + return value[0] not in "[]{}&*!|>%@`?," + + +def _line_value(raw): + """The value a line carries: a key's, a list item's, or the whole line's.""" + k = _ANY_KEY.match(raw) + if k: + return _inline_value(k.group(3)) + body = raw.strip() + return body[2:].strip() if body.startswith("- ") else body + + +def _opens_quote(value): + """True when a quoted inline value is not closed on its own line.""" + if value[:1] not in ("'", '"') or len(value) < 2: + return value[:1] in ("'", '"') + q, body = value[0], value[1:] + if q == "'": + return not body.replace("''", "").endswith("'") + return not re.sub(r'\\.', "", body).endswith('"') + + +def ci_steps(text): + """(job, value, disqualifiers) for every step of every job, and every line + mentioning scripts/checkers.sh that is not a step's whole `run:` value. + + Nothing is read as shell. A key with an inline value owns every following + line indented deeper, which is how a block scalar, a folded value, a plain + continuation or a heredoc inside one is skipped whole (improvise-st8q).""" + steps, stray, unread = [], [], [] + job, in_steps, job_flags, step = None, False, set(), None + in_jobs, job_keyed = False, False + owner, owner_step = None, None + needs, if_jobs, in_needs = {}, set(), False + + def close_step(): + if step is not None: + steps.append(step) + + for n, raw in enumerate(text.split("\n"), 1): + if _UNSAFE_CHAR.search(raw): + unread.append((n, "char")) + continue + indent = len(raw) - len(raw.lstrip()) + if owner is not None and (not raw.strip() or indent > owner): + if "scripts/checkers.sh" in raw: + stray.append(n) + if owner_step is not None and raw.strip(): + owner_step["multiline"] = True + continue + owner, owner_step = None, None + if not raw.strip() or raw.lstrip().startswith("#"): + continue + if indent == 0: + in_jobs = raw.startswith("jobs:") + readable = bool(_TOP_KEY.match(raw)) + else: + readable = not in_jobs or indent > 8 or _readable(raw, indent, in_steps, + in_needs, job_keyed) + k = _ANY_KEY.match(raw) + shaped = _plain_value(_line_value(raw)) + if _COMMENT.search(raw) or not shaped or not readable: + unread.append((n, "comment" if _COMMENT.search(raw) else + "plain" if not readable else "value")) + m = _JOB.match(raw) + if m: + close_step() + step = None + job, in_steps, job_flags, in_needs = m.group(1), False, set(), False + job_keyed = False + continue + if in_needs and re.match(r"^ - ", raw): + needs[job].add(_inline_value(raw.strip()[2:])) + continue + m = _JOB_KEY.match(raw) + if m and job is not None: + job_keyed = True + close_step() + step = None + key, value = m.group(1), _inline_value(m.group(2)) + in_steps = key == "steps" and value == "" + in_needs = key == "needs" and value == "" + if key == "needs": + needs[job] = {x.strip() for x in value.strip("[]").split(",") if x.strip()} + if key == "if": + if_jobs.add(job) + if key in _DISQUALIFY: + job_flags.add(key) + for s in steps: + if s["job"] == job: + s["flags"].add("job " + key) + if in_steps and _STEP_START.match(raw): + close_step() + step = {"job": job, "run": None, "flags": {"job " + f for f in job_flags}, + "line": n} + m = _STEP_KEY.match(raw) if in_steps and step is not None else None + if m: + key, value = m.group(1), _inline_value(m.group(2)) + if key == "run": + step["run"] = value + if "scripts/checkers.sh" in raw and not _CI_CALL.match(value): + stray.append(n) + elif key in _DISQUALIFY: + step["flags"].add(key) + elif "scripts/checkers.sh" in raw: + stray.append(n) + elif "scripts/checkers.sh" in raw: + stray.append(n) + if k and _inline_value(k.group(3)): + owner = len(k.group(1)) + (2 if raw.lstrip().startswith("- ") else 0) + owner_step = step if m and m.group(1) == "run" else None + close_step() + for s in steps: + s["flags"] |= {f"needs {j}, which has if" for j in _needed(s["job"], needs) & if_jobs} + return steps, stray, unread + + +def _needed(job, needs): + """Every job `job` needs, transitively: a skipped one skips it too (improvise-bbwa).""" + seen, todo = set(), list(needs.get(job, ())) + while todo: + j = todo.pop() + if j not in seen: + seen.add(j) + todo += needs.get(j, ()) + return seen + + +def would_run(args): + """What checkers.sh itself says these arguments run. Its argument rules and + its CI table are not restated here (improvise-maax).""" + out = subprocess.run(["scripts/checkers.sh", "--would-run", *args], + capture_output=True, text=True).stdout + ran, missing = set(), set() + for line in out.split("\n"): + if line.startswith("would-run: "): + ran.add(line[len("would-run: "):]) + elif line.startswith("missing: "): + missing.add(line[len("missing: "):]) + return ran, missing + + +def _unrun(s): + return (f"`{s}` is a checkers.sh suite that no CI step runs; add it to a row of " + "ci_suites in scripts/checkers.sh") + + +def _unread(n, kind="plain"): + if kind == "char": + return (f"ci.yml line {n} holds a control character or a line break YAML sees and " + "this gate does not (U+0085, U+2028, U+2029); remove it") + if kind == "value": + return (f"ci.yml line {n} has a value that is not one plain line (a multi-line " + "quote or flow, a nested or quoted flow, an anchor, alias or tag); spell " + "it plainly") + if kind == "comment": + return (f"ci.yml line {n} has a ` #`, which YAML may read as a comment; this gate " + "reads no comments, so put it on a line of its own") + return (f"ci.yml line {n} is a key or `needs:` value this gate cannot read as a " + "plain name, so it cannot trust the graph; spell it plainly") + + +def suite_verdict(suites, text): + """The complaints: suites no counted step runs, and every step or line that + does not count, each named so the fix is obvious rather than silent.""" + steps, stray, unread = ci_steps(text) + out, ran, missing = [_unread(n, c) for n, c in unread], set(), set() + for s in steps: + m = _CI_CALL.match(s["run"] or "") + if not m: + continue + if m.group(1) != s["job"]: + out.append(f"job `{s['job']}` runs `checkers.sh --ci {m.group(1)}`; " + "a job runs only its own row of the CI table") + elif s.get("multiline"): + out.append(f"job `{s['job']}`'s `checkers.sh --ci` step's run: continues " + "onto another line, so it does not count") + elif s["flags"]: + out.append(f"job `{s['job']}`'s `checkers.sh --ci` step is conditional " + f"({', '.join(sorted(s['flags']))}), so it does not count") + else: + r, mi = would_run(["--ci", s["job"]]) + ran |= r + missing |= mi + out += [f"ci.yml line {n} mentions scripts/checkers.sh outside a step whose " + "whole `run:` is `./scripts/checkers.sh --ci `" for n in stray] + out += [_unrun(s) for s in sorted(set(suites) - ran)] + out += [f"checkers.sh's CI table names `{s}`, which is not a suite" + for s in sorted(missing)] + return out + + +def read_workflow(path): + """The workflow's text. Universal newlines fold CR and CRLF into the one break the + gate splits on; every other break YAML knows is refused (improvise-xpzh).""" + return open(path, encoding="utf8").read() + + +def list_suites(): + """The suites, from checkers.sh itself, so its naming rule has one home.""" + out = subprocess.run(["scripts/checkers.sh", "--list"], capture_output=True, + text=True, check=True).stdout + return [s for s in out.split("\n") if s.strip()] + + +def _wf(*jobs): + return "jobs:\n" + "".join(jobs) + + +def _job(name, *steps, head=""): + return f" {name}:\n runs-on: x\n{head} steps:\n" + "".join(steps) + + +_BUILD = _job("build", " - run: ./scripts/checkers.sh --ci build\n") +_REQ = ["assemble-stamp", "pr-watch"] +_NO_PW = [_unrun("pr-watch")] + + +def _stray(n): + return (f"ci.yml line {n} mentions scripts/checkers.sh outside a step whose " + "whole `run:` is `./scripts/checkers.sh --ci `") + + +_GATE = " gate:\n if: false\n steps:\n - run: echo\n" +_KB_RUN = " - run: ./scripts/checkers.sh --ci kb\n" +_YAML_ONLY_BREAKS = ("\x85", "\u2028", "\u2029") +_INNER_RUN = " run: ./scripts/checkers.sh --ci kb\n" +_NEEDS_GATE = ["job `kb`'s `checkers.sh --ci` step is conditional (needs gate, which has if), " + "so it does not count"] + _NO_PW + + +def _unreadable(name, text, *markers, rest=(), kind="plain"): + """A case whose workflow spells a key or a `needs:` value the gate must refuse.""" + lines = text.split("\n") + at = [next(i for i, l in enumerate(lines, 1) if m in l) for m in markers] + return (name, _REQ, text, [_unread(n, kind) for n in at] + list(rest)) + + +# (name, suites to require, workflow text, the complaints it must draw). Every +# case asks the REAL checkers.sh table, where `kb`'s row holds pr-watch. +SUITE_SELF_TEST = [ + ("two jobs each running their own row are accepted", _REQ, + _wf(_BUILD, _job("kb", " - run: nix shell nixpkgs#trealla --command " + "./scripts/checkers.sh --ci kb\n")), []), + + _unreadable("a trailing comment on the counted run: line is refused (j49r)", + _wf(_BUILD, _job("kb", _KB_RUN.rstrip("\n") + " # the kb row\n")), + "# the kb row", rest=[_stray(9)] + _NO_PW, kind="comment"), + + ("a job that never calls its row leaves its suites unrun", _REQ, + _wf(_BUILD, _job("kb", " - run: echo nothing\n")), _NO_PW), + + ("a call inside a heredoc in a block run: does not count (st8q)", _REQ, + _wf(_BUILD, _job("kb", " - run: |\n cat >> x <<'EOF'\n" + " ./scripts/checkers.sh --ci kb\n EOF\n")), + [_stray(11)] + _NO_PW), + + ("a folded run: does not count (st8q)", _REQ, + _wf(_BUILD, _job("kb", " - run: >\n ./scripts/checkers.sh --ci kb\n")), + [_stray(10)] + _NO_PW), + + ("a plain continuation line does not count", _REQ, + _wf(_BUILD, _job("kb", " - run: echo\n ./scripts/checkers.sh --ci kb\n")), + [_stray(10)] + _NO_PW), + + _unreadable("a run: line inside a multi-line quoted value: the quote is refused (6zf1)", + _wf(_BUILD, _job("kb", " - name: \"a\n" + " run: ./scripts/checkers.sh --ci kb\n b\"\n")), + '- name: "a', kind="value", rest=[_unread(11)]), + + ("a whole job inside a top-level block scalar does not count", _REQ, + _wf(_BUILD) + "notes: |\n kb:\n steps:\n" + " - run: ./scripts/checkers.sh --ci kb\n", + [_stray(9)] + _NO_PW), + + ("a step-shaped line inside a job-level block scalar does not count", _REQ, + _wf(_BUILD, _job("kb", " - run: echo\n", + head=" notes: |\n - run: ./scripts/checkers.sh --ci kb\n")), + [_stray(9)] + _NO_PW), + + ("a step-shaped item under a job key other than steps: does not count", _REQ, + _wf(_BUILD, " kb:\n services:\n - run: ./scripts/checkers.sh --ci kb\n" + " steps:\n - run: echo\n"), + [_stray(8)] + _NO_PW), + + ("the call with a continuation line does not count (1be2)", _REQ, + _wf(_BUILD, _job("kb", " - run: ./scripts/checkers.sh --ci kb\n" + " || true\n")), + ["job `kb`'s `checkers.sh --ci` step's run: continues onto another line, " + "so it does not count"] + _NO_PW), + + ("a job that needs a job under if: does not count (bbwa)", _REQ, + _wf(_BUILD, " gate:\n if: false\n steps:\n - run: echo\n", + _job("kb", " - run: ./scripts/checkers.sh --ci kb\n", + head=" needs: [build, gate]\n")), + ["job `kb`'s `checkers.sh --ci` step is conditional (needs gate, which has if), " + "so it does not count"] + _NO_PW), + + ("the same, with needs: written as a list", _REQ, + _wf(_BUILD, " gate:\n if: false\n steps:\n - run: echo\n", + _job("kb", " - run: ./scripts/checkers.sh --ci kb\n", + head=" needs:\n - gate\n")), + ["job `kb`'s `checkers.sh --ci` step is conditional (needs gate, which has if), " + "so it does not count"] + _NO_PW), + + ("the 1be2 continuation exactly one column past the key does not count (w0j4)", _REQ, + _wf(_BUILD, _job("kb", _KB_RUN + " || true\n")), + ["job `kb`'s `checkers.sh --ci` step's run: continues onto another line, " + "so it does not count"] + _NO_PW), + + ("control: a padded flow list is still read (w0j4)", _REQ, + _wf(_BUILD, _GATE, _job("kb", _KB_RUN, head=" needs: [ gate ]\n")), + _NEEDS_GATE), + + ("control: a plain scalar needs: is still read (w0j4)", _REQ, + _wf(_BUILD, _GATE, _job("kb", _KB_RUN, head=" needs: gate\n")), + _NEEDS_GATE), + + _unreadable("a quoted name in a flow needs: is refused (w0j4)", + _wf(_BUILD, _GATE, _job("kb", _KB_RUN, head=' needs: ["gate"]\n')), + 'needs: ["gate"]'), + + _unreadable("a quoted scalar needs: is refused (w0j4)", + _wf(_BUILD, _GATE, _job("kb", _KB_RUN, head=" needs: 'gate'\n")), + "needs: 'gate'"), + + _unreadable("a quoted item in a block needs: is refused (w0j4)", + _wf(_BUILD, _GATE, _job("kb", _KB_RUN, head=' needs:\n - "gate"\n')), + '- "gate"'), + + _unreadable("an anchored flow needs: is refused (w0j4)", + _wf(_BUILD, _GATE, _job("kb", _KB_RUN, head=" needs: &d [gate]\n")), + "needs: &d"), + + _unreadable("an anchored block needs: is refused (w0j4)", + _wf(_BUILD, _GATE, _job("kb", _KB_RUN, head=" needs: &d\n - gate\n")), + "needs: &d"), + + _unreadable("a flow needs: over two lines is refused (w0j4)", + _wf(_BUILD, _GATE, _job("kb", _KB_RUN, head=" needs: [build,\n gate]\n")), + "needs: [build,"), + + _unreadable("a quoted job key is refused (w0j4)", + _wf(_BUILD, _job("kb", _KB_RUN, head=' "if": false\n')), + '"if"'), + + _unreadable("a quoted step key is refused (w0j4)", + _wf(_BUILD, _job("kb", " - \"if\": false\n" + " run: ./scripts/checkers.sh --ci kb\n")), + '"if"'), + + _unreadable("a single-quoted continue-on-error is refused (w0j4)", + _wf(_BUILD, _job("kb", _KB_RUN, head=" 'continue-on-error': true\n")), + "'continue-on-error'"), + + _unreadable("a key with a space before its colon is refused (w0j4)", + _wf(_BUILD, _job("kb", _KB_RUN, head=" if : false\n")), + "if : false"), + + _unreadable("a quoted job name is refused (w0j4)", + _wf(_BUILD, ' "gate":\n if: false\n steps:\n - run: echo\n', + _job("kb", _KB_RUN)), + '"gate"', + rest=["job `build`'s `checkers.sh --ci` step is conditional (job if), " + "so it does not count", _unrun("assemble-stamp")]), + + _unreadable("a quoted top-level key is refused (w0j4)", + '"defaults":\n run:\n shell: bash\n' + _wf(_BUILD, _job("kb", _KB_RUN)), + '"defaults"'), + + ("control: a job key's block content at indent 6 is content (rjaa)", _REQ, + _wf(_BUILD, _job("kb", _KB_RUN, head=" env:\n FOO: bar\n")), []), + + _unreadable("a needed job whose keys sit at indent 6 is refused (rjaa)", + _wf(_BUILD, " gate:\n if: false\n steps:\n - run: echo\n", + _job("kb", _KB_RUN, head=" needs: gate\n")), + " if: false", " steps:", " - run: echo"), + + _unreadable("a needed job whose keys sit at indent 5 is refused (rjaa)", + _wf(_BUILD, " gate:\n if: false\n steps:\n - run: echo\n", + _job("kb", _KB_RUN, head=" needs: [gate]\n")), + " if: false", " steps:", " - run: echo"), + + _unreadable("a job between, with its needs: at indent 6, is refused (rjaa)", + _wf(_BUILD, _GATE, + " mid:\n needs: gate\n steps:\n - run: echo\n", + _job("kb", _KB_RUN, head=" needs: mid\n")), + " needs: gate", " steps:", " - run: echo"), + + _unreadable("an explicit ? key is refused (w0j4)", + _wf(_BUILD, _job("kb", _KB_RUN, head=" ? if\n : false\n")), + "? if", ": false"), + + ("the same, two needs: away", _REQ, + _wf(_BUILD, " gate:\n if: false\n steps:\n - run: echo\n", + " mid:\n needs: gate\n steps:\n - run: echo\n", + _job("kb", " - run: ./scripts/checkers.sh --ci kb\n", + head=" needs: [mid]\n")), + ["job `kb`'s `checkers.sh --ci` step is conditional (needs gate, which has if), " + "so it does not count"] + _NO_PW), + + ("a step under if: does not count (tvte)", _REQ, + _wf(_BUILD, _job("kb", " - if: false\n run: ./scripts/checkers.sh --ci kb\n")), + ["job `kb`'s `checkers.sh --ci` step is conditional (if), so it does not count"] + + _NO_PW), + + ("a job under if: does not count (tvte)", _REQ, + _wf(_BUILD, _job("kb", " - run: ./scripts/checkers.sh --ci kb\n", + head=" if: false\n")), + ["job `kb`'s `checkers.sh --ci` step is conditional (job if), so it does not count"] + + _NO_PW), + + ("a job-level if: after its steps still disqualifies them", _REQ, + _wf(_BUILD, " kb:\n steps:\n - run: ./scripts/checkers.sh --ci kb\n" + " if: false\n"), + ["job `kb`'s `checkers.sh --ci` step is conditional (job if), so it does not count"] + + _NO_PW), + + ("a step under a defaults: run: shell: is counted (the user's ruling)", _REQ, + "defaults:\n run:\n shell: nix develop .#ci --command bash -e {0}\n" + + _wf(_BUILD, _job("kb", _KB_RUN, head=" defaults:\n run:\n shell: bash\n")), + []), + + ("continue-on-error does not count", _REQ, + _wf(_BUILD, _job("kb", " - run: ./scripts/checkers.sh --ci kb\n" + " continue-on-error: true\n")), + ["job `kb`'s `checkers.sh --ci` step is conditional (continue-on-error), " + "so it does not count"] + _NO_PW), + + ("shell control flow around the call does not count", _REQ, + _wf(_BUILD, _job("kb", " - run: true || ./scripts/checkers.sh --ci kb\n")), + [_stray(9)] + _NO_PW), + + ("a job running another job's row is refused", _REQ, + _wf(_BUILD, _job("kb", " - run: ./scripts/checkers.sh --ci build\n")), + ["job `kb` runs `checkers.sh --ci build`; a job runs only its own row of the CI table"] + + _NO_PW), + + ("a bare mention in a step name is refused", _REQ, + _wf(_BUILD, _job("kb", " - name: runs scripts/checkers.sh\n" + " run: ./scripts/checkers.sh --ci kb\n")), + [_stray(9)]), + + _unreadable("a job header with a trailing comment is refused (j49r)", + _wf(_BUILD, " test:\n runs-on: x\n", + " gate: # skipped on purpose\n if: false\n steps:\n - run: echo\n", + _job("kb", _KB_RUN, head=" needs: gate\n")), + "gate: # skipped", kind="comment"), + + _unreadable("a closed quote followed by a comment is refused (j49r)", + _wf(_BUILD, _job("kb", _KB_RUN, head=" env:\n A: \"x\" # note\n") + + " if: false\n", " after:\n steps:\n - run: echo \"z\"\n"), + '"x" # note', kind="comment", + rest=["job `kb`'s `checkers.sh --ci` step is conditional (job if), " + "so it does not count"] + _NO_PW), + + _unreadable("a # inside a quoted value, below indent 8, is refused (j49r)", + _wf(_BUILD, _job("kb", _KB_RUN + " - uses: x\n with:\n" + " foo: \"x # y\"\n") + + " if: false\n", " after:\n steps:\n - run: echo \"z\"\n"), + '"x # y"', kind="comment", + rest=["job `kb`'s `checkers.sh --ci` step is conditional (job if), " + "so it does not count"] + _NO_PW), + + _unreadable("a trailing comment on a step is refused (j49r)", + _wf(_BUILD, _job("kb", " - uses: actions/checkout@v7 # pin\n" + _KB_RUN)), + "# pin", kind="comment"), + + _unreadable("a one-line run: with a ` #` inside quotes is refused (j49r)", + _wf(_BUILD, _job("kb", " - run: echo \"a # b\"\n" + _KB_RUN)), + '"a # b"', kind="comment"), + + _unreadable("a multi-line double-quoted value is refused, escaped quote and all (6zf1)", + _wf(_BUILD, " gate:\n runs-on: x\n name: \"a\n it\\\" is\n b: \"\n" + " if: false\n steps:\n - run: echo \"z\"\n", + _job("kb", _KB_RUN, head=" needs: gate\n")), + 'name: "a', kind="value", rest=_NEEDS_GATE), + + _unreadable("a multi-line single-quoted value is refused, doubled quote and all (6zf1)", + _wf(_BUILD, " gate:\n runs-on: x\n name: 'a\n it''s\n b: '\n" + " if: false\n steps:\n - run: echo 'z'\n", + _job("kb", _KB_RUN, head=" needs: gate\n")), + "name: 'a", kind="value", rest=_NEEDS_GATE), + + _unreadable("an opening line ending in an escaped \\\" still opens a quote (6zf1)", + _wf(_BUILD, _GATE, _job("kb", _KB_RUN, head=" needs: gate\n name: \"a\\\"\n" + " needs: ok\n x: y\"\n")), + 'name: "a', kind="value"), + + _unreadable("an opening line ending in a doubled '' still opens a quote (6zf1)", + _wf(_BUILD, _GATE, _job("kb", _KB_RUN, head=" needs: gate\n name: 'it''\n" + " needs: ok\n x: y'\n")), + "name: 'it", kind="value"), + + _unreadable("a flow list whose quote runs onto the next lines is refused (0wn3)", + _wf(_BUILD, _job("kb", " - name: [a, \"b\n" + _INNER_RUN + " c: x\"]\n")), + 'name: [a, "b', kind="value"), + + _unreadable("the same with a single-quoted string (0wn3)", + _wf(_BUILD, _job("kb", " - name: [a, 'b\n" + _INNER_RUN + " c: x']\n")), + "name: [a, 'b", kind="value"), + + _unreadable("the same as a flow map (0wn3)", + _wf(_BUILD, _job("kb", " - name: {a: \"b\n" + _INNER_RUN + " c: x\"}\n")), + 'name: {a: "b', kind="value"), + + _unreadable("a flow list that ends in its closer with a quote still open is refused (0wn3)", + _wf(_BUILD, _job("kb", " - name: [a, \"b]\n" + _INNER_RUN + " c: x\"]\n")), + 'name: [a, "b]', kind="value"), + + _unreadable("a flow value opened on a line of its own is refused too (0wn3)", + _wf(_BUILD, _job("kb", " - name:\n [a, \"b\n" + _INNER_RUN + + " c: x\"]\n")), + '[a, "b', kind="value"), + + _unreadable("an anchored multi-line flow is refused (0wn3)", + _wf(_BUILD, _job("kb", " - name: &a [x, \"b\n" + _INNER_RUN + " c: x\"]\n")), + 'name: &a', kind="value"), + + _unreadable("a tagged multi-line flow is refused (0wn3)", + _wf(_BUILD, _job("kb", " - name: !!seq [x, \"b\n" + _INNER_RUN + + " c: x\"]\n")), + 'name: !!seq', kind="value"), + + *[_unreadable(f"a U+{ord(sep):04X} inside a line is refused (xpzh)", + _wf(_BUILD, " kb:\n runs-on: x" + sep + " if: false\n steps:\n" + _KB_RUN), + sep, kind="char") + for sep in _YAML_ONLY_BREAKS], + + _unreadable("a line break inside a block scalar's body is refused too (xpzh)", + _wf(_BUILD, _job("kb", _KB_RUN + " - run: |\n echo a\u2028 if: false\n")), + "echo a", kind="char"), + + _unreadable("a line break inside a full-line comment is refused too (xpzh)", + _wf(_BUILD, " kb:\n runs-on: x\n # note\u2028 if: false\n steps:\n" + + _KB_RUN), "# note", kind="char"), + + ("a YAML comment mentioning checkers.sh is fine", _REQ, + _wf(_BUILD, _job("kb", " # every suite goes through scripts/checkers.sh\n" + " - run: ./scripts/checkers.sh --ci kb\n")), []), +] + + +# (name, file bytes as text, the complaints it must draw), read back through +# read_workflow, because what a CR means depends on that read (improvise-xpzh). +FILE_SELF_TEST = [ + ("control: CRLF line ends are folded by the read and accepted (xpzh)", + _wf(_BUILD, _job("kb", _KB_RUN)).replace("\n", "\r\n"), []), + + ("control: a lone CR is a line break, as YAML reads it (xpzh)", + _wf(_BUILD, " kb:\n runs-on: x\r if: false\n steps:\n" + _KB_RUN), + ["job `kb`'s `checkers.sh --ci` step is conditional (job if), so it does not count"] + + _NO_PW), +] + + # (name, workflow text, the complaints it must draw). SELF_TEST = [ ("a job nobody gates is named", @@ -79,6 +696,11 @@ SELF_TEST = [ "jobs:\n test:\n x: 1\n pages-deploy:\n x: 1\n", [f"{DEPLOY} has no `needs:` list, so nothing gates the deploy"]), + ("a job header with a trailing comment is still a job (j49r)", + "jobs:\n test:\n x: 1\n gate: # skipped on purpose\n x: 1\n" + " pages-deploy:\n needs: [test]\n", + [f"{DEPLOY} does not need `gate`, so a failing gate cannot stop a deploy"]), + # `needs:` belongs to the job it is indented under. Taking the first one # seen would read ANOTHER job's list as the deploy's and pass. ("another job's needs is not mistaken for the deploy's", @@ -95,28 +717,48 @@ def self_test(): got = verdict(jobs, needs) if got != want: failures.append(f" {name}: expected {want}, saw {got}") + for name, suites, text, want in SUITE_SELF_TEST: + got = suite_verdict(suites, text) + if got != want: + failures.append(f" {name}: expected {want}, saw {got}") + for name, content, want in FILE_SELF_TEST: + with tempfile.TemporaryDirectory() as d: + path = os.path.join(d, "ci.yml") + with open(path, "w", encoding="utf8", newline="") as f: + f.write(content) + got = suite_verdict(_REQ, read_workflow(path)) + if got != want: + failures.append(f" {name}: expected {want}, saw {got}") if failures: print("ci-graph self-test FAILED:", file=sys.stderr) print("\n".join(failures), file=sys.stderr) return 1 - print(f"ci-graph self-test: {len(SELF_TEST)} graphs judged") + print(f"ci-graph self-test: {len(SELF_TEST)} graphs, " + f"{len(SUITE_SELF_TEST)} suite lists and {len(FILE_SELF_TEST)} files judged") return 0 def main(): if "--self-test" in sys.argv[1:]: return self_test() - jobs, needs = read_graph(open(WORKFLOW, encoding="utf8").read()) + text = read_workflow(WORKFLOW) + jobs, needs = read_graph(text) if len(jobs) < 2: print(f"error: {WORKFLOW}: found {len(jobs)} jobs; the parse is wrong", file=sys.stderr) return 2 - complaints = verdict(jobs, needs) + suites = list_suites() + if not suites: + print("error: checkers.sh --list named no suites; the listing is wrong", + file=sys.stderr) + return 2 + complaints = verdict(jobs, needs) + suite_verdict(suites, text) if complaints: for c in complaints: print(f"ci-graph: {c}", file=sys.stderr) return 1 - print(f"ci-graph: {DEPLOY} needs all {len(jobs) - 1} other jobs") + print(f"ci-graph: {DEPLOY} needs all {len(jobs) - 1} other jobs; " + f"all {len(suites)} checkers.sh suites run in CI") return 0 diff --git a/scripts/checkers-test.sh b/scripts/checkers-test.sh index 564ccacf..5b43a273 100755 --- a/scripts/checkers-test.sh +++ b/scripts/checkers-test.sh @@ -137,5 +137,47 @@ listed="$(tr '\n' ' ' < "$work/out")" && ok "an X-test.sh with no X beside it is not a suite" \ || bad "an X-test.sh with no X beside it is not a suite" "got [$listed]" +# --------------------------------------------------------------------------- +# 8. --would-run reports what the rest of the arguments run, and runs nothing. +# check-ci-graph.py asks it, so each argument rule below is answered HERE. +d="$(sandbox wouldrun)" +for n in alpha beta; do + suite "$d" "$n" < "$work/out" 2>&1 +got="$(tr '\n' ' ' < "$work/out")" +[ "$got" = "would-run: alpha would-run: beta " ] && [ ! -e "$work/ran-alpha" ] \ + && ok "--would-run with no names reports every suite and runs none" \ + || bad "--would-run with no names reports every suite and runs none" "got [$got]" +"$d/checkers.sh" --would-run --self-test alpha > "$work/out" 2>&1 +got="$(tr '\n' ' ' < "$work/out")" +[ "$got" = "missing: checkers " ] \ + && ok "--would-run --self-test reports checkers and discards the names" \ + || bad "--would-run --self-test reports checkers and discards the names" "got [$got]" +"$d/checkers.sh" --would-run --list alpha > "$work/out" 2>&1 +grep -q '^would-run:' "$work/out" \ + && bad "--would-run --list reports nothing as run" "$(tr '\n' ' ' < "$work/out")" \ + || ok "--would-run --list reports nothing as run" +expect "--would-run anywhere but first is a usage error" 4 "$d/checkers.sh" alpha --would-run +[ ! -e "$work/ran-alpha" ] && ok "and it runs nothing" \ + || bad "and it runs nothing" "alpha ran" + +# --------------------------------------------------------------------------- +# 9. --ci runs that job's row of the CI table and nothing else. The +# sandbox has none of the real suites, so a row reads back as `missing:`. +"$d/checkers.sh" --would-run --ci js > "$work/out" 2>&1 +got="$(tr '\n' ' ' < "$work/out")" +[ "$got" = "missing: node-test " ] && ok "--ci reads that job's row of the table" \ + || bad "--ci reads that job's row of the table" "got [$got]" +expect "--ci with a job the table does not have is exit 4" 4 "$d/checkers.sh" --would-run --ci nope +expect "--ci with nothing after it is exit 4" 4 "$d/checkers.sh" --would-run --ci +expect "--ci beside a suite name is exit 4, not a union" 4 \ + "$d/checkers.sh" --would-run --ci js alpha +expect "--ci beside --self-test is exit 4" 4 "$d/checkers.sh" --would-run --self-test --ci js + printf '\ncheckers-test: %d passed, %d failed\n' "$pass" "$fail" [ "$fail" -eq 0 ] diff --git a/scripts/checkers.sh b/scripts/checkers.sh index e98e9e1e..6429d427 100755 --- a/scripts/checkers.sh +++ b/scripts/checkers.sh @@ -18,7 +18,7 @@ HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" usage() { cat >&2 <<'EOF' -usage: checkers.sh [--list] [--self-test] [name ...] +usage: checkers.sh [--list] [--self-test] [--ci ] [name ...] name a suite to run, spelled without the -test.sh suffix (e.g. `pr-reviews` for scripts/pr-reviews-test.sh). @@ -26,6 +26,13 @@ usage: checkers.sh [--list] [--self-test] [name ...] --list print the suite names and exit --self-test run scripts/checkers-test.sh, which injects the faults this runner exists to catch + --ci run the suites the CI table gives that job of + .github/workflows/ci.yml, and nothing else. Each CI job runs + exactly this, so the table is the one list of what CI runs + --would-run as the FIRST argument only: run nothing, and print + `would-run: ` (or `missing: `) for each suite + the rest of the arguments would run. check-ci-graph.py + asks this, so the argument rules have one home exit 0 every suite passed exit 1 a suite failed, or a shebang is not `#!/usr/bin/env bash` @@ -88,13 +95,27 @@ run_suite() { env "${env_args[@]}" "$BASH" "$file" } +# The CI table: the suites each job of .github/workflows/ci.yml runs, by that +# job's `checkers.sh --ci ` step. check-ci-graph.py holds it to --list. +ci_suites() { + case "$1" in + build) echo assemble-stamp ;; + js) echo node-test ;; + kb) echo kb checkers pr-watch release-preflight pr-reviews fmt-workspaces bd-audit-ids ;; + *) return 1 ;; + esac +} + main() { - local self_test=0 + local self_test=0 would_run=0 ci_job="" local -a names=() + # Only as the FIRST argument, so a CI step spelling it anywhere is an error. + if [ "${1:-}" = --would-run ]; then would_run=1; shift; fi while [ $# -gt 0 ]; do case "$1" in --list) suites; return 0 ;; --self-test) self_test=1 ;; + --ci) ci_job="${2:-}"; [ -n "$ci_job" ] || { usage; return 4; }; shift ;; -h|--help) usage; return 4 ;; -*) printf 'checkers: unknown option %s\n' "$1" >&2; usage; return 4 ;; *) names+=("$1") ;; @@ -102,17 +123,33 @@ main() { shift done - printf 'checkers: bash %s at %s\n' "$BASH_VERSION" "$BASH" - local rc=0 - check_shebangs || rc=1 - - if [ "$self_test" = 1 ]; then + if [ -n "$ci_job" ]; then + if [ "$self_test" = 1 ] || [ ${#names[@]} -gt 0 ]; then + echo 'checkers: --ci takes a job and nothing else' >&2; return 4 + fi + local table + table="$(ci_suites "$ci_job")" || { + printf 'checkers: no CI job %s in the table\n' "$ci_job" >&2; return 4; } + read -r -a names <<< "$table" + elif [ "$self_test" = 1 ]; then names=(checkers) elif [ ${#names[@]} -eq 0 ]; then while IFS= read -r line; do names+=("$line"); done < <(suites) fi local n file + if [ "$would_run" = 1 ]; then + for n in ${names[@]+"${names[@]}"}; do + if [ -f "$HERE/$n-test.sh" ]; then printf 'would-run: %s\n' "$n" + else printf 'missing: %s\n' "$n"; fi + done + return 0 + fi + + printf 'checkers: bash %s at %s\n' "$BASH_VERSION" "$BASH" + local rc=0 + check_shebangs || rc=1 + for n in ${names[@]+"${names[@]}"}; do file="$HERE/$n-test.sh" if [ ! -f "$file" ]; then