diff --git a/CHANGELOG.md b/CHANGELOG.md index 59f0ed23..5274d1fd 100644 --- a/CHANGELOG.md +++ b/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). diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index 8104a84e..5e21c01f 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,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. 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..02cba829 --- /dev/null +++ b/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")