[Review] Audit the test suite for tests that cannot fail #441

New issue

Open

opened 2026-08-29 18:17:59 +00:00 by claude-bot · 0 comments

claude-bot commented

2026-08-29 18:17:59 +00:00

Contributor

Part of the v4.6.0 release-readiness gate.

Why this is its own issue

This project's most expensive recurring defect is not a bug — it is a green test that proves nothing. It has now happened at least six times:

A 60-second fixture standing in for a 77-minute session.
A validator check made unreachable by a clamp three lines away, with a test asserting only what the clamp already guaranteed.
A hallucination test that passed because the deduper collapsed its fixture, not because the filter it was named for existed.
A json_schema shape copied from a vendor README that shipped and did nothing, green unit test throughout.
#425's uncaptured-member fix, tested in isolation where it worked, while the composition it lived in discarded the result (#432).
#428: nothing asserted that a key in _ENCRYPTED_KEYS was actually stored encrypted, so a plaintext credential sat at rest for months.

Every one was found by accident, late, by someone reading the code for another reason. A suite that cannot fail is worse than no suite: it converts absence of coverage into a false claim of coverage.

What to look for

Assertions guaranteed by construction. The canonical shape: asserting two fields agree when both are set from the same source. speaker_label and track_owner_id are both stamped from the same Track, so asserting they match tests nothing.
Fixtures too small to reach the code under test. Anything below a guard's threshold, shorter than a window, or fewer items than a batch size.
Tests named for a behaviour they do not exercise — the name says "filter", the fixture never reaches the filter.
Unit tests of a function that is correct in isolation and wrong in composition. #432 exactly.
Missing invariants. Not a broken assertion, an absent one: nothing asserted encryption at rest, so nothing noticed its absence.
Over-mocking, where the mock encodes the assumption the test should be checking.

Method

Mutation is the only proof. For each test claiming to guard a behaviour, break that behaviour and confirm the test goes red. A test that stays green is either testing something else or nothing. This is slow and it is the entire point — every instance above would have been caught by one mutation.

Prioritise by blast radius: tests guarding data destruction, attribution, authentication and money-shaped invariants first.

Acceptance criteria

Every test guarding a data-loss, attribution or auth path has been mutation-checked and seen to fail against the broken behaviour
Tests found to be vacuous are fixed or deleted — a deleted vacuous test is an improvement, since it removes a false claim
Missing invariants identified during the audit are added as real assertions
The recurring patterns are written into CLAUDE.md or a testing guide, so the seventh instance is prevented rather than found
Where a test cannot be made meaningful without a larger fixture, that fixture is built or the gap is documented explicitly

#433 — the synthetic-audio harness exists partly because the transcription path had no way to be tested for real. Its design already assumes this discipline: every guard it adds is reverted and confirmed to fail first.

Part of the v4.6.0 release-readiness gate. ## Why this is its own issue This project's most expensive recurring defect is not a bug — it is a **green test that proves nothing**. It has now happened at least six times: 1. A 60-second fixture standing in for a 77-minute session. 2. A validator check made unreachable by a clamp three lines away, with a test asserting only what the clamp already guaranteed. 3. A hallucination test that passed because the deduper collapsed its fixture, not because the filter it was named for existed. 4. A `json_schema` shape copied from a vendor README that shipped and did nothing, green unit test throughout. 5. #425's uncaptured-member fix, tested in isolation where it worked, while the composition it lived in discarded the result (#432). 6. #428: nothing asserted that a key in `_ENCRYPTED_KEYS` was actually stored encrypted, so a plaintext credential sat at rest for months. Every one was found by accident, late, by someone reading the code for another reason. A suite that cannot fail is worse than no suite: it converts absence of coverage into a false claim of coverage. ## What to look for - **Assertions guaranteed by construction.** The canonical shape: asserting two fields agree when both are set from the same source. `speaker_label` and `track_owner_id` are both stamped from the same `Track`, so asserting they match tests nothing. - **Fixtures too small to reach the code under test.** Anything below a guard's threshold, shorter than a window, or fewer items than a batch size. - **Tests named for a behaviour they do not exercise** — the name says "filter", the fixture never reaches the filter. - **Unit tests of a function that is correct in isolation and wrong in composition.** #432 exactly. - **Missing invariants.** Not a broken assertion, an absent one: nothing asserted encryption at rest, so nothing noticed its absence. - **Over-mocking**, where the mock encodes the assumption the test should be checking. ## Method **Mutation is the only proof.** For each test claiming to guard a behaviour, break that behaviour and confirm the test goes red. A test that stays green is either testing something else or nothing. This is slow and it is the entire point — every instance above would have been caught by one mutation. Prioritise by blast radius: tests guarding data destruction, attribution, authentication and money-shaped invariants first. ## Acceptance criteria - [ ] Every test guarding a data-loss, attribution or auth path has been mutation-checked and seen to fail against the broken behaviour - [ ] Tests found to be vacuous are fixed or deleted — **a deleted vacuous test is an improvement**, since it removes a false claim - [ ] Missing invariants identified during the audit are added as real assertions - [ ] The recurring patterns are written into `CLAUDE.md` or a testing guide, so the seventh instance is prevented rather than found - [ ] Where a test cannot be made meaningful without a larger fixture, that fixture is built or the gap is documented explicitly ## Related #433 — the synthetic-audio harness exists partly because the transcription path had no way to be tested for real. Its design already assumes this discipline: every guard it adds is reverted and confirmed to fail first.

claude-bot added this to the v4.6.0 - Release Readiness: Review, Simplification & Documentation milestone

2026-08-29 18:19:25 +00:00