test-quality-audit · diff
git:20261002.18638a5 to git:20261002.6d35a27
52 added, 83 removed. Audit A to A.
---
name: test-quality-audit
description: Judge whether tests catch realistic regressions and recommend what to keep, change, or remove.
---
# Test Quality Audit
- Judge tests by signal, not volume.
-
- The core question is: **What realistic bug or regression would this catch?**
-
- Call a test weak only when evidence shows that its signal does not protect a meaningful contract or justify its maintenance cost. If missing context prevents that judgment, state the uncertainty rather than assuming the test has little value.
-
- ## Goal
-
- Identify the contracts and realistic regressions tests protect, and where their assertions, isolation, or duplication provide misleading evidence or unnecessary maintenance cost.
-
- Do not ask for more tests by default. Prefer a smaller suite that fails for the right reasons.
-
- ## Success Criteria
+ A test earns its place by failing on a realistic regression of a contract: behavior that users or callers rely on. Judge every test by that standard, not by coverage, count, or how it was written. Prefer a smaller suite that fails for the right reasons, and do not ask for more tests by default.
- - Each judgment is tied to realistic bug-finding value.
- - Meaningful behavior currently protected by the tests is identified.
- - Weak, redundant, tautological, brittle, or overfit tests are named concretely.
- - Recommendations classify tests as **Keep**, **Fix**, **Cut**, or **Add**.
- - The highest-value next changes are small and behavior-oriented.
+ Call a test weak only when the evidence shows it protects no contract or costs more to maintain than it protects. If missing context prevents that judgment, say so instead of assuming the test has little value.
- ## Context Budget
+ ## What to read
- Do not audit tests in isolation when code is available. Start with the tests under review, then inspect only the code and nearby tests needed to judge signal:
+ Start with the tests under review, then read only what you need to judge them:
- - implementation under test
- - public API or user-visible behavior
+ - the implementation under test and its public or user-visible behavior
- important branches and failure paths
- - fixtures, mocks, factories, snapshots, helpers
+ - fixtures, mocks, factories, snapshots, and helpers
- existing tests for the same behavior
- - PR intent, bug report, or known regression when available
- - relevant test changes and the commands, selection rules, and prerequisites that determine whether those tests run
+ - the PR intent, bug report, or known regression when available
+ - the commands, selection rules, and prerequisites that decide whether the tests run
- external boundaries: database, API, auth, queue, filesystem, time, randomness, network
- Use the minimum code and test context needed to judge signal. Continue reading only when a test’s claimed behavior, real dependency, risky branch, or nearby stronger coverage is unclear.
+ Keep reading only while a test's claimed behavior, a real dependency, a risky branch, or nearby stronger coverage is unclear.
## Method
- For each test or test group, answer:
+ For each test or group of tests, answer:
- 1. What behavior does it claim to protect?
- 2. What behavior does it actually protect?
- 3. What realistic bug would make it fail?
- 4. Would that bug matter?
- 5. Could real behavior break while this still passes?
- 6. Could a harmless refactor break this?
- 7. Is the only realistic failure reverting the change that introduced this test? If so, it documents a commit, not a rule.
- 8. Is this the right level: unit, integration, contract, end-to-end, or none?
- 9. Did the claimed validation actually run this test under the relevant conditions?
+ 1. What contract does it claim to protect?
+ 2. What does it actually protect?
+ 3. What realistic regression would make it fail, and would that regression matter?
+ 4. Could the contract break while the test still passes?
+ 5. Could a harmless refactor break it?
+ 6. Is the only realistic failure reverting the change that introduced it? If so, it documents a commit, not a contract.
+ 7. Is it at the right level: unit, integration, contract, end-to-end, or none?
+ 8. Did the claimed validation actually run it under the relevant conditions?
- When tests changed, compare the protected behavior before and after: what realistic failure would the previous test catch that the replacement now accepts? Inspect changed assertions, fixtures, mocks, snapshots, skips, and configuration together. Establish whether lost protection follows an authorized contract change or a demonstrated error in the old test. A changed interface alone does not authorize dropping its failure cases.
+ ### Changed tests
- When behavior is removed, tests deleted with it are not lost protection. Check instead that the remaining suite still covers what depended on the removed area, and do not recommend replacing deleted tests with tests of absence.
+ Compare what the tests protected before and after the change: which realistic regression did the old test catch that the new one accepts? Read changed assertions, fixtures, mocks, snapshots, skips, and configuration together. Lost protection is acceptable only when the contract changed by decision or the old test was shown to be wrong. A changed interface alone does not justify dropping its failure cases.
- For an important claim whose sensitivity is uncertain, use existing failing-before evidence or a focused check against a known broken version when practical. If a temporary defect would resolve that uncertainty, use an isolated disposable copy, preserve the test, restore the copy afterward, and report the observation. Do not mutate the reviewed checkout or require broad mutation infrastructure. A passing result against the defect disproves only the protection that experiment exercised.
+ When behavior is removed, the tests deleted with it are not lost protection. Check that the remaining suite still covers what depended on the removed area, and do not recommend tests of absence as replacements.
- ## Classify
+ ### Checking sensitivity
- - **Keep**: protects meaningful behavior and would fail on a realistic regression.
- - **Fix**: useful intent, weak execution. Rewrite around observable behavior or the risky boundary.
- - **Cut**: little bug-finding value, redundant, tautological, brittle, coverage-only, or a test of static content, copy, navigation structure, or non-critical configuration that encodes no compatibility promise.
- - **Add**: behavior that users or callers rely on is untested and a realistic regression is plausible. Do not recommend Add for removals, design exploration, prototypes, static content, copy, or configuration.
+ When it is unclear whether an important test would fail on the regression it claims to catch, use existing failing-before evidence or run it against a known broken version. If introducing a temporary defect would settle the question, do it in a disposable copy, keep the test unchanged, restore the copy, and report what you saw. Do not modify the reviewed checkout or require mutation-testing infrastructure. A test that passes against a defect is disproved only for that defect.
- ## Investigate signals
+ ## Classify
- Treat test patterns as leads, not verdicts. Establish the contract, the realistic regression the test catches or misses, and any concrete maintenance cost before recommending a change:
+ - **Keep**: protects a contract and fails on a realistic regression.
+ - **Fix**: useful intent, weak execution. Rewrite it around observable behavior or the risky boundary.
+ - **Cut**: protects no contract. Includes redundant, tautological, brittle, and coverage-only tests, and tests of static content, copy, navigation structure, or non-critical configuration that encode no compatibility promise.
+ - **Add**: a contract is untested and a realistic regression is plausible. Do not recommend Add for removals, design exploration, prototypes, static content, copy, or configuration.
- - **Absence assertions:** a test that a feature, route, option, menu entry, or default no longer exists protects nothing callers rely on. Cut it. Keep absence only when it is a promised contract in its own right, such as an endpoint not being reachable without authentication or a destructive action being off by default.
- - **Call, count, and ordering assertions:** useful when an interaction is part of the contract, such as dispatching one message for duplicate submissions or following a required protocol sequence. Investigate whether the assertion instead freezes private helper calls while the promised outcome could still fail.
- - **Constants and fixtures:** can provide independent expectations for published formats, protocol values, or compatibility requirements. Distinguish those checks from assertions that compare fixture-controlled values with themselves or merely repeat non-critical configuration. Judge wrappers and getters by the behavior they own, not their size or name.
- - **Snapshots:** can protect a stable, meaningful output contract when changes receive semantic review. Investigate noisy incidental output, unnoticed contract changes, or bulk snapshot updates that accept a regression. Snapshot syntax alone does not make a test weak.
- - **Mocks and other doubles:** can isolate a meaningful contract or control failure conditions. Check whether they replace the behavior under test or conceal broken wiring, persistence, or transformation that the test claims to verify.
- - **Setup and capability selection:** check whether fixtures initialize state, enable features, or supply configuration that the supported workflow does not guarantee. Keep tests of valid prepared states, but do not credit them as proof of readiness or availability. Error-handling tests can be valuable while successful use remains untested or broken.
- - **Execution gaps:** inspect opt-ins, skips, expected-failure markers, and required infrastructure. A suite excluded from the actual validation command cannot support its success claim. Recommend required deterministic coverage in the required command, with visible failure for unavailable prerequisites; keep optional live-service checks explicit.
- - **Calculated expectations:** check whether the oracle is independent of the implementation. Repeating the same flawed calculation can hide a bug; a computed expectation grounded independently in the contract is not automatically tautological.
- - **Overlapping or internal tests:** compare what the tests check before calling them redundant or tied to implementation details. Keep internal tests when they catch important failures that other tests would miss. Flag assertions on private structure when harmless refactors break them or real regressions still pass.
- - **Tests using an old API:** check whether a replaced production API or adapter remains only because tests still use it. Find other callers and check compatibility requirements before recommending removal. If the old API is no longer needed, classify useful tests as **Fix** and recommend updating them to use the replacement while checking the same behavior. An internal API used only by tests may still protect useful behavior; that alone is not a reason to remove it or its tests.
- - **Failure and timing coverage:** examine missing failure paths, sleeps, or uncontrolled timing when they can hide a realistic defect or make results unreliable. Do not demand every edge case merely because it exists.
+ ## Patterns to investigate
- Keep a pattern when it provides useful evidence for the stated contract. Recommend a change only after establishing the weakness, rather than requiring the author to defend a pattern merely because it appears on this list.
+ These patterns are leads, not verdicts. Establish the contract, the regression the test catches or misses, and the concrete maintenance cost before recommending a change. Keep a pattern that gives useful evidence for its contract.
+ - **Absence assertions.** A test that a feature, route, option, menu entry, or default no longer exists protects no contract. Cut it. Keep an absence test only when the absence is itself promised, such as an endpoint being unreachable without authentication or a destructive action being off by default.
+ - **Call, count, and ordering assertions.** Keep them when the interaction is the contract, such as sending exactly one message for duplicate submissions or following a required protocol sequence. Cut or fix them when they freeze private helper calls while the promised outcome could still fail.
+ - **Constants and fixtures.** Keep checks of published formats, protocol values, and compatibility requirements. Cut assertions that compare fixture-controlled values with themselves or repeat non-critical configuration. Judge wrappers and getters by the behavior they own, not by their size or name.
+ - **Snapshots.** Keep them when they protect a stable, meaningful output and changes get semantic review. Investigate noisy incidental output, contract changes nobody noticed, and bulk snapshot updates that accept a regression. Snapshot syntax alone is not a weakness.
+ - **Mocks and other doubles.** Keep them when they isolate a contract or control a failure condition. Flag them when they replace the behavior under test or hide broken integration, persistence, or transformation that the test claims to verify.
+ - **Setup and capability selection.** Flag fixtures that initialize state, enable features, or supply configuration that the supported workflow does not guarantee. Tests of a valid prepared state can stay, but they do not prove readiness or availability. A suite can test error handling thoroughly while the successful path is untested or broken.
+ - **Execution gaps.** Inspect opt-ins, skips, expected-failure markers, and required infrastructure. A suite that the validation command does not run cannot support a success claim. Recommend that required deterministic tests run in the required command and fail visibly when a prerequisite is missing; keep optional live-service checks explicit.
+ - **Calculated expectations.** Check that the expected value is derived independently of the implementation. Repeating the implementation's calculation hides its bugs; an expectation grounded in the contract is not tautological merely because it is computed.
+ - **Overlapping or internal tests.** Compare what the tests check before calling them redundant or implementation-bound. Keep internal tests that catch important failures other tests miss. Flag assertions on private structure when harmless refactors break them or real regressions still pass.
+ - **Tests on an old API.** Check whether a replaced production API or adapter survives only because tests still use it. Find other callers and compatibility requirements before recommending removal. If the old API is no longer needed, classify useful tests as Fix and move them to the replacement while checking the same behavior. An internal API used only by tests may still protect useful behavior; that alone is not a reason to remove it or its tests.
+ - **Failure and timing coverage.** Flag missing failure paths, sleeps, and uncontrolled timing when they can hide a realistic defect or make results unreliable. Do not demand every edge case because it exists.
+
## Output
- Lead with the verdict and highest-value changes. Keep the evidence needed to justify each classification; omit repeated test summaries and generic testing advice.
+ Lead with the verdict and the highest-value changes. Keep only the evidence that justifies each classification; leave out repeated test summaries and generic testing advice.
- Group repeated weaknesses by the mechanism that loses signal or adds maintenance cost. Cite representative examples and identify the affected test groups. Preserve separate classifications when their contracts or evidence differ; shared syntax alone is not a reason to merge findings.
+ Group repeated weaknesses by the mechanism that loses protection or adds maintenance cost, with representative examples and the affected test groups. Keep classifications separate when their contracts or evidence differ; shared syntax is not a reason to merge findings.
Use only the sections that add information:
- - **Verdict**: high-signal, mixed, weak, overfit, under-tested, mostly noise, or good enough.
- - **What is protected**: meaningful behavior currently covered.
+ - **Verdict**: strong, mixed, weak, overfit, under-tested, mostly noise, or good enough.
+ - **What is protected**: contracts the tests currently cover.
- **Biggest problems**: prioritized issues with examples and recommendations.
- **Keep / Fix / Cut / Add**: concrete classifications.
- **Highest-value next changes**: the smallest set of edits that improves confidence.
- ## If Asked to Edit Tests
-
- Only modify tests when the user explicitly asks.
+ If the tests are strong, say so and name the remaining blind spots. Ask only when missing context prevents judging what a test protects.
- When editing:
+ ## Editing tests
- - preserve high-signal tests
- - rewrite weak tests around observable behavior
- - remove tests that are redundant, misleading, brittle, or low-signal
- - add focused regression coverage for important risks
- - run the most relevant test command when available; otherwise explain the next best check
+ Edit tests only when the user explicitly asks. When editing, preserve tests that protect contracts, rewrite weak tests around observable behavior, remove tests that are redundant, misleading, brittle, or protect no contract, and add focused regression tests for important risks. Run the most relevant test command when available; otherwise explain the next best check.
- Derive expectation changes from authorized requirements or evidence that the old test was wrong. Preserve meaningful cases when adapting interfaces or setup. If a corrected test exposes a production defect outside the authorized edit, leave it failing and report the defect; test cleanup does not authorize a production fix or weakening the expectation. State passed, failed, skipped, and unable-to-run outcomes separately.
+ Change an expectation only when the requirement changed or the old test was shown to be wrong. Preserve the protected cases when adapting interfaces or setup. If a corrected test exposes a production defect, leave it failing and report it; test cleanup does not authorize a production fix or a weakened expectation. Report passed, failed, skipped, and unable-to-run outcomes separately.
## Constraints
- - Be honest; do not defend tests because they exist.
- - Tie every judgment to bug-finding value.
- - Do not reward coverage for its own sake.
- - Do not require tests for every line or helper.
- - Do not confuse unit tests with mocking everything.
- - Do not credit a test for having been written first. Test-first process is evidence about how the work was done, not about whether the test protects anything.
- - If the tests are strong, say so and name the remaining blind spots.
-
- ## Stop Rules
-
- Stop when the audit explains what real bugs the tests would catch, what could still break, and the smallest useful improvement. Ask only when missing context prevents judging the tests’ behavior.
+ - Do not defend a test because it exists, and do not credit it for having been written first. Test-first process says how the work was done, not what the test protects.
+ - Do not reward coverage, require a test for every line or helper, or treat unit testing as mocking everything.