Guard tests that pass for the wrong reason: five PRs, no mechanical gate nf-p8p ← Beads

open priority 2 task unassigned

Twinned from https://github.com/cfsh/ninefold/issues/782 by tools/beads/import_github.py's reconcile pass.

Unclaimed, unlabelled — a systemic finding, not a chunk. Filed from the bork standing slot during a poll-loop strategic-uplift window.

The class

A test whose name states a property, whose assertions check something adjacent to it, and which therefore passes for a reason unrelated to the property. Deleting the code it exists to guard leaves the suite green.

This is distinct from "untested code", and worse than it, because it reports as *covered*. Nothing in this repo distinguishes the two.

It is not rare, and it is not one session

ninefold-reviewer has raised it on at least five PRs by five different authors-of-record, and the issue titles alone tell the story:

| issue | title fragment | |---|---| | #723 | "a test name that asserts its own opposite, and the tool-path check that was stubbed out" | | #719 | "f5_health's unreachable no-ref guard, a dead OSError clause, two soft assertions" | | #688 | "the claims half asserts more than it checked" | | #597 | "making the B2 body-currency check actually fire" | | #772 | four separate instances on one branch — see below |

⚠️ Every one of these was caught by a model re-deriving the assertion by hand at review time. There is no mechanical gate, so the catch rate is however careful the reviewer happened to be on that pass — and a reviewer that skips a pass, or a PR that merges Tier-1 without one, has no second line.

The four on #772, because the pattern in them is specific
  1. _count()'s test pinned the lie (a failed rev-list reported as 0) rather than the safety.
  2. The fixture separated tracked from untracked files using the *same conflation* the code under test had, so it could not detect it — 32 green tests said nothing.
  3. test_the_fetch_is_bounded_well_inside_the_hooks_own_timeout asserted a constant against a constant plus a mapping three other tests already covered. Deleting timeout=FETCH_TIMEOUT from both call sites left the suite green (verified by doing it).
  4. FakeRunner recorded **kwargs and silently dropped check, the one keyword this tier's trust boundary is about — and the docstring claimed it recorded everything.

⚠️ The through-line: all four were written *while* fixing the defect they were meant to lock down — the moment the author is most convinced the property holds and least inclined to check that the check checks. That is not a discipline problem to be exhorted away; it is a predictable blind spot at a predictable moment.

What would actually catch it

Cheap first, in the order I would try them:

  1. The manual move, made a convention. Delete the line the test guards, run the suite, expect red, restore. Costs ~30s. Worth a line in docs/DEV_PRINCIPLES.md alongside F1 — it converts "I believe this test works" into evidence, and it is the only thing that reliably caught any of the four above.
  2. A presubmit.py mutation lane, scoped and opt-in. Full mutation testing over tools/ would be far too slow for a ~7s gate. But a narrow version is tractable: for each tools/**/*.py line matching a small set of high-value patterns (a keyword argument at a call site, a comparison operator, a not), delete-or-flip it, run only the module's own test file, and report any mutant that survives. Run as a separate slow lane — nightly, or --lane mutate — never in the 7s path. test_nflib_git.py runs in well under a second, so a few hundred mutants over nflib/ is minutes, not hours.
  3. A reviewer CHARTER prompt for it, if 2 is judged not worth building. Cheapest of the three and it already half-happens; making it an explicit checklist item would at least make the catch rate deliberate rather than incidental.

⚠️ I would not start with 2. Option 1 is free and closes the specific moment the four instances above share. Measure how much survives it before building a lane.

What this is not

Dependencies

None.

Comments

No comments.

Add a comment