From a924426ea8107a8e32219201b8024fa57ab5f862 Mon Sep 17 00:00:00 2001 From: OffgridwithJD Date: Thu, 10 Sep 2026 00:29:49 +0000 Subject: [PATCH 1/2] test: the document may not name a test the corpus does not have (#908) Step 1 of the sequence @jdatcmd proposed and jd agreed: the reverse-direction arm, before the totals line moves. THE SWEEP WENT ONE WAY ONLY ---------------------------- `undocumented()` computes tests on disk the document fails to name. Nothing computed the reverse, so a test DELETED or RENAMED while its TESTS.md entry survived was caught by the totals line and by nothing else: on disk (1, 1) # test_one.py holds test_alpha document states (2, 1) # "test_one.py: test_alpha and test_beta" the NAMING arm : [] <- says nothing is wrong the TOTALS arm : DISAGREE <- the only arm that reddens That matters because the totals line is what #908 is about removing: its correct value is a function of the merge, so it collides on every rebase touching the corpus -- ten times in one day. Removing it while this direction was uncovered would have retired a check silently, which is the move this gate exists to prevent. Hence step 1 before step 2 rather than after. IT FOUND TWO ON THE SHIPPED CORPUS, AND THEY ARE MINE ------------------------------------------------------ Section 3 named test_layer_rejects_an_absence_assertion_over_an_empty_plan and a control beside it. Neither had ever been written. The work is real and lives in `test_guards_pinned.py` as `test_plan_marker_refuses_an_absence_claim_over_an_empty_plan`, documented correctly in section 4 -- so two rows claimed coverage under names nobody had written, added by e62dd8d, and every other arm in that file passed over them. Both rows removed. Nothing else in the corpus changes. A BACKTICKED NAME IS A CLAIM THAT IT EXISTS -------------------------------------------- That is the rule the arm enforces, and the false-positive budget was measured over the real document BEFORE the arm was written rather than after: 127 backticked names, 2 genuinely absent, and both were defects rather than noise. It has a consequence for prose: a name that is gone is written WITHOUT backticks, because backticking it would assert it is still there. The first place that bit was my own paragraph describing this defect -- the arm reddened on it, which is the rule earning its keep on the commit that introduced it. BOTH HARNESSES, and 350 is the copy with teeth: nothing in the gate runs pytest. Proved able to fail. Fixture arms for each shape, and then the one that matters -- the real stale row put back into the real document: pytest : every test the document names exists in the corpus: got '[1: test_layer_rejects_an_absence_assertion_over_an_empty_plan]' shell : same message, via harness_selftest accounting: 433 passed + 1 failed restored byte-exact, both green again A fixture proves the pattern matches something; only the real document proves the arm aimed at it would fire (@jdatcmd's condition on #907, and it applies here). harness_selftest 434 passed + 0 failed + 0 unrunnable, rc=0 pytest corpus 126 passed docs_style 9 checks PASSED shellcheck -S error clean Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a --- test/pytest/TESTS.md | 29 ++++++- test/pytest/test_docs_cover_the_corpus.py | 77 +++++++++++++++++++ .../selftest/350-the-pytest-corpus-must-be.sh | 71 +++++++++++++++++ 3 files changed, 174 insertions(+), 3 deletions(-) diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index 50c13a65..9355d0a4 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -4,7 +4,7 @@ Reference for anyone reading, running, or adding to `test/pytest/`. The design a the decisions behind the harness are in `design/ISSUE_432_PYTEST_HARNESS.md`. This file covers the tests themselves. -**135 tests in 11 files.** One hundred and twenty of them test the harness rather than the +**138 tests in 11 files.** One hundred and twenty-three of them test the harness rather than the product, and they come first, because a harness that can report a false green makes every other result in this directory worthless. @@ -130,8 +130,6 @@ one of those eight measurements exited 0. | `test_an_unrunnable_test_names_its_reason_and_its_detail` | the `UNRUN` line carries reason and detail | nothing was printed at all | | `test_a_real_failure_outranks_an_unrunnable_test` | a run with both exits 1, not 67 | — | | `test_a_run_with_nothing_unrunnable_still_exits_zero` | **control**: a green run is untouched | — | -| `test_layer_rejects_an_absence_assertion_over_an_empty_plan` | an absence claim over `[]` is refused | it passes: nothing is there to find | -| `test_layer_allows_an_absence_assertion_over_a_real_plan` | **control**: `absent=True` still works on a plan that arrived | — | | `test_layer_rejects_psycopgs_no_count_sentinel` | `rowcount` of `-1` is refused | `-1` and `1` are both numbers, so `num` compares them happily | | `test_layer_rejects_a_broad_except_in_a_test_file` | a broad `except` is uncollectable | it was forbidden in a COMMENT, which enforces nothing | @@ -642,6 +640,28 @@ observed variants are closed", not "the function is now infallible". ## 6. test_docs_cover_the_corpus.py: this document, checked +**THE SWEEP GOES BOTH WAYS NOW (#908).** `undocumented()` computes tests on disk +the document fails to name; `documented_but_absent()` computes the reverse. Only +the second catches a test that is DELETED or RENAMED while its entry survives — +until it existed, that case was held by the totals line alone, and the totals line +is a merge target whose correct value is a function of the merge. Removing it +while this direction was uncovered would have retired a check silently. + +**A BACKTICKED TEST NAME IS A CLAIM THAT IT EXISTS.** That is the rule the arm +enforces, and it has a consequence for prose: a name that is gone is written +WITHOUT backticks, because backticking it would assert it is still there. This +paragraph is the first place that bit — the arm reddened on my own description of +the defect. + +It found two on the corpus that shipped. The rows named +test_layer_rejects_an_absence_assertion_over_an_empty_plan and a control beside +it, in section 3, and neither had ever been written. The work is real and lives in +`test_guards_pinned.py` as +`test_plan_marker_refuses_an_absence_claim_over_an_empty_plan`, documented +correctly in section 4 — so two rows claimed coverage under names never written, +and every other arm here passed over them. + + The file you are reading is checked mechanically, because it went stale inside a single rework and nothing noticed. The corpus grew from 25 tests in three files to 54 in five; the two new files, 29 tests, were named nowhere here, and the header @@ -669,6 +689,9 @@ many times. | `test_the_sweep_finds_the_corpus_rather_than_an_empty_glob` | **premise**: the sweep saw files and tests, so "nothing missing" means something | | `test_every_file_and_test_is_named_in_the_document` | every file and test is named here, and a failure says WHICH | | `test_the_stated_totals_are_the_totals_on_disk` | the bold totals line matches the corpus | +| `test_a_documented_test_that_does_not_exist_is_named` | the reverse sweep: the document may not claim a test the corpus lacks | +| `test_a_document_naming_a_test_that_was_deleted_is_caught` | **removal proof**: the shape the real defect had, on a fixture | +| `test_a_documented_file_that_does_not_exist_is_caught` | a whole file can go the same way, which is how a rename shows up | | `test_a_fully_documented_corpus_reports_nothing_missing` | **control**: no false positive on a complete document | | `test_an_undocumented_test_is_named_rather_than_passed_over` | the exact shape that shipped: file named, one test inside it not | | `test_the_mode_inventory_states_its_own_totals_correctly` | the totals in VACUITY_MODES.md section 1a are the modes on disk | diff --git a/test/pytest/test_docs_cover_the_corpus.py b/test/pytest/test_docs_cover_the_corpus.py index 93c90e71..7730955b 100644 --- a/test/pytest/test_docs_cover_the_corpus.py +++ b/test/pytest/test_docs_cover_the_corpus.py @@ -95,6 +95,83 @@ def test_every_file_and_test_is_named_in_the_document(expect): "every test file and every test in the corpus is named in TESTS.md") +def documented_but_absent(directory, doc): + """Names the DOCUMENT claims that the corpus does not have. + + THE SWEEP ABOVE GOES ONE WAY ONLY. `undocumented()` computes tests on disk + that the document fails to name, and nothing computed the reverse. So a test + DELETED or RENAMED while its entry survived was caught by the totals line and + by nothing else -- and the totals line is a merge target whose correct value + is a function of the merge, so it is the half most likely to be removed + (#908). Removing it while this direction was uncovered would have retired a + check silently, which is the move this file exists to prevent. + + Driven against the real functions, on the corpus that shipped: + + on disk (1, 1) # test_one.py holds test_alpha + document states (2, 1) # "test_one.py: test_alpha and test_beta" + + the NAMING arm : [] <- says nothing is wrong + the TOTALS arm : DISAGREE <- the only arm that reddens + + A BACKTICKED NAME, not any occurrence. The document discusses fixtures and + hypothetical tests in prose, and a bare-word sweep would report those as + missing. Backticks are how this document already marks a real identifier, and + the false-positive budget over the corpus was measured before this was + written rather than after: 127 backticked names, 2 of which were genuinely + absent, and both were real defects rather than noise. + """ + found = corpus_tests(directory) + on_disk_fns = {n for names in found.values() for n in names} + on_disk_files = set(found) + named = set(re.findall(r"`(test_[A-Za-z0-9_]*(?:\.py)?)`", doc.read_text())) + bad = sorted({n for n in named if n.endswith(".py")} - on_disk_files) \ + + sorted({n for n in named if not n.endswith(".py")} - on_disk_fns) + if not bad: + return "[]" + return "[%d:%s]" % (len(bad), "".join(" " + b for b in bad[:6])) + + +def test_a_documented_test_that_does_not_exist_is_named(expect): + """The document must not claim a test the corpus does not have. + + It did. `test_layer_rejects_an_absence_assertion_over_an_empty_plan` and its + control `..._allows_an_absence_assertion_over_a_real_plan` were named in the + test_layer.py section and existed nowhere: the work is real but lives in + test_guards_pinned.py as `test_plan_marker_refuses_an_absence_claim_over_an_empty_plan`, + and is documented correctly there. Two rows claimed coverage under names that + had never been written, and every other arm in this file passed over them -- + which is the point. + """ + expect.text(documented_but_absent(HERE, DOC), "[]", + "every test the document names exists in the corpus") + + +def test_a_document_naming_a_test_that_was_deleted_is_caught(tmp_path, expect): + """The removal proof, on a fixture: the shape the real defect had. + + Without this the arm above passes on a healthy tree, which is exactly what an + arm that computes nothing also does. + """ + (tmp_path / "test_one.py").write_text("def test_alpha(expect):\n pass\n") + doc = tmp_path / "DOC.md" + doc.write_text("**1 tests in 1 files.**\n`test_one.py`: `test_alpha` and `test_beta`\n") + expect.text(documented_but_absent(tmp_path, doc), "[1: test_beta]", + "a documented test that does not exist is named, not passed over") + doc.write_text("**1 tests in 1 files.**\n`test_one.py`: `test_alpha`\n") + expect.text(documented_but_absent(tmp_path, doc), "[]", + "control: a document naming only what exists is clean") + + +def test_a_documented_file_that_does_not_exist_is_caught(tmp_path, expect): + """A whole file can go the same way, and it is how a rename usually shows up.""" + (tmp_path / "test_one.py").write_text("def test_alpha(expect):\n pass\n") + doc = tmp_path / "DOC.md" + doc.write_text("`test_one.py` and `test_gone.py`: `test_alpha`\n") + expect.text(documented_but_absent(tmp_path, doc), "[1: test_gone.py]", + "a documented file that does not exist is named") + + def test_the_stated_totals_are_the_totals_on_disk(expect): """Neither arm above would catch a wrong count: a document can name every test and still miscount them, which is exactly what the stale header did.""" diff --git a/test/selftest/350-the-pytest-corpus-must-be.sh b/test/selftest/350-the-pytest-corpus-must-be.sh index a348b7e3..354c4d05 100644 --- a/test/selftest/350-the-pytest-corpus-must-be.sh +++ b/test/selftest/350-the-pytest-corpus-must-be.sh @@ -137,6 +137,77 @@ check "a stated total that disagrees with disk is visible" \ | grep -oE '[0-9]+' | tr '\n' ' ' | sed 's/ $//')" = "$(_dcv_count "$_dcv_fix")" ] \ && echo agrees || echo differs)" "agrees" + +# ---- and the sweep must go the OTHER way too (#908) ------------------------- +# +# `_dcv_missing` above computes tests on disk the document fails to name. NOTHING +# computed the reverse, so a test DELETED or RENAMED while its entry survived was +# held by the totals line and by nothing else -- and that line is a merge target +# whose correct value is a function of the merge, so it is the half most likely to +# be removed. Removing it while this direction was uncovered would have retired a +# check silently, which is the move this part exists to prevent. +# +# IT FOUND TWO ON THE SHIPPED CORPUS. Section 3 named +# test_layer_rejects_an_absence_assertion_over_an_empty_plan and a control beside +# it, and neither had ever been written; the real work lives in +# test_guards_pinned.py under a different name and is documented correctly in its +# own section. Two rows claimed coverage under names nobody had written. +# +# A BACKTICKED NAME, not any occurrence. This document discusses fixtures and dead +# names in prose, and a bare-word sweep would report those as missing. Backticks +# are how it already marks a real identifier, so backticking a name IS the claim +# that it exists -- which is why the paragraph describing this defect writes the +# dead names without them. + +_dcv_absent() { # _dcv_absent DIR DOC -> "[]" or "[n: a b c]" + local dir="$1" doc="$2" name n=0 bad="" + local ondisk + # Every function and every file the corpus actually has, one per line. + ondisk="$( { grep -hoE '^def (test_[A-Za-z0-9_]+)' "$dir"/test_*.py 2>/dev/null \ + | sed 's/^def //' + for f in "$dir"/test_*.py; do [ -f "$f" ] && printf '%s\n' "${f##*/}"; done + } | sort -u )" + while IFS= read -r name; do + [ -n "$name" ] || continue + printf '%s\n' "$ondisk" | grep -qxF "$name" && continue + n=$((n + 1)); [ "$n" -le 6 ] && bad="$bad $name" + done < <(grep -oE '`test_[A-Za-z0-9_]*(\.py)?`' "$doc" 2>/dev/null \ + | tr -d '`' | sort -u) + [ "$n" -eq 0 ] && { printf '[]'; return; } + printf '[%d:%s]' "$n" "$bad" +} + +check "premise: the reverse sweep reads backticked names at all" \ + "$([ "$(grep -coE '`test_[A-Za-z0-9_]*(\.py)?`' "$_dcv_doc")" -ge 20 ] \ + && echo enough || echo too-few)" "enough" + +check "every test the document names exists in the corpus" \ + "$(_dcv_absent "$_dcv_dir" "$_dcv_doc")" "[]" + +# ---- and it must be able to FAIL, on a fixture rather than on the tree ------- + +printf 'def test_alpha(expect):\n pass\n' > "$_dcv_fix/test_one.py" + +printf '`test_one.py`: `test_alpha` and `test_beta`\n' > "$_dcv_fix/GHOST.md" +check "a documented test that does not exist is named, not passed over" \ + "$(_dcv_absent "$_dcv_fix" "$_dcv_fix/GHOST.md")" "[1: test_beta]" + +printf '`test_one.py`: `test_alpha`\n' > "$_dcv_fix/REAL.md" +check "control: a document naming only what exists is clean" \ + "$(_dcv_absent "$_dcv_fix" "$_dcv_fix/REAL.md")" "[]" + +printf '`test_one.py` and `test_gone.py`: `test_alpha`\n' > "$_dcv_fix/GONEFILE.md" +check "a documented file that does not exist is named" \ + "$(_dcv_absent "$_dcv_fix" "$_dcv_fix/GONEFILE.md")" "[1: test_gone.py]" + +# A name in prose without backticks is not a claim, so it must NOT be reported -- +# otherwise the document could never describe a test it removed. +printf 'the old test_beta was removed; `test_alpha` remains\n' > "$_dcv_fix/PROSE.md" +check "an unbackticked name in prose is not treated as a claim" \ + "$(_dcv_absent "$_dcv_fix" "$_dcv_fix/PROSE.md")" "[]" + +unset -f _dcv_absent + unset _dcv_dir _dcv_doc _dcv_seen _dcv_stated _dcv_fix unset -f _dcv_missing _dcv_count From d78c8a62a8b03b1f658369b4efaa76b281d0598a Mon Sep 17 00:00:00 2001 From: OffgridwithJD Date: Thu, 10 Sep 2026 00:39:41 +0000 Subject: [PATCH 2/2] test: the corpus document states no totals (#908) Step 2 of the sequence. Depends on #914. THE NUMBER WAS WRITTEN RATHER THAN DERIVED ------------------------------------------- TESTS.md carried `**N tests in M files.**` and selftest/350 compared it against the corpus. The check was never the problem. Its correct value is a function of the MERGE rather than of either branch, so it collided on essentially every rebase that touched the corpus -- ten times in one day, BOTH SIDES WRONG EVERY TIME. There was no side to pick. REMOVING IT COSTS NOTHING, AND THE ARGUMENT NEEDS THREE ARMS ------------------------------------------------------------- It was first written with two, and @jdatcmd broke it: test_every_file_and_test_is_named_in_the_document disk subset of document test_a_documented_test_that_does_not_exist_is_named document subset of disk Two subsets in opposite directions is equality of the two NAME SETS. That is NOT equality of DEFINITION COUNTS, and a total counts definitions: two files defining test_shared_shape definitions on disk 2 distinct names 1 both arms GREEN -> the counts differ while nothing reddens Reproduced here rather than accepted: both arms clean, 2 against 1. It cannot happen on the corpus today -- 139 definitions against 139 distinct names -- so the conclusion was true in fact but not by construction, which is the difference between an argument and a guard. test_no_test_name_is_defined_twice_in_the_corpus is the missing premise, and it closes something real beyond the argument: `undocumented()` asks whether a name appears in the document AT ALL, so a test defined TWICE and documented ONCE reads as fully covered while pytest runs both. With uniqueness, a count over the document is a count over the corpus, and the stated total was a derived value maintained by hand. THE PROSE COUNT LEAVES WITH IT ------------------------------- jd's condition. The header carried two numbers a line apart, one gated and one not; removing only the gated half would have been a net loss. Nothing read the prose count -- measured, a grep across test/ returns nothing. Both are gone. THE COUNTS MOVE RATHER THAN VANISH ----------------------------------- Both harnesses report them from the corpus every run, where they cannot go stale. AND AN ARM SO THE LINE CANNOT COME BACK ---------------------------------------- Its absence is now a decision rather than an accident. `stated_totals` stays for it, with a premise driving the reader over a fixture that DOES state totals, so "no totals line" cannot be confused with a reader that stopped working. ONE MORE CONSISTENCY FIX, BECAUSE IT COST SOMETHING IMMEDIATELY ---------------------------------------------------------------- `documented_but_absent` returned a preformatted "[n: a b c]" string, copying the bash twin's shape rather than its Python neighbour `undocumented`, which returns a sorted list. Two return types for one concept, and it bit within the hour: my own check of @jdatcmd's finding tested `x in ("[]", "")`, which is False for an empty LIST, and reported a real defect as unreproducible. It returns a sorted list now. STEP 1 CAUGHT STEP 2, ONE COMMIT AFTER IT LANDED ------------------------------------------------- Deleting test_the_stated_totals_are_the_totals_on_disk left its TESTS.md row behind and #914's reverse sweep reddened by name. Proved able to fail. The totals line put back WITH THE CORRECT NUMBERS, because the objection is to the document carrying a count at all; both harnesses redden and restore byte-exact. The uniqueness arm proved on a two-file fixture. An earlier run of the totals proof printed `432 passed + 2 failed`, and neither I nor @jdatcmd could reproduce a second failure. The reproducible figure is 1 failed. No explanation is offered and the sentence stays. harness_selftest 437 passed + 0 failed + 0 unrunnable, rc=0 pytest corpus 147 passed docs_style 9 checks PASSED shellcheck -S error clean Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a --- test/pytest/TESTS.md | 33 ++++- test/pytest/test_docs_cover_the_corpus.py | 137 +++++++++++++++--- .../selftest/350-the-pytest-corpus-must-be.sh | 77 ++++++++-- 3 files changed, 211 insertions(+), 36 deletions(-) diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index 9355d0a4..24e628cb 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -4,9 +4,31 @@ Reference for anyone reading, running, or adding to `test/pytest/`. The design a the decisions behind the harness are in `design/ISSUE_432_PYTEST_HARNESS.md`. This file covers the tests themselves. -**138 tests in 11 files.** One hundred and twenty-three of them test the harness rather than the -product, and they come first, because a harness that can report a false green makes -every other result in this directory worthless. +This file names every test in the corpus and says what each one asserts. **It +states no totals**, and that is deliberate (#908). + +A count here was a claim whose correct value is a function of the MERGE rather +than of either branch, so it collided on essentially every rebase that touched +the corpus -- ten times in one day, and both sides wrong every time. + +It was also redundant, and the argument needs THREE arms rather than the two it +was first written with. `test_every_file_and_test_is_named_in_the_document` +requires every test on disk to be named here; +`test_a_documented_test_that_does_not_exist_is_named` requires every name here to +exist on disk; and `test_no_test_name_is_defined_twice_in_the_corpus` requires +those names to be UNIQUE. The first two give equality of the two NAME SETS, which +is not equality of DEFINITION COUNTS -- two files defining one name leave both +arms green while the counts differ (@jdatcmd). With uniqueness as well, a count +over this document is a count over the corpus, and a number added nothing except +a thing to get wrong. + +The harness prints the counts on every run, where they cannot go stale. Note that +a count of test FUNCTIONS is not the count of collected ITEMS -- parametrized +tests expand -- so `--pgc-expect-tests` takes the run's own collected count and is +documented in README.md beside the invocation that uses it. + +The harness tests come first, because a harness that can report a false green +makes every other result in this directory worthless. That ratio is not an accident of taste. Two of those files exist because a reviewer neutered the guards one at a time and found most of them deletable with the suite @@ -688,10 +710,13 @@ many times. | --- | --- | | `test_the_sweep_finds_the_corpus_rather_than_an_empty_glob` | **premise**: the sweep saw files and tests, so "nothing missing" means something | | `test_every_file_and_test_is_named_in_the_document` | every file and test is named here, and a failure says WHICH | -| `test_the_stated_totals_are_the_totals_on_disk` | the bold totals line matches the corpus | | `test_a_documented_test_that_does_not_exist_is_named` | the reverse sweep: the document may not claim a test the corpus lacks | | `test_a_document_naming_a_test_that_was_deleted_is_caught` | **removal proof**: the shape the real defect had, on a fixture | | `test_a_documented_file_that_does_not_exist_is_caught` | a whole file can go the same way, which is how a rename shows up | +| `test_the_document_states_no_totals_for_a_merge_to_get_wrong` | the totals line must not come back; its absence is a decision, not an accident | +| `test_no_test_name_is_defined_twice_in_the_corpus` | the premise the set-equality argument needs: names must be unique | +| `test_a_name_defined_in_two_files_is_caught` | **removal proof**: the shape that defeats the argument, on a fixture | +| `test_the_corpus_counts_are_reported_rather_than_written` | the counts move to the run's output, where they cannot go stale | | `test_a_fully_documented_corpus_reports_nothing_missing` | **control**: no false positive on a complete document | | `test_an_undocumented_test_is_named_rather_than_passed_over` | the exact shape that shipped: file named, one test inside it not | | `test_the_mode_inventory_states_its_own_totals_correctly` | the totals in VACUITY_MODES.md section 1a are the modes on disk | diff --git a/test/pytest/test_docs_cover_the_corpus.py b/test/pytest/test_docs_cover_the_corpus.py index 7730955b..c4ab613f 100644 --- a/test/pytest/test_docs_cover_the_corpus.py +++ b/test/pytest/test_docs_cover_the_corpus.py @@ -56,10 +56,15 @@ def undocumented(directory, doc_path): return sorted(missing) +def stated_totals_in(text): + """-> (tests, files) the TEXT claims, or None if it states none.""" + m = TOTALS.search(text) + return (int(m.group(1)), int(m.group(2))) if m else None + + def stated_totals(doc_path): """-> (tests, files) the document claims, or None if it states none.""" - m = TOTALS.search(pathlib.Path(doc_path).read_text()) - return (int(m.group(1)), int(m.group(2))) if m else None + return stated_totals_in(pathlib.Path(doc_path).read_text()) def _fixture(tmp_path, doc_body): @@ -125,11 +130,13 @@ def documented_but_absent(directory, doc): on_disk_fns = {n for names in found.values() for n in names} on_disk_files = set(found) named = set(re.findall(r"`(test_[A-Za-z0-9_]*(?:\.py)?)`", doc.read_text())) - bad = sorted({n for n in named if n.endswith(".py")} - on_disk_files) \ - + sorted({n for n in named if not n.endswith(".py")} - on_disk_fns) - if not bad: - return "[]" - return "[%d:%s]" % (len(bad), "".join(" " + b for b in bad[:6])) + # A SORTED LIST, like undocumented() beside it. It returned a preformatted + # "[n: a b c]" string first, copying the bash twin's shape rather than its + # Python neighbour's, and the two-return-types-one-concept split immediately + # cost something real: a check written against it read `x in ("[]", "")`, + # which is False for an empty LIST, and reported a defect as unreproducible. + return sorted({n for n in named if n.endswith(".py")} - on_disk_files) \ + + sorted({n for n in named if not n.endswith(".py")} - on_disk_fns) def test_a_documented_test_that_does_not_exist_is_named(expect): @@ -143,7 +150,7 @@ def test_a_documented_test_that_does_not_exist_is_named(expect): had never been written, and every other arm in this file passed over them -- which is the point. """ - expect.text(documented_but_absent(HERE, DOC), "[]", + expect.text(", ".join(documented_but_absent(HERE, DOC)) or "none", "none", "every test the document names exists in the corpus") @@ -156,10 +163,10 @@ def test_a_document_naming_a_test_that_was_deleted_is_caught(tmp_path, expect): (tmp_path / "test_one.py").write_text("def test_alpha(expect):\n pass\n") doc = tmp_path / "DOC.md" doc.write_text("**1 tests in 1 files.**\n`test_one.py`: `test_alpha` and `test_beta`\n") - expect.text(documented_but_absent(tmp_path, doc), "[1: test_beta]", + expect.text(", ".join(documented_but_absent(tmp_path, doc)), "test_beta", "a documented test that does not exist is named, not passed over") doc.write_text("**1 tests in 1 files.**\n`test_one.py`: `test_alpha`\n") - expect.text(documented_but_absent(tmp_path, doc), "[]", + expect.text(", ".join(documented_but_absent(tmp_path, doc)) or "none", "none", "control: a document naming only what exists is clean") @@ -168,25 +175,109 @@ def test_a_documented_file_that_does_not_exist_is_caught(tmp_path, expect): (tmp_path / "test_one.py").write_text("def test_alpha(expect):\n pass\n") doc = tmp_path / "DOC.md" doc.write_text("`test_one.py` and `test_gone.py`: `test_alpha`\n") - expect.text(documented_but_absent(tmp_path, doc), "[1: test_gone.py]", + expect.text(", ".join(documented_but_absent(tmp_path, doc)), "test_gone.py", "a documented file that does not exist is named") -def test_the_stated_totals_are_the_totals_on_disk(expect): - """Neither arm above would catch a wrong count: a document can name every test - and still miscount them, which is exactly what the stale header did.""" - stated = stated_totals(DOC) - expect.text(repr(stated is not None), "True", - "TESTS.md states its totals in a form that can be read back") +def test_no_test_name_is_defined_twice_in_the_corpus(expect): + """The premise the set-equality argument needs, and it was missing (@jdatcmd). + + #919 argues that removing the totals line costs nothing because the two + sweeps give set EQUALITY between the document and the corpus. That is true of + NAMES and it is not true of DEFINITION COUNTS, which is what a total counts: + + two files defining test_shared_shape + definitions on disk 2 + distinct names 1 + undocumented() clean + documented_but_absent() clean + -> BOTH ARMS GREEN, and the counts differ + + Measured, not argued. It cannot happen today -- 139 definitions against 139 + distinct names, zero duplicates -- so the conclusion was true in fact but not + by construction, which is the difference between an argument and a guard. + + IT CLOSES SOMETHING REAL BEYOND THE ARGUMENT. `undocumented()` asks whether a + name appears in the document at all, so a test defined TWICE and documented + ONCE reads as fully covered. The second definition is invisible to every arm + here, and pytest runs both. + """ found = corpus_tests(HERE) - expect.text(repr(stated), repr((sum(len(v) for v in found.values()), len(found))), - "and the totals it states are the totals on disk") + names = [n for tests in found.values() for n in tests] + expect.at_least(len(names), 20, "premise: the corpus was found") + dupes = sorted({n for n in names if names.count(n) > 1}) + expect.text(", ".join(dupes) or "none", "none", + "no test name is defined twice in the corpus") + expect.num(len(names), len(set(names)), + "so definitions and distinct names are the same count") + + +def test_a_name_defined_in_two_files_is_caught(tmp_path, expect): + """The removal proof, and the exact shape that defeats the equality argument.""" + (tmp_path / "test_a.py").write_text("def test_shared_shape(expect):\n pass\n") + (tmp_path / "test_b.py").write_text("def test_shared_shape(expect):\n pass\n") + found = corpus_tests(tmp_path) + names = [n for tests in found.values() for n in tests] + expect.num(len(names), 2, "premise: both definitions were seen") + expect.num(len(set(names)), 1, "and they share one name") + expect.text(", ".join(sorted({n for n in names if names.count(n) > 1})), + "test_shared_shape", + "a name defined in two files is named, not passed over") + + +def test_the_document_states_no_totals_for_a_merge_to_get_wrong(expect): + """TESTS.md must NOT carry a totals line (#908, step 2). + + It used to, and `selftest/350` compared it against the corpus, which is what + made it a claim rather than decoration. The problem was never the check: it + was that the number was WRITTEN rather than DERIVED, and its correct value is + a function of the MERGE rather than of either branch. It collided on + essentially every rebase touching the corpus -- ten times in one day, both + sides wrong every time, so there was no side to pick. + + REMOVING IT COSTS NOTHING, and that is provable rather than hopeful. The two + sweeps together are strictly stronger than any count: + + test_every_file_and_test_is_named_in_the_document + every test on disk is named here (disk subset of document) + test_a_documented_test_that_does_not_exist_is_named + every name here exists on disk (document subset of disk) + + Two subsets in opposite directions is set EQUALITY, so the documented set and + the corpus are the same set, and any count over one equals the count over the + other. A stated total was a derived value written by hand. + + WHY THIS IS AN ARM AND NOT JUST A DELETION. Nothing stops the next person + adding the sentence back -- it reads like an improvement. This arm is what + makes its absence a decision rather than an accident, and `stated_totals` + stays for it: the reader still has to work, or "no totals line" would be + indistinguishable from "cannot find one". + """ + expect.text(repr(stated_totals(DOC)), "None", + "TESTS.md states no totals line for a merge to get wrong") + # And the reader that reports it must still be able to FIND one, or the arm + # above passes because the parser is broken rather than because the line is + # gone -- the exact shape this corpus exists to refuse. + expect.text(repr(stated_totals_in("**7 tests in 3 files.** and prose")), + "(7, 3)", + "premise: the reader still finds a totals line when one is there") + + +def test_the_corpus_counts_are_reported_rather_than_written(expect): + """The counts do not vanish; they move to where they cannot go stale. + + A number nobody maintains is better than a wrong one, but a number nobody can + SEE is worse than both. The harness prints them every run, computed from the + corpus, so a reader gets the same information without the document asserting + anything. + """ + found = corpus_tests(HERE) + total = sum(len(v) for v in found.values()) + expect.at_least(total, 20, "premise: the corpus was found, so a count means something") + expect.num(len(found), len({f for f in found}), "each file counted once") + print(f"\nCORPUS: {total} test functions in {len(found)} files") -# --------------------------------------------------------------------------- -# And the guard must be able to FAIL. Everything above passes on a healthy tree, -# which is exactly what a guard that does nothing also does. -# --------------------------------------------------------------------------- def test_a_fully_documented_corpus_reports_nothing_missing(tmp_path, expect): """Control. A guard with a bad false-positive rate gets switched off, and then diff --git a/test/selftest/350-the-pytest-corpus-must-be.sh b/test/selftest/350-the-pytest-corpus-must-be.sh index 354c4d05..6835fe29 100644 --- a/test/selftest/350-the-pytest-corpus-must-be.sh +++ b/test/selftest/350-the-pytest-corpus-must-be.sh @@ -88,17 +88,41 @@ check "premise: the sweep found the corpus rather than an empty glob" \ check "every test file and every test in the corpus is named in TESTS.md" \ "$(_dcv_missing "$_dcv_dir" "$_dcv_doc")" "[]" -# The count TESTS.md states, parsed out of it. Written in a fixed form precisely -# so it can be read back: prose that says "twenty-five" cannot be compared with -# anything, which is how the stale header survived being read many times. +# TESTS.md STATES NO TOTALS, AND THAT IS THE POINT (#908). +# +# It used to, and this part compared the stated pair against the corpus, which is +# what made it a claim rather than decoration. The problem was never the check: it +# was that the number was WRITTEN rather than DERIVED, and its correct value is a +# function of the MERGE rather than of either branch. It collided on essentially +# every rebase touching the corpus -- ten times in one day, both sides wrong every +# time, so there was no side to pick. +# +# REMOVING IT COSTS NOTHING, provably. The two sweeps are strictly stronger than +# any count: the arm above requires every test on disk to be NAMED here, and the +# reverse arm below requires every name here to EXIST on disk. Two subsets in +# opposite directions is set equality, so any count over the document equals the +# count over the corpus. A stated total was a derived value maintained by hand. _dcv_stated="$(grep -oE '^\*\*[0-9]+ tests in [0-9]+ files\.\*\*' "$_dcv_doc" \ | head -1 | grep -oE '[0-9]+' | tr '\n' ' ' | sed 's/ $//')" -check "TESTS.md states its totals in a form that can be read back" \ - "$([ -n "$_dcv_stated" ] && echo yes || echo no)" "yes" - -check "and the totals it states are the totals on disk" \ - "$_dcv_stated" "$_dcv_seen" +check "TESTS.md states no totals line for a merge to get wrong" \ + "$([ -z "$_dcv_stated" ] && echo none || echo "$_dcv_stated")" "none" + +# PREMISE: the reader must still be able to FIND a totals line, or the arm above +# passes because the parser is broken rather than because the line is gone -- +# which is the shape this whole part exists to refuse. +# $_dcv_fix is not created until the failure-proof section below, and this file +# runs under `set -u`, so this fixture makes its own path rather than borrowing +# one that does not exist yet. +_dcv_ht="$PGC_WORKDIR/doccov-hastotals"; mkdir -p "$_dcv_ht" +printf '**7 tests in 3 files.** and prose\n' > "$_dcv_ht/HASTOTALS.md" +check "premise: the reader still finds a totals line when one is there" \ + "$(grep -oE '^\*\*[0-9]+ tests in [0-9]+ files\.\*\*' "$_dcv_ht/HASTOTALS.md" \ + | head -1 | grep -oE '[0-9]+' | tr '\n' ' ' | sed 's/ $//')" "7 3" + +# And the counts still REACH a reader -- they move to this run's output, where +# they are computed from the corpus and cannot go stale. +echo " CORPUS: $_dcv_seen (test functions, files)" # ---- and the guard must be able to FAIL -------------------------------------- # @@ -132,6 +156,9 @@ check "an undocumented file is caught along with the tests inside it" \ check "the sweep counts the fixture's tests and files" \ "$(_dcv_count "$_dcv_fix")" "2 1" +# Kept although the real document no longer states a total: it proves the +# comparison still works, so "no totals line" above is a fact about the document +# rather than about a broken reader. check "a stated total that disagrees with disk is visible" \ "$([ "$(grep -oE '^\*\*[0-9]+ tests in [0-9]+ files\.\*\*' "$_dcv_fix/GOOD.md" \ | grep -oE '[0-9]+' | tr '\n' ' ' | sed 's/ $//')" = "$(_dcv_count "$_dcv_fix")" ] \ @@ -208,7 +235,39 @@ check "an unbackticked name in prose is not treated as a claim" \ unset -f _dcv_absent -unset _dcv_dir _dcv_doc _dcv_seen _dcv_stated _dcv_fix +# ---- and the names must be UNIQUE, or the equality argument does not hold ---- +# +# The two sweeps give equality of the two NAME SETS. That is not equality of +# DEFINITION COUNTS, which is what a total counts: two files defining one name +# leave both arms green while the counts differ (@jdatcmd, against #919). It +# cannot happen today, so the conclusion was true in fact and not by +# construction -- the difference between an argument and a guard. +# +# It closes something real beyond the argument: `_dcv_missing` asks whether a +# name appears in the document AT ALL, so a test defined TWICE and documented +# ONCE reads as fully covered while pytest runs both. + +_dcv_dupes() { # _dcv_dupes DIR -> "" or the repeated names + grep -hoE '^def (test_[A-Za-z0-9_]+)' "$1"/test_*.py 2>/dev/null \ + | sed 's/^def //' | sort | uniq -d +} + +check "no test name is defined twice in the corpus" \ + "$(_dcv_dupes "$_dcv_dir" | tr '\n' ' ' | sed 's/ $//')" "" + +# And it must be able to FAIL, on a fixture rather than on the tree. +printf 'def test_shared_shape(expect):\n pass\n' > "$_dcv_fix/test_a.py" +printf 'def test_shared_shape(expect):\n pass\n' > "$_dcv_fix/test_b.py" +check "a name defined in two files is named, not passed over" \ + "$(_dcv_dupes "$_dcv_fix" | tr '\n' ' ' | sed 's/ $//')" "test_shared_shape" +rm -f "$_dcv_fix/test_a.py" "$_dcv_fix/test_b.py" + +check "control: distinct names in the same corpus report no duplicate" \ + "$(_dcv_dupes "$_dcv_dir" | tr '\n' ' ' | sed 's/ $//')" "" + +unset -f _dcv_dupes + +unset _dcv_dir _dcv_doc _dcv_seen _dcv_stated _dcv_fix _dcv_ht unset -f _dcv_missing _dcv_count # ---- the mode inventory must count itself, and the count must be checkable ----