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