Test Audit

Test Audit

Three modes, one value bar. Authoring mode gates every new or changed test at write time. Audit mode runs focused sweeps for tests that re-assert source, duplicate stronger proof, couple behavior to implementation, or keep test-only production seams alive. Campaign mode reviews every test file with a per-test verdict when the maintainer asks to prune the whole suite; see Campaign mode. Optimize for confidence, not deletion count. Land one coherent batch per PR; continue broad audits as separate follow-ups.

The repository's own rules live in the "Testing" section of AGENTS.md. This skill is the procedure for applying them; when the two disagree, AGENTS.md wins and this file needs an update.

Authoring gate

Before adding a test, answer four questions. A missing answer means do not add it yet:

1. What observable behavior, invariant, or independent contract does it protect?
2. What credible regression makes it fail?
3. Why does existing coverage not already catch that failure? Each contract has one primary owner at the strongest boundary (usually the service or the MCP tool handler, not the repository or a private helper). Another layer needs its own distinct risk. Prefer extending a table-driven case or a shared factory (ga4-test-fixtures.ts, tool-test-support.ts) over a near-duplicate test, and consolidate duplicated setup in the same change.
4. Does it need a production seam (export, flag, wrapper, injection hook) that no production caller needs? If yes, test at the real boundary instead. Grep the whole repo for callers, not just src/; scripts/ counts.

Then check the test against every junk pattern; a match fails the gate unless the retention bar names the contract it independently guards. A test that would break under behavior-preserving refactoring is asserting implementation, not behavior; rewrite it at the owning boundary before landing it.

Bug regression tests must fail on the pre-fix code for the intended reason and pass after the fix. A regression test that never demonstrably failed proves the mock, not the fix. One regression at the owner boundary covers the bug; do not replay it at every layer it crosses.

Junk patterns

The shared checklist for both modes. The authoring gate rejects a new test that matches one; audits hunt for existing tests that do.

Repo-specific (each is a AGENTS.md rule):

• per-test await import() or vi.resetModules() without a comment explaining which module-level state must reset;
• a production class re-declared inside a test instead of imported from a leaf module (ga4Errors.ts, gscErrors.ts are the pattern);
• mockReset / mockClear ceremonies in beforeEach (Vitest clearMocks is already on);
• fixtures longer than the assertions that read them, or inline copies of a shape that already has a factory;
• Zod schemas or third-party libraries re-tested through the module under test;
• an output-schema round-trip repeated in every happy path;
• Drizzle builder chains mocked (select().from().where() with vi.fn() chains); repositories are tested through services or real SQL;
• argument forwarding to a mock asserted when the mapping is not the contract (billing params and telemetry events are the contract; internal helper arguments are not).

General:

• assertion-free coverage probes;
• self-comparisons, and expected values produced by the helper or renderer under test;
• copied fixtures, inventories, manifests, or export lists;
• exact source, import, or string greps;
• private predicate or call-shape tests duplicated at real boundaries;
• duplicate invocations of the same contract;
• tests whose only purpose is preserving test-only exports or wrappers;
• dead production code whose only callers are tests;
• mocks that implement the asserted behavior, or one identical mock standing in for different APIs;
• negative controls that pass for an unrelated reason, such as a denial from a different guard or a rejection the production path never reaches;
• names or fixtures that promise more than the input exercises.

Value bar

Tests justify their maintenance cost by protecting behavior, a credible regression, or an independently meaningful contract. In an audit, an existing test that must change for behavior-preserving source reorganization is suspect, not automatically deletable; the authoring gate still rejects new ones.

Before judging a candidate, read the complete test and its production owner, the entry point, callers, callees, sibling implementations, overlapping tests, and relevant git history. When the test claims dependency-backed behavior (DataForSEO response shapes, Autumn, Better Auth, Drizzle), inspect the dependency source or types directly.

Discovery

Keep discovery read-only and report evidence before editing. For broad scope, run parallel read-only lanes (Explore subagents work well):

• src/server/mcp/ and src/server/mcp/tools/;
• src/server/lib/, src/server/auth/, src/server/billing/, src/server/workflows/, src/serverFunctions/;
• src/server/features/;
• src/client/, src/shared/, src/lib/, src/types/, scripts/, web/tests/, plus a cross-cutting grep sweep for the repo-specific patterns above.

Prefer a few high-confidence candidates over a large speculative inventory.

Campaign mode

Use only when the maintainer explicitly asks for a suite-wide prune. Run the pipeline in two phases with disjoint file slices so agents never edit the same file:

1. Review, read-only. Split every *.test.ts into 6-8 slices balanced by line count and grouped by directory. One reviewer per slice reads each test file and its production owner completely and writes a report with a table per file: test name, DELETE / MERGE / KEEP, one-line evidence naming the code change that would fail it or the test that already owns it. The report also lists test-only production seams (with callers checked in src/, scripts/, tests/badseo/, web/), test support that becomes unused, whole files to delete, and a "risky calls" section. Reviewers do not edit.
2. Apply, per slice. One applier per slice re-verifies each DELETE and MERGE against the source before acting, keeps anything whose evidence does not hold, folds MERGE rows into table-driven siblings, deletes newly unused fixtures and mocks, and skips every risky item, listing it back for the maintainer. If a deletion's justification cites a test in another slice, the applier confirms that test still exists at the end of its run.

Rules that hold throughout a campaign:

• The only permitted non-test edit is removing an export keyword from a helper whose remaining callers are all tests. Knip fails ci:check on the orphaned export otherwise. Never remove parameters, delete branches, or change behavior in the same change, even when a test looks like the only reason the code exists; report those as follow-ups.
• Route modules under src/routes/ and barrel files keep their exports.
• Use pnpm exec prettier --write <files>; pnpm format:write ignores arguments and formats the whole repository.
• After all slices land, run the full suite in normal order and with two or three --sequence.shuffle.tests --sequence.seed=<n> runs. Removing reset ceremony can surface latent order dependence; fix it by setting the mock's default in beforeEach, never by restoring the ceremony.