From 46d7b6d1361734ed5d25b5487376937b1856753f Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Fri, 11 Sep 2026 21:33:49 -0600 Subject: [PATCH 1/3] test: the pytest half of the skip-loop sweep that #998 did not ship (#994) #998 added `test/selftest/470` and no pytest twin. CONTEXT.md is explicit that a test living in one harness is not finished, and neither the author nor the reviewer caught it. This is that half. IT IS NOT A PORT, and the difference is why it is worth writing rather than an excuse for having written it. test/selftest/470 measures the CORPUS a loop that drifted from its arms test_skip_loop_arms.py measures the INSTRUMENT a classifier that stopped being able to tell A classifier that filed every site as `armless` would report zero mismatches, and 470's population premises would still pass on whatever loops remained. So this drives the same tool over PLANTED trees whose right answer is known, and asserts the classification itself. Six tests, seventeen checks. The clean site is compared rather than quietly filed as armless or interpolated; a renamed sibling is caught; the clean and drifted fixtures must DISAGREE, or neither of those arms is evidence; an armless branch and an interpolated one are each counted as themselves; and `compared + armless + interpolated == loops`, so no site falls out of the report. REMOVAL PROOFS, each mutation asserted to have applied before the run, because a clean pass reads identically whether the code is load-bearing or the edit never landed: stop reporting mismatches -> the two rename arms go red compare the interpolated site -> the interpolated and partition arms go red The second is the one worth keeping: removing the tool's REFUSAL to compare manufactures a false mismatch on a correct site, and a guard that invents a red is the guard people switch off. It drives .github/scripts/skip-loop-arms.py by subprocess and never the shell harness. That is a python tool rather than test/lib.sh, so this is a second measurement and not the first one wearing a python wrapper. Two guards in the tree caught my omissions while writing it, which is the argument for both of them: `test_harness_deps` named the file as undeclared the moment it appeared, and `test_docs_cover_the_corpus` refused it until TESTS.md named every test. Neither was told; both derived it from the corpus. Verified: 255 passed / 580 checks / 0 fail over the 14 files the CI job runs, in a venv pinned to requirements-test.txt with psycopg absent. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01EbyGSaU93XYQr8aH4NrUiw --- CHANGELOG.md | 21 ++++ test/pytest/TESTS.md | 58 ++++++++++ test/pytest/test_harness_deps.py | 7 ++ test/pytest/test_skip_loop_arms.py | 166 +++++++++++++++++++++++++++++ 4 files changed, 252 insertions(+) create mode 100644 test/pytest/test_skip_loop_arms.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 59f0ed23..657b72df 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,27 @@ true until the next version shipped. ### Added +- `test/selftest/470` now has the pytest half it shipped without (#994). + + #998 added the shell part and no pytest twin, against the owner's rule that a test + living in one harness is not finished. `test_skip_loop_arms.py` is that half, and it + is not a port: the shell part runs the sweep over the real corpus and asserts no + mismatch survives, which measures the TREE; this one drives the same tool over + planted trees whose right answer is known, which measures the INSTRUMENT. + + The distinction is the reason for writing it rather than a justification after the + fact. A classifier that filed every site as `armless` would report zero mismatches, + and the shell part's population premises would still pass on whatever loops remained. + Six tests, seventeen checks: the clean site is compared rather than quietly skipped, + a renamed sibling is caught, an armless branch and an interpolated one are each + counted as themselves, and `compared + armless + interpolated == loops` so nothing + falls out of the report. + + Two removal proofs, each mutation asserted to apply before the run. Stopping the + mismatch report reddens the rename arms; comparing the interpolated site literally + reddens the other two, and that is the one worth keeping, because removing the + tool's refusal to compare manufactures a FALSE mismatch on a correct site. + - The pytest harness reports its own check totals, and the record stream is reconciled against what arrived (#937, third phase). diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index 8104a84e..9906e3e6 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -72,6 +72,7 @@ behaviour, the source of that number is named. - [24. test_loop_coverage_premise.py: a loop that never ran asserted nothing](#24-test_loop_coverage_premisepy-a-loop-that-never-ran-asserted-nothing) - [25. test_join_runtime_filter.py: serial join runtime filter](#25-test_join_runtime_filterpy-serial-join-runtime-filter) - [26. test_check_records.py: every counted assertion is a record](#26-test_check_recordspy-every-counted-assertion-is-a-record) +- [27. test_skip_loop_arms.py: a skipped arm records under its own name](#27-test_skip_loop_armspy-a-skipped-arm-records-under-its-own-name) ## 1. How to read a test in here @@ -2534,3 +2535,60 @@ can fail. `tryfirst=True` on those hooks is load-bearing: a wrapper's post-`yiel code runs in the reverse of call order, so `trylast` made the injector run *before* the layer attached anything, the inner run passed, and the arm read exactly like a reconciliation that does not fire. + + +## 27. test_skip_loop_arms.py: a skipped arm records under its own name + +#994. A site skipped a whole block under **one** name that none of its arms had. When +the condition failed those arms produced no record at all, so a reader could not tell +which did not run and the ledger could not tell a skipped arm from a deleted one — a +skipped arm's row has no matching record, exactly as a removed check's would. + +The convention that fixes it was already in the tree: skip under each arm's own name, +in a loop over those names. The loop then **duplicates** the names, so a rename in the +sibling branch desynchronises the two silently, and the skip records under a name +nothing emits — the failure the fix exists to remove, reintroduced by an edit nobody +thought was risky. + +### Why this exists separately from `test/selftest/470` + +`470` shipped in #998 with no pytest half, against the owner's rule that a test in one +harness is not finished. This is that half, and it is **not a port**. + +| | measures | can it see | +| --- | --- | --- | +| `test/selftest/470` | the real corpus | a loop that drifted from its arms | +| `test_skip_loop_arms.py` | the instrument | a classifier that stopped being able to tell | + +A classifier that filed every site as `armless` would report zero mismatches, and +`470`'s population premises would still pass on whatever loops remained. So this file +drives the same tool over **planted trees whose right answer is known** and asserts the +classification itself. Two measurements of one property: the shell says the corpus is +clean, this says the thing that reads the corpus can tell the difference. + +It drives `.github/scripts/skip-loop-arms.py` by subprocess and never the shell +harness — a python tool rather than `test/lib.sh`, so this is a second measurement and +not the first one wearing a Python wrapper. + +| test | what it pins | +| --- | --- | +| `test_a_loop_naming_its_siblings_arms_is_compared_and_agrees` | the clean site is **compared**, not quietly filed as armless or interpolated | +| `test_a_rename_in_the_sibling_branch_is_caught` | the whole point: a drifted name is a mismatch | +| `test_the_clean_and_drifted_sites_differ_by_exactly_the_rename` | the pair must **disagree**, or neither arm above is evidence | +| `test_an_armless_branch_is_counted_as_armless_and_not_compared` | a site nobody compared and a site that agreed are not the same number | +| `test_an_interpolated_sibling_is_reported_rather_than_compared_wrongly` | it declines to compare a site where a literal comparison would be **false in both directions** | +| `test_every_loop_is_classified_into_exactly_one_category` | `compared + armless + interpolated == loops`, so nothing fell out of the report | + +### Removal proofs + +Each mutation was asserted to apply before the run, because a clean pass reads +identically whether the code is load-bearing or the edit never landed. + +| mutation | goes red | +| --- | --- | +| stop reporting mismatches | `..._rename_in_the_sibling_branch_is_caught`, `..._differ_by_exactly_the_rename` | +| compare the interpolated site literally | `..._interpolated_sibling_is_reported...`, `..._classified_into_exactly_one_category` | + +The second is the one worth keeping: removing the tool's *refusal* to compare produces +a **false** mismatch on a correct site, and a guard that manufactures a red is the guard +people switch off. diff --git a/test/pytest/test_harness_deps.py b/test/pytest/test_harness_deps.py index c7d4b517..7145db60 100644 --- a/test/pytest/test_harness_deps.py +++ b/test/pytest/test_harness_deps.py @@ -95,6 +95,13 @@ # agree. The fifth time this arm has decided a membership rather than been told # one. "test_check_records.py", + # The twin #998 did not ship. That PR added `test/selftest/470` and no pytest + # half, against the owner's rule that a test in one harness is not finished -- + # and the sixth catch of this arm was the file's arrival, not its absence. It + # drives .github/scripts/skip-loop-arms.py by subprocess, which is a python + # tool rather than the shell harness, so it needs neither a cluster nor the + # driver. + "test_skip_loop_arms.py", ] diff --git a/test/pytest/test_skip_loop_arms.py b/test/pytest/test_skip_loop_arms.py new file mode 100644 index 00000000..1203a737 --- /dev/null +++ b/test/pytest/test_skip_loop_arms.py @@ -0,0 +1,166 @@ +"""A skip loop must name exactly the arms its sibling branch would emit. + +#994. A site skipped a whole block under ONE name none of its arms had, so when the +condition failed those arms produced no record at all -- a reader could not tell +which did not run, and the ledger could not tell a skipped arm from a deleted one. +The convention that fixes it is to skip under each arm's own name, in a loop over +those names. The loop then DUPLICATES them, so a rename in the sibling branch +desynchronises the two silently. + +WHY THIS EXISTS SEPARATELY FROM `test/selftest/470`. The shell part runs the sweep +over the REAL corpus and asserts no mismatch survives. That measures the tree. It +cannot measure the INSTRUMENT: a classifier that filed every site as `armless` would +report zero mismatches, and 470's population premises would still pass on whatever +loops remained. So this file drives the same tool over PLANTED trees whose right +answer is known, and asserts the classification itself. + +Two measurements of one property, which is what the two harnesses are for. The shell +says the corpus is clean; this says the thing that reads the corpus can tell the +difference. Where they disagree, one of them is wrong. + +These tests drive the real tool by subprocess and never the shell harness, for the +reason `test_mutation_ledger.py` gives: a Python twin of a Python tool would agree +with itself. +""" + +import pathlib +import subprocess + +REPO = pathlib.Path(__file__).resolve().parents[2] +TOOL = REPO / ".github" / "scripts" / "skip-loop-arms.py" + +# A loop whose list matches its sibling's arms exactly: the shape the fix produces. +AGREE = '''\ +if [ -z "$reader" ]; then +\tfor _n in "premise: the first thing" \\ +\t\t\t"premise: the second thing"; do +\t\tcheck_skip "$_n" "SKIP $_n" "no reader" +\tdone +else +\tcheck "premise: the first thing" "$(f)" "yes" +\tcheck "premise: the second thing" "$(g)" "yes" +fi +''' + +# The same site after a rename the loop did not follow. One edit from AGREE. +DRIFTED = AGREE.replace('check "premise: the second thing"', + 'check "premise: the second thing, renamed"') + +# The skip IS the record: the branch it stands opposite emits no arm at all. +ARMLESS = '''\ +if [ -z "$reader" ]; then +\tfor _n in "premise: the only thing"; do +\t\tcheck_skip "$_n" "SKIP $_n" "no reader" +\tdone +else +\techo "nothing is checked here" +fi +''' + +# A sibling arm generated by a loop of its own, so a literal set comparison is wrong +# in BOTH directions. This is `340`'s uncomparable site in miniature. +INTERPOLATED = '''\ +if [ -z "$reader" ]; then +\tfor _n in "an unreadable b.c yields no fingerprint" \\ +\t\t\t"an unreadable c.c yields no fingerprint"; do +\t\tcheck_skip "$_n" "SKIP $_n" "no reader" +\tdone +else +\tfor _f in b.c c.c; do +\t\tcheck "an unreadable $_f yields no fingerprint" "$(h)" "yes" +\tdone +fi +''' + + +def _sweep(root, name, body): + """Plant a one-suite tree at `root` and return (counters, mismatches).""" + root.mkdir(parents=True, exist_ok=True) + (root / "selftest").mkdir(exist_ok=True) + (root / name).write_text(body) + r = subprocess.run(["python3", str(TOOL), str(root)], + capture_output=True, text=True) + counts, bad = {}, [] + for line in (r.stdout + r.stderr).splitlines(): + if line.startswith("MISMATCH "): + bad.append(line[len("MISMATCH "):]) + else: + k, _, v = line.partition(" ") + if v.isdigit(): + counts[k] = int(v) + return counts, bad + + +def test_a_loop_naming_its_siblings_arms_is_compared_and_agrees(tmp_path, expect): + counts, bad = _sweep(tmp_path / "t", "agree.sh", AGREE) + expect.num(counts.get("loops", 0), 1, "the planted loop is found") + expect.num(counts.get("compared", 0), 1, "and it is COMPARED, not filed as unreadable") + expect.num(len(bad), 0, "and it agrees with its sibling") + # The premise without which the arm above is worth nothing: a site the tool + # merely failed to parse also reports zero mismatches. + expect.num(counts.get("armless", 0) + counts.get("interpolated", 0), 0, + "and it was not quietly filed as armless or interpolated instead") + + +def test_a_rename_in_the_sibling_branch_is_caught(tmp_path, expect): + """The whole point. Without this the loop is a comment that looks like a guard.""" + counts, bad = _sweep(tmp_path / "t", "drift.sh", DRIFTED) + expect.num(counts.get("compared", 0), 1, "the drifted site is still compared") + expect.num(len(bad), 1, "and the rename is reported as a mismatch") + expect.text(bad[0].split(":")[0], "drift.sh", "named by the file it is in") + + +def test_the_clean_and_drifted_sites_differ_by_exactly_the_rename(tmp_path, expect): + """A pair that must DISAGREE, or neither arm above is evidence. + + Two fixtures one edit apart. If the tool returned the same verdict for both, + each test above would still pass alone while the tool decided nothing. + """ + _, clean = _sweep(tmp_path / "clean", "x.sh", AGREE) + _, drift = _sweep(tmp_path / "drift", "x.sh", DRIFTED) + expect.text(f"{len(clean)}/{len(drift)}", "0/1", + "the same site one rename apart reports differently") + + +def test_an_armless_branch_is_counted_as_armless_and_not_compared(tmp_path, expect): + """Counted out loud, because a site nobody compared and a site that agreed + are indistinguishable in a total of mismatches.""" + counts, bad = _sweep(tmp_path / "t", "armless.sh", ARMLESS) + expect.num(counts.get("loops", 0), 1, "the loop is found") + expect.num(counts.get("armless", 0), 1, "and reported as armless") + expect.num(counts.get("compared", 0), 0, "and NOT counted among the compared") + expect.num(len(bad), 0, "and it is not a mismatch") + + +def test_an_interpolated_sibling_is_reported_rather_than_compared_wrongly(tmp_path, expect): + """`340`'s uncomparable site in miniature. + + The sibling arm is generated by a loop of its own, so a literal set comparison + would report a FALSE mismatch on a correct site. The tool must decline to compare + it and say so -- a guard that manufactures a red is the guard people switch off. + """ + counts, bad = _sweep(tmp_path / "t", "interp.sh", INTERPOLATED) + expect.num(counts.get("interpolated", 0), 1, "the site is reported as interpolated") + expect.num(len(bad), 0, "and NOT reported as a mismatch, which would be false") + expect.num(counts.get("compared", 0), 0, "and not silently counted as compared") + + +def test_every_loop_is_classified_into_exactly_one_category(tmp_path, expect): + """The partition closes, which is what makes the four numbers readable. + + Three planted sites, one of each kind, in one tree: loops must equal + compared + armless + interpolated, or some site fell out of the report. + """ + root = tmp_path / "all" + root.mkdir(parents=True, exist_ok=True) + (root / "selftest").mkdir(exist_ok=True) + for fn, body in (("a.sh", AGREE), ("b.sh", ARMLESS), ("c.sh", INTERPOLATED)): + (root / fn).write_text(body) + r = subprocess.run(["python3", str(TOOL), str(root)], capture_output=True, text=True) + c = {k: int(v) for k, _, v in (l.partition(" ") for l in r.stdout.splitlines()) + if v.isdigit()} + expect.num(c.get("loops", 0), 3, "all three planted loops are found") + expect.text(f"{c.get('compared',0)}+{c.get('armless',0)}+{c.get('interpolated',0)}" + f"={c.get('compared',0)+c.get('armless',0)+c.get('interpolated',0)}", + f"1+1+1={c.get('loops',0)}", + "and the three categories account for every one of them") From 8847a0ae4dcefa41c22c04b0136b9b3856129372 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Fri, 11 Sep 2026 21:38:48 -0600 Subject: [PATCH 2/3] test: the control must name the line it broke, not that something broke (#994) Review round on #1000, from @OffgridwithJD. THE CONTROL WAS THIN AND THE MEASUREMENT SAYS SO. It compared two trees -- clean reports nothing, drifted reports something -- which restates the two arms above it with an `and`. A classifier that reports a mismatch for the WRONG REASON passes it. Driven, rather than argued. Mutant: report a mismatch but name the wrong line. new control got ["two.sh:1"] want ["two.sh:12"] FAIL old control clean=0 drift=1, asserts 0/1 PASSES, blind So the control now plants BOTH loops in one file, the clean one above and the drifted one below, asserts as a premise that both were compared, and pins the reported site to the drifted loop's own line. That is the only form that can tell "it found my bug" from "it found something". AND THE PARTITION ARM IS NOT THE GUARD, which is now written where a reader will find it instead of being left to work out. `compared + armless + interpolated == loops` is the strongest line in the file and the cheapest to satisfy wrongly: a classifier that filed everything as `armless` satisfies it perfectly. It is load-bearing only because the per-bucket arms assert that a KNOWN site lands in the RIGHT bucket. Both halves or neither. The rename was caught in BOTH directions by test_docs_cover_the_corpus -- a documented test that no longer exists, and an existing test not documented -- which is the third time tonight a guard in this tree has decided something rather than been told it. Verified: 255 passed / 581 checks / 0 fail over the 14 files the CI job runs. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01EbyGSaU93XYQr8aH4NrUiw --- CHANGELOG.md | 18 ++++++++++++--- test/pytest/TESTS.md | 20 ++++++++++++++++- test/pytest/test_skip_loop_arms.py | 35 +++++++++++++++++++++++------- 3 files changed, 61 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 657b72df..ffb6181e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,10 +34,22 @@ true until the next version shipped. counted as themselves, and `compared + armless + interpolated == loops` so nothing falls out of the report. - Two removal proofs, each mutation asserted to apply before the run. Stopping the + Three removal proofs, each mutation asserted to apply before the run. Stopping the mismatch report reddens the rename arms; comparing the interpolated site literally - reddens the other two, and that is the one worth keeping, because removing the - tool's refusal to compare manufactures a FALSE mismatch on a correct site. + reddens the other two, because removing the tool's refusal to compare manufactures a + FALSE mismatch on a correct site. + + The third decided the control's shape. Its first version compared two trees, one clean + and one drifted, and a classifier that reports a mismatch naming the WRONG LINE passes + that: `0/1` either way, measured on the mutant. So the control plants both loops in one + file and pins the drifted loop's own line, which is the only form that can tell "it + found my bug" from "it found something". Raised in review by @OffgridwithJD. + + And the partition arm, `compared + armless + interpolated == loops`, is the strongest + line in the file and the cheapest to satisfy wrongly: a classifier filing everything as + `armless` satisfies it perfectly. It is load-bearing only because the per-bucket arms + sit beside it, and that is now written where a reader will find it rather than left to + be worked out. - The pytest harness reports its own check totals, and the record stream is reconciled against what arrived (#937, third phase). diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index 9906e3e6..4281ccd7 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -2574,7 +2574,7 @@ not the first one wearing a Python wrapper. | --- | --- | | `test_a_loop_naming_its_siblings_arms_is_compared_and_agrees` | the clean site is **compared**, not quietly filed as armless or interpolated | | `test_a_rename_in_the_sibling_branch_is_caught` | the whole point: a drifted name is a mismatch | -| `test_the_clean_and_drifted_sites_differ_by_exactly_the_rename` | the pair must **disagree**, or neither arm above is evidence | +| `test_the_mismatch_names_the_loop_that_drifted_and_not_the_clean_one` | it names the **line it broke**, not merely that something is wrong | | `test_an_armless_branch_is_counted_as_armless_and_not_compared` | a site nobody compared and a site that agreed are not the same number | | `test_an_interpolated_sibling_is_reported_rather_than_compared_wrongly` | it declines to compare a site where a literal comparison would be **false in both directions** | | `test_every_loop_is_classified_into_exactly_one_category` | `compared + armless + interpolated == loops`, so nothing fell out of the report | @@ -2592,3 +2592,21 @@ identically whether the code is load-bearing or the edit never landed. The second is the one worth keeping: removing the tool's *refusal* to compare produces a **false** mismatch on a correct site, and a guard that manufactures a red is the guard people switch off. + +| mutation | goes red | +| --- | --- | +| report a mismatch but name the **wrong line** | `..._mismatch_names_the_loop_that_drifted...` | + +That third one is why the control is shaped as it is. Its first version compared two +trees — clean reports nothing, drifted reports something — and **a classifier naming the +wrong line passes that**: `0/1` either way. Measured, on the mutant above. So the test +plants both loops in one file and pins `two.sh:12`, which is the only form that can tell +"it found my bug" from "it found something". + +### The partition is not the guard + +`compared + armless + interpolated == loops` is the strongest line in the file and the +cheapest to satisfy wrongly: a classifier that filed **everything** as `armless` satisfies +it perfectly. It is load-bearing only because the per-bucket tests assert that a known +site lands in the right bucket; the identity then says nothing else escaped. Both halves +or neither. diff --git a/test/pytest/test_skip_loop_arms.py b/test/pytest/test_skip_loop_arms.py index 1203a737..a43b510b 100644 --- a/test/pytest/test_skip_loop_arms.py +++ b/test/pytest/test_skip_loop_arms.py @@ -110,16 +110,28 @@ def test_a_rename_in_the_sibling_branch_is_caught(tmp_path, expect): expect.text(bad[0].split(":")[0], "drift.sh", "named by the file it is in") -def test_the_clean_and_drifted_sites_differ_by_exactly_the_rename(tmp_path, expect): - """A pair that must DISAGREE, or neither arm above is evidence. +def test_the_mismatch_names_the_loop_that_drifted_and_not_the_clean_one(tmp_path, expect): + """It must name the site it broke, not merely report that something is wrong. - Two fixtures one edit apart. If the tool returned the same verdict for both, - each test above would still pass alone while the tool decided nothing. + A two-tree comparison -- clean reports nothing, drifted reports something -- is + satisfied by a classifier that reports A mismatch for the WRONG reason. So this + plants BOTH loops in one file and pins the line: the clean loop at the top and the + drifted one below it, with only the second able to be a finding. + + @OffgridwithJD asked for this after the first version restated the two arms above + it with an `and`, which is fair: "they differ" is weaker than "it named the line + I broke". """ - _, clean = _sweep(tmp_path / "clean", "x.sh", AGREE) - _, drift = _sweep(tmp_path / "drift", "x.sh", DRIFTED) - expect.text(f"{len(clean)}/{len(drift)}", "0/1", - "the same site one rename apart reports differently") + body = AGREE.replace("_n", "_a") + "\n" + DRIFTED.replace("_n", "_b") + counts, bad = _sweep(tmp_path / "t", "two.sh", body) + + # The premise, or the pin below is pinning one of one rather than one of two. + expect.num(counts.get("compared", 0), 2, "premise: BOTH loops were compared") + + drift_line = next(i + 1 for i, l in enumerate(body.splitlines()) + if l.lstrip().startswith("for _b in")) + expect.row_set(bad, [f"two.sh:{drift_line}"], + "the mismatch names the drifted loop's own line, and only it") def test_an_armless_branch_is_counted_as_armless_and_not_compared(tmp_path, expect): @@ -150,6 +162,13 @@ def test_every_loop_is_classified_into_exactly_one_category(tmp_path, expect): Three planted sites, one of each kind, in one tree: loops must equal compared + armless + interpolated, or some site fell out of the report. + + THIS IDENTITY IS NOT THE GUARD, AND A LATER READER SHOULD NOT THINK IT IS. A + classifier that filed every site as `armless` satisfies it perfectly. It is + load-bearing only because the per-bucket tests above assert that a KNOWN site + lands in the RIGHT bucket; this arm then says nothing else escaped. Raised by + @OffgridwithJD, who pointed out it is the strongest line in the file and the + cheapest to satisfy wrongly. """ root = tmp_path / "all" root.mkdir(parents=True, exist_ok=True) From ac989b4d4e0f8cec48f48199112349786a000f37 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Fri, 11 Sep 2026 21:49:11 -0600 Subject: [PATCH 3/3] test: assert the sweep tool succeeded, not merely that it printed (#994) Review round two on #1000, from @OffgridwithJD. EVERY TEST HERE READS THE TOOL'S STDOUT AND NONE NOTICED IT BECOMING UNUSABLE. Reproduced before fixing, with the behaviour asserted to have moved rather than only the edit to have applied: mutation: return 2 from main(), counters still printed correctly tool rc 0 -> 2 behaviour moved pytest half 6 passed / 18 checks BLIND after this commit 6 failed caught The shell half already catches it, through `|| _sk_out="TOOL FAILED"`, so the pair was stronger than this half alone. THE SHARPEST PART IS WHICH ARM CAUGHT IT THERE: the headline `every comparable skip loop names exactly the arms its sibling would emit` stayed GREEN under the mutation, because with no parseable output there are no mismatches to report. A premise failed. An empty finding list and a tool that cannot run are the same observation to an arm that only counts findings. So the assertion goes in `_sweep`, once, covering all six tests -- for that reason rather than for tidiness. The partition test built its own subprocess call and had the same blindness, so it is routed through the helper too. AND THE REPORTER'S OWN FIRST PROBE WAS INVALID, which is worth recording beside the fix: `sys.exit(2)` appended at module level, after the `__main__` guard had already exited. The edit applied cleanly and the behaviour did not move -- rc still 0. An arm asserting only that a mutation APPLIED would have let that be reported as a blindness on no evidence. `a-mutation-that-did-not-apply` needs its second half: assert the behaviour moved, not the text. Verified: 255 passed / 587 checks / 0 fail over the 14 files the CI job runs. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01EbyGSaU93XYQr8aH4NrUiw --- CHANGELOG.md | 10 ++++++++ test/pytest/TESTS.md | 15 +++++++++++ test/pytest/test_skip_loop_arms.py | 41 ++++++++++++++++++------------ 3 files changed, 50 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ffb6181e..5274d1fd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,16 @@ true until the next version shipped. file and pins the drifted loop's own line, which is the only form that can tell "it found my bug" from "it found something". Raised in review by @OffgridwithJD. + A fourth proof closed a blindness the first three shared. Returning 2 from the tool's + `main()` while still printing correct counters left all six tests green: every one of + them reads stdout and none noticed the tool had become unusable. The shell half caught + it through its `TOOL FAILED` fallback, and there it was a PREMISE that failed while the + headline arm stayed green. So the exit code is now asserted once in the shared helper, + and under the same mutation all six fail. Reported by @OffgridwithJD, whose own first + probe of it was invalid and said so: `sys.exit(2)` appended after the `__main__` guard + applied cleanly and moved nothing. Asserting a mutation APPLIED is not asserting the + behaviour MOVED. + And the partition arm, `compared + armless + interpolated == loops`, is the strongest line in the file and the cheapest to satisfy wrongly: a classifier filing everything as `armless` satisfies it perfectly. It is load-bearing only because the per-bucket arms diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index 4281ccd7..5e21c01f 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -2596,6 +2596,7 @@ people switch off. | mutation | goes red | | --- | --- | | report a mismatch but name the **wrong line** | `..._mismatch_names_the_loop_that_drifted...` | +| return 2 from `main()`, counters still correct | **all six** | That third one is why the control is shaped as it is. Its first version compared two trees — clean reports nothing, drifted reports something — and **a classifier naming the @@ -2603,6 +2604,20 @@ wrong line passes that**: `0/1` either way. Measured, on the mutant above. So th plants both loops in one file and pins `two.sh:12`, which is the only form that can tell "it found my bug" from "it found something". +### The exit code is asserted in the helper, not in one test + +Every test here reads the tool's stdout, and **none of them would notice the tool +becoming unusable**. Measured: returning `2` from `main()` while still printing correct +counters left all six green at 18 checks. The shell half catches that through +`|| _sk_out="TOOL FAILED"` — so the pair was stronger than this half alone, and on that +side it was a *premise* that failed while the headline arm stayed green. + +So `_sweep` asserts `returncode == 0` once, covering all six. Under the same mutation +all six now fail. Reported by @OffgridwithJD, whose first probe of it was invalid and +said so: an `sys.exit(2)` appended after the `__main__` guard applied cleanly and changed +nothing, `rc` still 0. **Asserting that a mutation applied is not asserting that the +behaviour moved** — both halves, or the proof is of the edit rather than of the code. + ### The partition is not the guard `compared + armless + interpolated == loops` is the strongest line in the file and the diff --git a/test/pytest/test_skip_loop_arms.py b/test/pytest/test_skip_loop_arms.py index a43b510b..02cba829 100644 --- a/test/pytest/test_skip_loop_arms.py +++ b/test/pytest/test_skip_loop_arms.py @@ -73,13 +73,28 @@ ''' -def _sweep(root, name, body): - """Plant a one-suite tree at `root` and return (counters, mismatches).""" +def _sweep(root, expect, **files): + """Plant a tree of `name=body` suites at `root`, return (counters, mismatches). + + THE EXIT CODE IS ASSERTED HERE rather than in one test, because every test in + this file reads the tool's stdout and none of them would notice the tool + becoming unusable. Measured: returning 2 from `main()` while still printing + correct counters left all six tests green at 18 checks. The shell half catches + that through `|| _sk_out="TOOL FAILED"`, so the pair was stronger than this half + alone -- and on that side it was a PREMISE that failed while the headline arm + stayed green, which is the argument for putting it in the helper. + + Reported by @OffgridwithJD, whose first probe of it was invalid and said so: an + `sys.exit(2)` appended after the `__main__` guard applied cleanly and changed + nothing. Asserting a mutation APPLIED is not asserting the behaviour MOVED. + """ root.mkdir(parents=True, exist_ok=True) (root / "selftest").mkdir(exist_ok=True) - (root / name).write_text(body) + for name, body in files.items(): + (root / f"{name}.sh").write_text(body) r = subprocess.run(["python3", str(TOOL), str(root)], capture_output=True, text=True) + expect.num(r.returncode, 0, "the sweep tool reports success, not merely output") counts, bad = {}, [] for line in (r.stdout + r.stderr).splitlines(): if line.startswith("MISMATCH "): @@ -92,7 +107,7 @@ def _sweep(root, name, body): def test_a_loop_naming_its_siblings_arms_is_compared_and_agrees(tmp_path, expect): - counts, bad = _sweep(tmp_path / "t", "agree.sh", AGREE) + counts, bad = _sweep(tmp_path / "t", expect, agree=AGREE) expect.num(counts.get("loops", 0), 1, "the planted loop is found") expect.num(counts.get("compared", 0), 1, "and it is COMPARED, not filed as unreadable") expect.num(len(bad), 0, "and it agrees with its sibling") @@ -104,7 +119,7 @@ def test_a_loop_naming_its_siblings_arms_is_compared_and_agrees(tmp_path, expect def test_a_rename_in_the_sibling_branch_is_caught(tmp_path, expect): """The whole point. Without this the loop is a comment that looks like a guard.""" - counts, bad = _sweep(tmp_path / "t", "drift.sh", DRIFTED) + counts, bad = _sweep(tmp_path / "t", expect, drift=DRIFTED) expect.num(counts.get("compared", 0), 1, "the drifted site is still compared") expect.num(len(bad), 1, "and the rename is reported as a mismatch") expect.text(bad[0].split(":")[0], "drift.sh", "named by the file it is in") @@ -123,7 +138,7 @@ def test_the_mismatch_names_the_loop_that_drifted_and_not_the_clean_one(tmp_path I broke". """ body = AGREE.replace("_n", "_a") + "\n" + DRIFTED.replace("_n", "_b") - counts, bad = _sweep(tmp_path / "t", "two.sh", body) + counts, bad = _sweep(tmp_path / "t", expect, two=body) # The premise, or the pin below is pinning one of one rather than one of two. expect.num(counts.get("compared", 0), 2, "premise: BOTH loops were compared") @@ -137,7 +152,7 @@ def test_the_mismatch_names_the_loop_that_drifted_and_not_the_clean_one(tmp_path def test_an_armless_branch_is_counted_as_armless_and_not_compared(tmp_path, expect): """Counted out loud, because a site nobody compared and a site that agreed are indistinguishable in a total of mismatches.""" - counts, bad = _sweep(tmp_path / "t", "armless.sh", ARMLESS) + counts, bad = _sweep(tmp_path / "t", expect, armless=ARMLESS) expect.num(counts.get("loops", 0), 1, "the loop is found") expect.num(counts.get("armless", 0), 1, "and reported as armless") expect.num(counts.get("compared", 0), 0, "and NOT counted among the compared") @@ -151,7 +166,7 @@ def test_an_interpolated_sibling_is_reported_rather_than_compared_wrongly(tmp_pa would report a FALSE mismatch on a correct site. The tool must decline to compare it and say so -- a guard that manufactures a red is the guard people switch off. """ - counts, bad = _sweep(tmp_path / "t", "interp.sh", INTERPOLATED) + counts, bad = _sweep(tmp_path / "t", expect, interp=INTERPOLATED) expect.num(counts.get("interpolated", 0), 1, "the site is reported as interpolated") expect.num(len(bad), 0, "and NOT reported as a mismatch, which would be false") expect.num(counts.get("compared", 0), 0, "and not silently counted as compared") @@ -170,14 +185,8 @@ def test_every_loop_is_classified_into_exactly_one_category(tmp_path, expect): @OffgridwithJD, who pointed out it is the strongest line in the file and the cheapest to satisfy wrongly. """ - root = tmp_path / "all" - root.mkdir(parents=True, exist_ok=True) - (root / "selftest").mkdir(exist_ok=True) - for fn, body in (("a.sh", AGREE), ("b.sh", ARMLESS), ("c.sh", INTERPOLATED)): - (root / fn).write_text(body) - r = subprocess.run(["python3", str(TOOL), str(root)], capture_output=True, text=True) - c = {k: int(v) for k, _, v in (l.partition(" ") for l in r.stdout.splitlines()) - if v.isdigit()} + c, _ = _sweep(tmp_path / "all", expect, + a=AGREE, b=ARMLESS, c=INTERPOLATED) expect.num(c.get("loops", 0), 3, "all three planted loops are found") expect.text(f"{c.get('compared',0)}+{c.get('armless',0)}+{c.get('interpolated',0)}" f"={c.get('compared',0)+c.get('armless',0)+c.get('interpolated',0)}",