Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 89 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,95 @@ true until the next version shipped.

### Fixed

- A loop that never ran asserted nothing, and half of that was already refused by a
mechanism nobody had recorded covered it (#432).

`assert-inside-a-loop-over-zero-rows` in VACUITY_MODES.md 3.5 is two shapes.

**Already refused.** When a loop's body holds a test's ONLY counted assertions, a
zero-trip loop leaves the count at 0 and `pytest_runtest_call` raises `VacuityError`.
Measured on a planted test rather than read off the hook: the zero-trip case fails
with "made no counted assertion" and the same loop with one row passes.

**Was open.** When the test ALSO asserts outside the loop, the count is non-zero, the
test passes, and the loop's assertions simply never ran. That is the shape a query
returning no rows produces, and the shape a glob matching nothing produces.

`test_loop_coverage_premise.py` requires a cardinality premise for exactly that shape.
THE POPULATION, measured over the whole corpus before the arm was written: 20 loops
non-empty by construction and so unable to be zero-trip, 0 in the already-refused
shape, and **2 at risk** -- both of which already carried a premise. The arm is green
on arrival, which is the point rather than a weakness: the property was true and
nothing was holding it there, so what this catches is the third one.

**IT NARROWS RATHER THAN CLOSES, and 3.5 names the residual.** The honest requirement
is a premise bounding the cardinality of THIS iterable; what is enforced is a counted
assertion outside the loop taking `len(...)` of something. `test_harness_deps.py`'s
loop iterates `sorted(found)` while its premise bounds `len(files)`, because `found` is
built from `files` in a preceding loop -- so a rule demanding the names match would
reject correct code, which is how a guard gets switched off. The residual is a loop
whose premise bounds the wrong collection: a reviewer catches it, a sweep does not.

Section 5 gains entry 6, struck and anchored to its mode id, which is the first use of
the rule the previous commit added.

- Section 5 of VACUITY_MODES.md, the "what to add next" list, is checked (#432).

1a, 2 and 3 are all compared against the mode ids on disk. Section 5 was prose, and
it was **wrong**: entry 1 still said "the constant exists and nothing writes it" long
after `query_error()` existed and `test_failed_query_sentinel.py` carried ten arms
over it, including the producer's uniqueness and the constant's non-uniqueness that
is the reason the producer exists.

**That is the most expensive place in the document for a stale sentence**, because
its only reader is someone about to build something. The near-miss one document over
is what it costs: a bad enumeration of `test/selftest/340` made an existing block
look like a coverage gap, and the duplicate was written and proven to discriminate
before the duplication was noticed.

**What is checkable is the anchor, not the work.** Entry 1's work landed under four
test names, none of them the one the entry proposed, so asking whether the NAMED test
exists would have passed and said nothing. Two arms hold the list instead:

- every entry names at least one mode id, so it is tied to the inventory at all.
Entry 1 named none, which is exactly how it stayed wrong.
- no UN-STRUCK entry names an id section 2 already claims as refused. An entry whose
id has reached section 2 is done by the document's own accounting, whatever the
test ended up being called.

Entry 1 is struck and anchored to `error-swallowed-to-empty`, and it records what the
entry got wrong rather than replacing it: both halves of "the constant exists and
nothing writes it" were false -- two sites already minted sentinels by hand, and the
missing thing was the REFUSAL in four of the five comparisons.

**Fixing it made the second arm vacuous on this document**, because with every entry
struck there is nothing left to refuse. The planted fixture beside it is therefore
the whole of its evidence, and the document says so rather than leaving it implied: a
two-entry fixture where moving the open entry's id into section 2 must be named, with
the clean control beside it.

The totals do not move -- 28 refused, 44 not refused, 72 named -- because the mode was
already counted as refused. Only the list that tells the next person what to do was
wrong.

**And 3.6 now records measured populations, so the next entry is chosen on evidence.**
Section 5's new rule is that an entry must name a mode id, which makes WHICH id worth
measuring. Four of 3.6's were counted by AST scan over the whole corpus:
`truthy-cursor-from-execute` has 7 sites and **all are benign** -- every one is
`x = cur.execute(...)`, idiomatic in psycopg3, and **zero** branch on it, which is the
dangerous form; `empty-query-string-succeeds` has 0; the multistatement mode has 1, a
setup that fetches nothing with a `count(*)` premise right after it; and the
server-cursor rowcount mode has 2, both legitimate. So all four are PROSPECTIVE, and a
guard for any of them would be insurance rather than a closure. Recorded because a
refusal with no population has not refused anything, and 1a counts section 2 as
"refused today".

One instrument defect of mine, caught by that fixture: the parser matched the
SECTION HEADING as an entry. `re.split(r"^## ")` leaves a chunk beginning "5. What to
add next", which the numbered-item pattern also matches -- inventing an entry 5 that
is the title and colliding with the real entry 5. The fixture reported 3 entries in a
two-entry document, which is how it was found.

- A count `grep` never produced no longer reads as "present" (#929).

#922 replaced roughly 28 `producer | grep -q PAT` tests with
Expand Down
75 changes: 75 additions & 0 deletions test/pytest/TESTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ behaviour, the source of that number is named.
- [21. test_failed_query_sentinel.py: a failed query is not a comparison](#21-test_failed_query_sentinelpy-a-failed-query-is-not-a-comparison)
- [22. test_writes_wrote_rows.py: a write that wrote nothing](#22-test_writes_wrote_rowspy-a-write-that-wrote-nothing)
- [23. test_mutation_ledger.py: which checks have ever been red](#23-test_mutation_ledgerpy-which-checks-have-ever-been-red)
- [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)

## 1. How to read a test in here

Expand Down Expand Up @@ -840,6 +841,27 @@ many times.
| `test_the_anchor_rule_drops_punctuation_and_keeps_underscores` | GitHub's derivation, on the heading the defect was found in |
| `test_an_anchor_that_strips_the_underscores_is_caught` | the exact broken link that shipped, with a control |
| `test_every_in_document_link_in_this_directory_reaches_a_heading` | every contents-list link resolves, with a coverage premise |
| `test_the_next_steps_list_is_anchored_to_the_inventory` | every section 5 entry names a mode id, so the entry can be checked at all |
| `test_no_open_next_step_names_work_the_document_calls_done` | an un-struck entry whose id reached section 2 is stale work to do |
| `test_a_stale_next_step_is_caught_on_a_fixture` | **removal proof**: the shape, planted, with the control beside it |


**Section 5 was the last unchecked part of VACUITY_MODES.md, and it was wrong (#432).**
1a, 2 and 3 are all compared against the ids on disk; "what to add next" was prose.
Entry 1 still said *"the constant exists and nothing writes it"* long after
`query_error()` existed and `test_failed_query_sentinel.py` had ten arms over it — the
most expensive place in the document for a stale sentence, because its only reader is
someone about to build something. The near-miss one document over is the argument: a
bad enumeration of `test/selftest/340` made an existing block look like a gap, and the
duplicate was written and proven to discriminate before anyone noticed.

**What is checkable is the anchor, not the work.** Entry 1's work landed under four
test names, none of them the one the entry proposed, so asking whether the NAMED test
exists would have passed and said nothing. So the arms check that every entry names a
mode id, and that no un-struck entry names an id section 2 already claims.

With every entry now struck, the second arm has nothing to refuse on the real
document. That is what the fixture arm is for.

The five fixture arms exist because everything above them passes on a healthy tree,
which is exactly what a guard that does nothing also does. They run the identical
Expand Down Expand Up @@ -1340,6 +1362,7 @@ fixtures are read off `conftest.py` rather than named in the classifier.
| `test_a_cluster_test_still_needs_the_driver` | **control**: deferring made the IMPORT lazy, not the database optional |
| `test_conftest_imports_no_database_driver_at_module_scope` | the regression named in one line, for whoever edits conftest next |
| `test_the_declaration_is_exactly_the_database_free_half` | `NO_CLUSTER` equals the property, both ways, so an undeclared database-free file is named |
| `test_the_declaration_names_each_file_once` | the list's cardinality, which `membership_report`'s set comparison cannot see. Measured: pytest deduplicates the paths, so the cost is the job's own printed file count, not a double run |
| `test_the_partition_accounts_for_every_file_in_the_corpus` | **premise**: every file lands in exactly one bucket, and neither bucket is the whole corpus |
| `test_the_cluster_fixtures_are_read_off_conftest_rather_than_named_here` | the roots of the property are derived from `conftest.py`, not typed |
| `test_the_classifier_tells_a_plain_file_from_one_that_requests_a_cluster` | the base case and its control, over a fixture corpus |
Expand Down Expand Up @@ -2071,3 +2094,55 @@ If they disagree, one was edited by hand. `suites_not_covered` is 250 of 251, so
gate cannot refuse a new check in 250 suites — a real limit, counted rather than hidden,
which falls as suites are seeded.


## 24. test_loop_coverage_premise.py: a loop that never ran asserted nothing

**Why this file exists.** `assert-inside-a-loop-over-zero-rows` in VACUITY_MODES.md 3.5
is two shapes, and the layer already refused one of them without anyone recording that
it did.

**The half already refused.** When a loop's body holds the test's ONLY counted
assertions, a zero-trip loop leaves the count at 0 and `pytest_runtest_call` raises
`VacuityError`. Measured on a planted test rather than read off the hook:

```
only assertion inside a zero-trip loop VacuityError: made no counted assertion
the same loop with one row 1 passed
```

**The half that was open.** When the test *also* asserts outside the loop, the count is
non-zero, the test passes, and the loop's assertions simply never ran. Nothing noticed.
That is the shape a query returning no rows produces, and the shape a glob matching
nothing produces.

**The population, measured before the arm was written:**

| shape | loops | state |
| --- | ---: | --- |
| non-empty by construction (literal, `range`, local literal) | 20 | cannot be zero-trip |
| derived, loop holds the only assertions | 0 | already refused |
| derived, **with** assertions outside the loop | 2 | at risk, now guarded |

Both at-risk loops already carried a premise, so this arm is **green on arrival**. That
is the point rather than a weakness: the property was true of the corpus and nothing was
holding it there, so what this catches is the third one. It is not insurance against an
imagined shape — it has a population of two and locks it in.

| test | asserts |
| --- | --- |
| `test_every_at_risk_loop_carries_a_coverage_premise` | the corpus itself: every derived loop carrying an assertion has a cardinality premise |
| `test_the_sweep_finds_the_loops_it_is_meant_to_police` | **premise**: the sweep classified loops, because one that parses nothing reports no offenders either |
| `test_a_loop_with_no_premise_is_caught` | **removal proof**: the real shape with the premise removed, with the clean control beside it |
| `test_a_bounded_loop_needs_no_premise` | a literal, a `range` and a local dict's `.items()` are all exempt, because demanding a premise there would be noise a reader edits away |
| `test_the_layer_already_refuses_a_loop_holding_every_assertion` | the measured half, so this file does not claim the whole mode — and that the sweep deliberately skips that shape rather than double-reporting it |

### Why the rule is looser than the property, and what is left

The honest requirement is *a premise bounding the cardinality of **this** iterable*. What
is enforced is *a counted assertion outside the loop that takes `len(...)` of something*.

The two differ, and the reason is dataflow. `test_harness_deps.py`'s loop iterates
`sorted(found)` while its premise bounds `len(files)` — `found` is built from `files` in
a preceding loop. A rule that demanded the names match would reject correct code, which
is how a guard gets switched off. **So the residual is a loop whose premise bounds the
wrong collection**, which a reviewer catches and a sweep does not. 3.5 names it.
Loading
Loading