test: the pytest half of the skip-loop sweep that #998 did not ship (#994)

CHANGELOG.md

@@ -18,6 +18,49 @@ 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.
+
+  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, 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.
+
+  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
+  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).
 

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,93 @@ 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_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 |
+
+### 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.
+
+| 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
+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 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
+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.

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",
 ]
 
 

test/pytest/test_skip_loop_arms.py

@@ -0,0 +1,194 @@
+"""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, 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)
+    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 "):
+            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", 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")
+    # 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", 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")
+
+
+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.
+
+    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".
+    """
+    body = AGREE.replace("_n", "_a") + "\n" + DRIFTED.replace("_n", "_b")
+    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")
+
+    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):
+    """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", 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")
+    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", 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")
+
+
+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.
+
+    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.
+    """
+    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)}",
+                f"1+1+1={c.get('loops',0)}",
+                "and the three categories account for every one of them")