tooling: check-ci-graph fails when a checkers.sh suite runs in no CI step
improvise-dak5: ci.yml names its checker suites by hand, so a new scripts/X-test.sh ran locally and never in CI. check-ci-graph.py now asks `checkers.sh --list` for the suites, so the naming rule keeps one home, and reads every checkers.sh invocation in ci.yml. A bare run covers every suite, --self-test covers `checkers`, and a YAML comment is never a run. A suite planted in the real tree fails the check, naming it, and adding it to the step clears it. Seven new self-test cases. Rivals: counting comments fails the comment case; answering "everything runs" fails five. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
This commit is contained in:
@@ -259,6 +259,17 @@ 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. It then
|
||||
reads every `checkers.sh` invocation in the workflow. A bare one covers every
|
||||
suite, and `--self-test` covers `checkers`. A YAML comment is skipped, because a
|
||||
bare mention there would otherwise read as "everything runs", which is the
|
||||
failure that looks like success.
|
||||
|
||||
### pr-watch and mergeability (improvise-edpl, improvise-zlj2)
|
||||
|
||||
`scripts/pr-watch` is what the orchestrator's merge procedure stands on, and
|
||||
|
||||
@@ -763,3 +763,34 @@ to spare a harness directory. `opfs.spec.js` instead asserts `/opfs/index.html`
|
||||
is served, naming the fix. Injected by running `web/assemble.sh harness/serve`
|
||||
alone: 4 chromium failures, each carrying that message, where it used to fail
|
||||
with no hint of the cause.
|
||||
|
||||
### 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".
|
||||
|
||||
+110
-4
@@ -6,10 +6,15 @@ 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).
|
||||
|
||||
`--self-test` checks the checker: each case is a graph this must refuse.
|
||||
"""
|
||||
|
||||
import re
|
||||
import subprocess
|
||||
import sys
|
||||
|
||||
WORKFLOW = ".github/workflows/ci.yml"
|
||||
@@ -60,6 +65,95 @@ def verdict(jobs, needs):
|
||||
return out
|
||||
|
||||
|
||||
_CHECKERS = re.compile(r"(?:^|[\s/])scripts/checkers\.sh\b(.*)$")
|
||||
ALL = object()
|
||||
|
||||
|
||||
def suites_run(text):
|
||||
"""The suite names CI runs, or ALL when some step runs checkers.sh bare.
|
||||
|
||||
A YAML comment is skipped: a bare mention there must not read as "every
|
||||
suite runs". A trailing backslash continues onto the next line.
|
||||
"""
|
||||
lines = text.split("\n")
|
||||
names, i = set(), 0
|
||||
while i < len(lines):
|
||||
raw = lines[i]
|
||||
i += 1
|
||||
if raw.lstrip().startswith("#"):
|
||||
continue
|
||||
m = _CHECKERS.search(raw)
|
||||
if not m:
|
||||
continue
|
||||
rest = m.group(1)
|
||||
while rest.rstrip().endswith("\\") and i < len(lines):
|
||||
rest = rest.rstrip()[:-1] + " " + lines[i]
|
||||
i += 1
|
||||
rest = re.split(r"&&|\|\||[;|#]", rest)[0]
|
||||
args = rest.split()
|
||||
if "--self-test" in args:
|
||||
names.add("checkers")
|
||||
picked = [a for a in args if not a.startswith("-")]
|
||||
if not picked and "--self-test" not in args:
|
||||
return ALL
|
||||
names.update(picked)
|
||||
return names
|
||||
|
||||
|
||||
def suite_verdict(suites, ran):
|
||||
"""The complaints about suites CI never runs, or runs by a wrong name."""
|
||||
if ran is ALL:
|
||||
return []
|
||||
out = [f"`{s}` is a checkers.sh suite that no CI step runs; add it to ci.yml"
|
||||
for s in sorted(set(suites) - ran)]
|
||||
out += [f"ci.yml runs checkers.sh `{s}`, which is not a suite"
|
||||
for s in sorted(ran - set(suites))]
|
||||
return out
|
||||
|
||||
|
||||
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()]
|
||||
|
||||
|
||||
# (name, suites checkers.sh knows, workflow text, the complaints it must draw).
|
||||
SUITE_SELF_TEST = [
|
||||
("a suite no step runs is named",
|
||||
["kb", "zz-new"], " run: ./scripts/checkers.sh kb\n",
|
||||
["`zz-new` is a checkers.sh suite that no CI step runs; add it to ci.yml"]),
|
||||
|
||||
("every suite named across steps is accepted",
|
||||
["kb", "pr-watch", "checkers"],
|
||||
" run: nix shell x --command ./scripts/checkers.sh kb\n"
|
||||
" run: ./scripts/checkers.sh --self-test\n"
|
||||
" run: ./scripts/checkers.sh pr-watch\n", []),
|
||||
|
||||
("a bare run covers every suite",
|
||||
["kb", "zz-new"], " run: ./scripts/checkers.sh\n", []),
|
||||
|
||||
("--self-test covers the checkers suite and nothing else",
|
||||
["checkers", "kb"], " run: ./scripts/checkers.sh --self-test\n",
|
||||
["`kb` is a checkers.sh suite that no CI step runs; add it to ci.yml"]),
|
||||
|
||||
("a bare mention in a comment does not count as running everything",
|
||||
["kb", "zz-new"],
|
||||
" # Every suite goes through scripts/checkers.sh\n"
|
||||
" run: ./scripts/checkers.sh kb\n",
|
||||
["`zz-new` is a checkers.sh suite that no CI step runs; add it to ci.yml"]),
|
||||
|
||||
("names continued onto the next line count, and are not a bare run",
|
||||
["kb", "pr-watch", "zz-new"],
|
||||
" run: ./scripts/checkers.sh \\\n kb pr-watch\n",
|
||||
["`zz-new` is a checkers.sh suite that no CI step runs; add it to ci.yml"]),
|
||||
|
||||
("a step naming a suite that does not exist is caught",
|
||||
["kb"], " run: ./scripts/checkers.sh kb ghost\n",
|
||||
["ci.yml runs checkers.sh `ghost`, which is not a suite"]),
|
||||
]
|
||||
|
||||
|
||||
# (name, workflow text, the complaints it must draw).
|
||||
SELF_TEST = [
|
||||
("a job nobody gates is named",
|
||||
@@ -95,28 +189,40 @@ 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, suites_run(text))
|
||||
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 and "
|
||||
f"{len(SUITE_SELF_TEST)} suite lists 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 = open(WORKFLOW, encoding="utf8").read()
|
||||
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, suites_run(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
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user