From bb6dfe45119f1d8b47a26f5acb60c1015d00c560 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Mon, 14 Sep 2026 13:22:23 -0600 Subject: [PATCH 1/3] test: arm orphan-scan in the matrix runner (#983, #1015) The gate answers "has this run a check the ledger has never seen". Nothing answered the other half -- "does the ledger name a check that no longer exists" -- and two rows naming deleted checks sat in the committed ledger from #917 until `orphan-scan` was written for exactly that defect and HAD NO CALLER IN THE TREE. Its only invocations were the selftest arms testing the tool itself: tested, and unable to fire on anybody's change. ONE LOG PER CALL, NOT ALL OF THEM. `_by_run` returns one entry per LOG, so `len(runs) > 1` is true whenever more than one file is passed even when every log came from the same matrix run. A call shaped like the gate's `$_led_logs` is refused by the COUNT, before the before-log/after-log union the error names. So the runner loops and reduces the statuses. --orphans-only, BECAUSE A SKIP IS NOT A DELETION. By default rc=1 also covers a part that skipped, and that default is correct on the merits: `not checked` is silence by construction and OUT of scope, `unprunable` is a gap in a part the run CLAIMED and in it. Three arms in part 410 pin that and their names argue for it. So this does not change what rc=1 means. I was going to -- `return 1 if orphans else 0` -- and reading 410 stopped it: that deletes a deliberate contract to make the wiring convenient, which is weakening a guard to get green. The flag changes the QUESTION instead. With it rc=1 is orphans and nothing else; without it the old question and the old answer. rc=1 never acquires a second meaning, and `unprunable` is printed either way, so narrowing what the gate refuses on cannot hide it. MEASURED BEFORE WIRING, and the clean run proved less than it looked: only part 340 of 46 has a real check_skip; 320/400/410/480 match on comments, a grep-based sweep and printf fixtures -- source-derived, box-independent none of CI's 9 skipped suites has a ledger row, so none can put a part in scope five majors on one box: orphans=0 unprunable=0 rc=0 BUT part 340 recorded 108 PASS and 0 SKIP, so the skip path never fired The skipped-part case was therefore forced synthetically rather than waited for, and the tool comes out well: the siblings are classified `unprunable`, not orphans -- "absence there is not removal". REMOVAL PROOF, both directions: a planted row naming a deleted check rc=1, orphans=1 refused a part that skipped rc=0 not refused unprunable still reported harness_selftest 962 passed + 0 failed + 0 unrunnable + 0 skipped shellcheck -S error test/run_all_versions.sh clean Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NhwXKAgSmYDUjteWkfajHK --- CHANGELOG.md | 31 ++++++++++++++++ test/pgc_ledger.py | 20 +++++++++++ test/run_all_versions.sh | 36 +++++++++++++++++++ .../410-a-check-must-have-been-red.sh | 30 ++++++++++++++++ 4 files changed, 117 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ad0b461..29c0eafd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,37 @@ true until the next version shipped. ### Added +- `orphan-scan` is armed in the matrix runner, so a ledger row naming a deleted + check is refused rather than reported (#983, #1015). + + The gate answers "has this run a check the ledger has never seen". Nothing answered + the other half, "does the ledger name a check that no longer exists" -- and two rows + naming deleted checks sat in the committed ledger from #917 until #983 found them + while doing something else. The census counted both. `orphan-scan` was written for + exactly that and had NO CALLER IN THE TREE: tested, and unable to fire on anybody's + change. + + **One log per call, not all of them.** `_by_run` returns one entry per LOG, so + `len(runs) > 1` is true whenever more than one file is passed even when they came + from one matrix run. A call shaped like the gate's `$_led_logs` is refused by the + COUNT, before the before-log/after-log union the message names. So the runner loops. + + **`--orphans-only`, because a skip is not a deletion.** By default `rc=1` also covers + a part that skipped, and that is right: `not checked` is silence by construction and + out of scope, while `unprunable` is a gap in a part the run claimed and in it. But a + skip is box-dependent -- part 340 skips only where there is no non-root user to read + as -- and failing a matrix for that is a gate somebody turns off. The flag changes the + QUESTION rather than the severity of an answer, so `rc=1` never acquires a second + meaning, and the skipped part is still printed. + + Measured before wiring: only part 340 of the 46 has a real `check_skip` call; 320, + 400, 410 and 480 match on comments, a grep-based sweep and `printf` fixtures. Five + majors on one box gave `orphans=0, unprunable=0` -- but with `108 PASS and 0 SKIP`, + so the skip path never fired and that run tested nothing about skips. The skipped-part + case was forced synthetically instead, and the tool classifies the siblings + `unprunable` rather than orphans. + +- The piped-loop sweep reported a clean tree without reading one (#1033). - Userinfo in an object-store ENDPOINT was accepted, and the diagnostic told the operator to allow-list it (#995). diff --git a/test/pgc_ledger.py b/test/pgc_ledger.py index a1d66925..ab50f1ea 100755 --- a/test/pgc_ledger.py +++ b/test/pgc_ledger.py @@ -721,6 +721,22 @@ def cmd_orphan_scan(args): f"ledger rows -- every row must land in exactly one of the four") if not args.prune: + # THE FLAG CHANGES THE QUESTION, NOT THE SEVERITY OF AN ANSWER (#983, #1015). + # The default question is "is anything outstanding in the parts this run + # claimed", and a skipped part IS outstanding -- the run held those rows and + # failed to speak for them, which is why it differs from `not checked`: that + # one is silence by construction and out of scope, this one is a gap in a part + # the run claimed. So rc=1 covering both is right by default. + # + # A GATE NEEDS THE NARROWER QUESTION. `run_all_versions.sh` must refuse a row + # whose check no longer exists and must NOT refuse a box where a part skipped, + # since a skip is box-dependent and is not a deletion. Rather than give rc=1 a + # second meaning under a flag -- the same defect one level up -- this asks a + # different question, so rc=1 always means "the thing this invocation asked + # about was found". `unprunable` is still PRINTED above either way, so + # narrowing the gate's question cannot silence the report. + if getattr(args, "orphans_only", False): + return 1 if orphans else 0 return 1 if (orphans or unprunable) else 0 if unprunable: @@ -1136,6 +1152,10 @@ def main(argv=None): o = sub.add_parser("orphan-scan", help="refuse a ledger row no record in its own part matches") o.add_argument("--ledger", required=True) + o.add_argument("--orphans-only", action="store_true", + help="ask only whether a ledger row's check has been DELETED, so a " + "part that merely skipped does not make the exit status non-zero; " + "the skipped part is still reported") o.add_argument("--prune", action="store_true", help="remove orphan rows that carry no history; refuse the whole " "prune if any of them does") diff --git a/test/run_all_versions.sh b/test/run_all_versions.sh index 3f460709..8b7083fe 100755 --- a/test/run_all_versions.sh +++ b/test/run_all_versions.sh @@ -1451,6 +1451,42 @@ pgc_tally_suite() { # pgc_tally_suite NAME VERDICT LOGFILE esac fi + # THE OTHER HALF OF THE COMPARISON (#983, #1015). The gate answers "has this run a + # check the ledger has never seen". It cannot answer "does the ledger name a check + # that no longer exists", and nothing did: two rows naming deleted checks sat in the + # committed ledger from #917 until #983 found them by accident, and the census + # counted both. `orphan-scan` was written for exactly that and had no caller in the + # tree at all -- tested, and unable to fire on anybody's change. + # + # ONE LOG PER CALL, NOT ALL OF THEM. `_by_run` returns one entry per LOG, so + # `len(runs) > 1` is true whenever more than one file is passed even when they came + # from the same matrix run, and `cmd_orphan_scan` refuses outright. Shaping this call + # like the gate's `$_led_logs` is refused by the COUNT, before the union argument the + # message names. So it loops. + # + # --orphans-only, BECAUSE A SKIP IS NOT A DELETION. Without it rc=1 also covers a + # part that skipped, which is box-dependent -- part 340 skips only where there is no + # non-root user to read as -- and failing a matrix for that would be a gate somebody + # turns off. The skipped part is still PRINTED by the tool, so narrowing what the + # gate refuses on does not hide it. + if [ -n "${_led_logs:-}" ] && [ -f "$builddir/test/check_ledger.tsv" ]; then + _orph_fail=0 + # shellcheck disable=SC2086 + for _orph_log in $_led_logs; do + python3 "$builddir/test/pgc_ledger.py" orphan-scan --orphans-only \ + --ledger "$builddir/test/check_ledger.tsv" "$_orph_log" || _orph_fail=$? + done + case "$_orph_fail" in + 0) ;; + 1) echo " PG$major: the ledger names a check that no longer exists, which is" + echo " not a pass. Regenerate the ledger, or rename the row if the check" + echo " moved rather than went." + verfail=1 ;; + *) echo " PG$major could not run the orphan scan at all, which is not a pass." + verfail=1 ;; + esac + fi + # How many of the suites counted as having RUN actually accounted for their # checks (#916). Counting a suite that never accounted among the suites that ran # is the overcount #447 added this line to stop, one level further down, and diff --git a/test/selftest/410-a-check-must-have-been-red.sh b/test/selftest/410-a-check-must-have-been-red.sh index 46bf621d..e7015453 100644 --- a/test/selftest/410-a-check-must-have-been-red.sh +++ b/test/selftest/410-a-check-must-have-been-red.sh @@ -527,6 +527,36 @@ check "control: a prune that leaves nothing outstanding returns 0" \ check "control: and that one did prune, so 0 is not a refusal in disguise" \ "$(wc -l < "$_lw/rc.tsv" | tr -d ' ')" "1" +# ---- --orphans-only: a QUESTION, not a change of severity (#983, #1015) ------ +# +# The default question is "is anything outstanding in the parts this run claimed", +# and a skipped part IS outstanding: the run held those rows and failed to speak for +# them. That is why rc=1 covers it and why the three arms above are right on the +# merits rather than only by precedent -- `not checked` is silence by construction +# and OUT of scope, `unprunable` is a gap in a part the run claimed and IN it. +# +# A GATE NEEDS THE NARROWER QUESTION. `run_all_versions.sh` must refuse a ledger row +# whose check no longer exists (#983) and must NOT refuse a box where a part skipped, +# because a skip is box-dependent and not a deletion. Those share rc=1 by design, so +# the flag changes WHAT IS ASKED rather than what a code means: with it, rc=1 is +# orphans and nothing else. rc=1 never acquires a second meaning. +_oo_pre="$(wc -l < "$_lw/rc.tsv" | tr -d ' ')" +check "premise: the fixture is back to a state with rows to speak about" \ + "$([ "$_oo_pre" -ge 1 ] && echo yes || echo no)" "yes" +: > "$_lw/oo.tsv" +_led_run merge --ledger "$_lw/oo.tsv" --date 2026-09-01 "$_lw/sk_full.log" >/dev/null +check "premise: the skipped-part fixture still yields a finding by default" \ + "$(_led_rc orphan-scan --ledger "$_lw/oo.tsv" "$_lw/sk_skipped.log")" "1" +check "a skipped part is NOT an orphan, so --orphans-only returns 0" \ + "$(_led_rc orphan-scan --orphans-only --ledger "$_lw/oo.tsv" "$_lw/sk_skipped.log")" "0" +check "and it still REPORTS the skipped part, so the gate cannot silence it" \ + "$(_led_run orphan-scan --orphans-only --ledger "$_lw/oo.tsv" "$_lw/sk_skipped.log" \ + | grep -c 'unprunable:')" "1" +check "control: a REAL orphan still returns 1 under --orphans-only" \ + "$(_led_rc orphan-scan --orphans-only --ledger "$_lw/orph.tsv" "$_lw/o_after.log")" "1" +check "control: and a clean run still returns 0 under it" \ + "$(_led_rc orphan-scan --orphans-only --ledger "$_lw/orph.tsv" "$_lw/o_before.log")" "0" + # WHY THIS REPORTS AND DOES NOT GATE -- pinned to the PRECONDITION, not to one # instance of it. # From d7ad2e8b7dad2e2ee20ffa5fcc498fa1c753fb5a Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Mon, 14 Sep 2026 15:19:34 -0600 Subject: [PATCH 2/3] test: the six new 410 rows claim every major they were observed on (#983, #1015) CI refused this branch for exactly the reason I had spent the hour posting recipes about on four other PRs: not in the ledger: harness_selftest 410-a-check-must-have-been-red a skipped part is NOT an orphan, so --orphans-only returns 0 (on major 17) ... and five more The six arms this PR adds are checks the committed ledger has never seen, and the gate refuses a check it has never seen on the major being run. I reviewed that defect on #1039, #1063, #1065 and #1068 and then shipped it myself. TWO THINGS WORTH RECORDING FROM THAT. `harness_selftest.sh` GREEN DOES NOT COVER THE LEDGER GATE. The gate runs in run_all_versions.sh, not in the suite, so a local suite run passes while the matrix refuses. Every local verification I did on this branch was of the suite. AND THE WIRING WORKS, which the same failing log shows: orphan scan: parts in the run=1, rows in those parts=0, orphans=0, unprunable=0, not checked=1217 That is this PR's own change running in CI for the first time. Fixed the way 3a640b0 established: harness_selftest run on all five majors and all five logs merged, so the majors field is OBSERVED rather than written. PG15..PG19 rc=0 960 passed + 0 failed + 0 unrunnable + 0 skipped, each merge: rows=1223 | runs=5, distinct checks this merge=960 majors: uniform, all 1223 rows carry 15;16;17;18;19 rows 1223 = sum of buckets printed 1223 ledger 1217 -> 1223, purely additive: 0 removed, 6 added CENSUS RE-DERIVED BY COUNTING, not by adding 6 to 1209: awk -F'\t' '$5=="never"' test/check_ledger.tsv | wc -l -> 1215 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NhwXKAgSmYDUjteWkfajHK --- test/check_ledger.tsv | 6 ++++++ test/check_ledger_budget.txt | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/test/check_ledger.tsv b/test/check_ledger.tsv index f7515224..e40992b4 100644 --- a/test/check_ledger.tsv +++ b/test/check_ledger.tsv @@ -908,6 +908,7 @@ harness_selftest 410-a-check-must-have-been-red a run that emits every row in it harness_selftest 410-a-check-must-have-been-red a run whose checks are all ledgered passes the gate 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red a second major is ADDED to the set, sorted, not written over the first 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red a second mutation ACCUMULATES rather than replacing the first 15;16;17;18;19 never - +harness_selftest 410-a-check-must-have-been-red a skipped part is NOT an orphan, so --orphans-only returns 0 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red a stale prior is named WITH its distance from HEAD 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red a verdict pgc_record cannot emit is an integrity failure 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red an absolute budget path resolves against git rather than shrugging 15;16;17;18;19 never - @@ -942,6 +943,7 @@ harness_selftest 410-a-check-must-have-been-red and it says so rather than decli harness_selftest 410-a-check-must-have-been-red and it says the change introduces the file rather than raising anything 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red and it says the ref does not resolve, rather than claiming the file is new 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red and it says why, rather than falling back to something weaker 15;16;17;18;19 never - +harness_selftest 410-a-check-must-have-been-red and it still REPORTS the skipped part, so the gate cannot silence it 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red and never says a change introduces a file at a ref that is not there 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red and none of them ends in a tab 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red and not against one that stayed green 15;16;17;18;19 never - @@ -974,10 +976,12 @@ harness_selftest 410-a-check-must-have-been-red auto with no base ref and no ups harness_selftest 410-a-check-must-have-been-red both failure arms fail the major 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red but that suite is counted as not covered, which is the debt 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red but the gate says so, so the skip is visible rather than silent 15;16;17;18;19 never - +harness_selftest 410-a-check-must-have-been-red control: a REAL orphan still returns 1 under --orphans-only 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red control: a check the ledger claims for 15 that a PG15 run did not emit IS an orphan 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red control: a prune that leaves nothing outstanding returns 0 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red control: a well-formed log still merges 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red control: an all-PASS log still merges with no flag at all 15;16;17;18;19 never - +harness_selftest 410-a-check-must-have-been-red control: and a clean run still returns 0 under it 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red control: and that one did prune, so 0 is not a refusal in disguise 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red control: and the ledger really is shorter afterwards 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red control: on a covered major a new check is named and refused 15;16;17;18;19 never - @@ -1007,6 +1011,7 @@ harness_selftest 410-a-check-must-have-been-red premise: that part has rows the harness_selftest 410-a-check-must-have-been-red premise: the budget is a tracked file too 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red premise: the budget was restored byte-exact 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red premise: the check has history before the rename 15;16;17;18;19 never - +harness_selftest 410-a-check-must-have-been-red premise: the fixture is back to a state with rows to speak about 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red premise: the fixture ledger holds two rows 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red premise: the fixture log names checks the real ledger already knows 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red premise: the gate's status block was found in the runner 15;16;17;18;19 never - @@ -1024,6 +1029,7 @@ harness_selftest 410-a-check-must-have-been-red premise: the scratch prior reall harness_selftest 410-a-check-must-have-been-red premise: the scratch repo has a committed ceiling and no upstream 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red premise: the scratch repo has a prior ceiling committed 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red premise: the skip-loop sweep is present, so its counts can be read 15;16;17;18;19 never - +harness_selftest 410-a-check-must-have-been-red premise: the skipped-part fixture still yields a finding by default 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red premise: the workflow file is where this part thinks it is 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red raising the ceiling above its committed value is refused 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red raising the ceiling in the tracked file is refused 15;16;17;18;19 never - diff --git a/test/check_ledger_budget.txt b/test/check_ledger_budget.txt index 1a00d366..5611d5cb 100644 --- a/test/check_ledger_budget.txt +++ b/test/check_ledger_budget.txt @@ -54,4 +54,4 @@ suites_not_covered 249 # the number of attacked checks -- and it overcounts from the first moment this # ledger does the job it exists for. The gate prints both quantities side by side # (`rows=N | never observed red=M`) because they are different questions. -checks_never_observed_red 1209 +checks_never_observed_red 1215 From 57f5c6771beb4ff0428a5333ea09b49ef261f513 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Mon, 14 Sep 2026 15:52:14 -0600 Subject: [PATCH 3/3] test: reduce the orphan statuses with two flags, and prove the WIRING (#983, #1015) @OffgridwithJD found two defects in the runner wiring, neither of which any arm in this PR could see, because every proof I had written was of the TOOL. THE REDUCTION OVERWROTE. `|| _orph_fail=$?` across N logs tells the operator about whichever log failed LAST. Measured with a stub, both orders: rcs=[1 2] reports "could not run the orphan scan" and hides a real orphan rcs=[2 1] reports "names a check that no longer exists" and hides the broken tool The verdict was right both times -- verfail=1 either way -- and the DIAGNOSIS was wrong half the time. That is the defect the gate's own comment thirty lines above says it fixed, in its own words about sending the reader at the wrong repair, which is how I know a comment does not transfer to the code beneath it. AND THE OBVIOUS REPAIR IS WORSE THAN THE BUG, which they measured before suggesting it: `|| { [ "$?" -gt "$_orph_fail" ] && _orph_fail=$?; }` yields 0 for EVERY input, because `$?` inside the braces is the `[` test. It turns an order-dependent misdiagnosis into a silent pass. Two independent conditions need two independent flags, not a max: keeping the worst still hides a real orphan behind a tool failure. Both messages can now print, and the second says that both can be true. THE WIRING IS NOW PROVED, not just the tool. The arms EVAL THE RUNNER'S OWN TEXT, the way link 5 of part 330 does, because an arm that re-derives the rule tests the world instead of the code: premise: the runner's orphan reduction was extracted, not an empty range both conditions survive the reduction whatever order they arrive in (1 2) both conditions survive the reduction whatever order they arrive in (2 1) control: all-clean logs leave both flags down control: one orphan among clean logs raises only the orphan flag REMOVAL PROOF ON THE WIRING: collapsing the two flags back into one reddens both order arms with `got [10] want [11]` while the control stays green. harness_selftest 965 passed + 0 failed + 0 unrunnable + 0 skipped shellcheck -S error test/run_all_versions.sh clean PG15..PG19 rc=0, 965 each; merge rows=1228, majors uniform at 15;16;17;18;19 census re-derived by counting: 1220 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NhwXKAgSmYDUjteWkfajHK --- test/check_ledger.tsv | 5 ++ test/check_ledger_budget.txt | 2 +- test/run_all_versions.sh | 49 ++++++++++++++----- .../410-a-check-must-have-been-red.sh | 37 ++++++++++++++ 4 files changed, 81 insertions(+), 12 deletions(-) diff --git a/test/check_ledger.tsv b/test/check_ledger.tsv index e40992b4..39c2fd6b 100644 --- a/test/check_ledger.tsv +++ b/test/check_ledger.tsv @@ -973,6 +973,8 @@ harness_selftest 410-a-check-must-have-been-red and the summary keeps the two ap harness_selftest 410-a-check-must-have-been-red auto refuses when GITHUB_BASE_REF names a ref that is not here 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red auto uses the base ref when it resolves, and names it 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red auto with no base ref and no upstream is an integrity failure 15;16;17;18;19 never - +harness_selftest 410-a-check-must-have-been-red both conditions survive the reduction whatever order they arrive in (1 2) 15;16;17;18;19 never - +harness_selftest 410-a-check-must-have-been-red both conditions survive the reduction whatever order they arrive in (2 1) 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red both failure arms fail the major 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red but that suite is counted as not covered, which is the debt 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red but the gate says so, so the skip is visible rather than silent 15;16;17;18;19 never - @@ -980,11 +982,13 @@ harness_selftest 410-a-check-must-have-been-red control: a REAL orphan still ret harness_selftest 410-a-check-must-have-been-red control: a check the ledger claims for 15 that a PG15 run did not emit IS an orphan 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red control: a prune that leaves nothing outstanding returns 0 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red control: a well-formed log still merges 15;16;17;18;19 never - +harness_selftest 410-a-check-must-have-been-red control: all-clean logs leave both flags down 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red control: an all-PASS log still merges with no flag at all 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red control: and a clean run still returns 0 under it 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red control: and that one did prune, so 0 is not a refusal in disguise 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red control: and the ledger really is shorter afterwards 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red control: on a covered major a new check is named and refused 15;16;17;18;19 never - +harness_selftest 410-a-check-must-have-been-red control: one orphan among clean logs raises only the orphan flag 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red control: the row in the part the run never mentioned SURVIVES the prune 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red control: the same log merges with a different reason, so the refusal above is --mutation-across-two-checks and not the log 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red control: the same row with a real major is read without complaint 15;16;17;18;19 never - @@ -1025,6 +1029,7 @@ harness_selftest 410-a-check-must-have-been-red premise: the orphan about to be harness_selftest 410-a-check-must-have-been-red premise: the orphan now carries a date and a mutation 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red premise: the raised copy really does carry a higher ceiling 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red premise: the real budget is inside a git repository 15;16;17;18;19 never - +harness_selftest 410-a-check-must-have-been-red premise: the runner's orphan reduction was extracted, not an empty range 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red premise: the scratch prior really is three commits behind 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red premise: the scratch repo has a committed ceiling and no upstream 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red premise: the scratch repo has a prior ceiling committed 15;16;17;18;19 never - diff --git a/test/check_ledger_budget.txt b/test/check_ledger_budget.txt index 5611d5cb..bdfcff19 100644 --- a/test/check_ledger_budget.txt +++ b/test/check_ledger_budget.txt @@ -54,4 +54,4 @@ suites_not_covered 249 # the number of attacked checks -- and it overcounts from the first moment this # ledger does the job it exists for. The gate prints both quantities side by side # (`rows=N | never observed red=M`) because they are different questions. -checks_never_observed_red 1215 +checks_never_observed_red 1220 diff --git a/test/run_all_versions.sh b/test/run_all_versions.sh index 8b7083fe..3f297e3c 100755 --- a/test/run_all_versions.sh +++ b/test/run_all_versions.sh @@ -1470,21 +1470,48 @@ pgc_tally_suite() { # pgc_tally_suite NAME VERDICT LOGFILE # turns off. The skipped part is still PRINTED by the tool, so narrowing what the # gate refuses on does not hide it. if [ -n "${_led_logs:-}" ] && [ -f "$builddir/test/check_ledger.tsv" ]; then - _orph_fail=0 + # TWO FLAGS, NOT A COMBINED STATUS. The loop runs once per log, so the + # statuses have to be reduced, and every single-variable reduction loses + # one of the two answers: + # + # `|| _orph_fail=$?` OVERWRITES, so across 246 logs the operator is told + # about whichever failed LAST. Measured with a stub: + # orphan-then-toolfail reports "could not run" and + # hides a real orphan; the reverse hides the broken + # tool. The verdict is right either way and the + # DIAGNOSIS is wrong half the time, which is the + # defect the gate's own comment above says it fixed. + # keeping the MAX still hides a real orphan behind a tool failure. + # `|| { [ "$?" -gt ... ; }` is worse again: `$?` inside the braces is the + # `[` test, so it yields 0 for every input and reports + # CLEAN. Measured, both orders. + # + # Reported by @OffgridwithJD, who also measured that third one before + # suggesting it. Two independent conditions need two independent flags. + _orph_orphan=0 + _orph_broken=0 # shellcheck disable=SC2086 for _orph_log in $_led_logs; do python3 "$builddir/test/pgc_ledger.py" orphan-scan --orphans-only \ - --ledger "$builddir/test/check_ledger.tsv" "$_orph_log" || _orph_fail=$? + --ledger "$builddir/test/check_ledger.tsv" "$_orph_log" + _orph_rc=$? + case "$_orph_rc" in + 0) ;; + 1) _orph_orphan=1 ;; + *) _orph_broken=1 ;; + esac done - case "$_orph_fail" in - 0) ;; - 1) echo " PG$major: the ledger names a check that no longer exists, which is" - echo " not a pass. Regenerate the ledger, or rename the row if the check" - echo " moved rather than went." - verfail=1 ;; - *) echo " PG$major could not run the orphan scan at all, which is not a pass." - verfail=1 ;; - esac + if [ "$_orph_orphan" = 1 ]; then + echo " PG$major: the ledger names a check that no longer exists, which is" + echo " not a pass. Regenerate the ledger, or rename the row if the check" + echo " moved rather than went." + verfail=1 + fi + if [ "$_orph_broken" = 1 ]; then + echo " PG$major could not run the orphan scan on at least one log, which is" + echo " not a pass. That is separate from the line above, and both can be true." + verfail=1 + fi fi # How many of the suites counted as having RUN actually accounted for their diff --git a/test/selftest/410-a-check-must-have-been-red.sh b/test/selftest/410-a-check-must-have-been-red.sh index e7015453..1c1e1ad5 100644 --- a/test/selftest/410-a-check-must-have-been-red.sh +++ b/test/selftest/410-a-check-must-have-been-red.sh @@ -557,6 +557,43 @@ check "control: a REAL orphan still returns 1 under --orphans-only" \ check "control: and a clean run still returns 0 under it" \ "$(_led_rc orphan-scan --orphans-only --ledger "$_lw/orph.tsv" "$_lw/o_before.log")" "0" +# ---- the WIRING, not the tool: the runner's reduction over N logs ------------ +# +# The arms above prove the TOOL. They say nothing about run_all_versions.sh, and +# the first version of that wiring was wrong in a way none of them could see: it +# reduced N per-log statuses with `|| _orph_fail=$?`, which OVERWRITES, so the +# operator was told about whichever log failed LAST. Measured with a stub, both +# orders: orphan-then-toolfail reported "could not run the orphan scan" and hid a +# real orphan; the reverse hid the broken tool. The verdict was right both times +# and the DIAGNOSIS was wrong half the time -- the same defect the gate's own +# comment says it fixed, which is how I know a comment does not transfer. +# +# Caught by @OffgridwithJD, who also measured that the obvious repair is worse: +# `|| { [ "$?" -gt "$_orph_fail" ] && _orph_fail=$?; }` yields 0 for EVERY input, +# because `$?` inside the braces is the `[` test, so it reports CLEAN. +# +# EVALS THE RUNNER'S OWN TEXT, the way link 5 of part 330 does, because an arm +# that re-derives the rule tests the world instead of the code. +_orw="$PGC_TESTDIR/run_all_versions.sh" +_orw_txt="$(awk '/^\t\t\tcase "\$_orph_rc" in$/{f=1} f{print} f&&/^\t\t\tesac$/{exit}' "$_orw")" +check "premise: the runner's orphan reduction was extracted, not an empty range" \ + "$(printf '%s\n' "$_orw_txt" | grep -c '_orph_orphan=1')" "1" + +for _ord in "1 2" "2 1"; do + _orph_orphan=0; _orph_broken=0 + for _orph_rc in $_ord; do eval "$_orw_txt"; done + check "both conditions survive the reduction whatever order they arrive in ($_ord)" \ + "$_orph_orphan$_orph_broken" "11" +done +# CONTROL: it does not simply set both flags for everything. +_orph_orphan=0; _orph_broken=0 +for _orph_rc in 0 0 0; do eval "$_orw_txt"; done +check "control: all-clean logs leave both flags down" "$_orph_orphan$_orph_broken" "00" +_orph_orphan=0; _orph_broken=0 +for _orph_rc in 0 1 0; do eval "$_orw_txt"; done +check "control: one orphan among clean logs raises only the orphan flag" \ + "$_orph_orphan$_orph_broken" "10" + # WHY THIS REPORTS AND DOES NOT GATE -- pinned to the PRECONDITION, not to one # instance of it. #