Test Audit

Test Audit

A test suite has one job: go red when the code is wrong. This skill looks for the places it cannot do that.

Repo profile. The engine here is repo-agnostic. Every repo fact — scan surface, assertion dialect, per-rule severity, how to run a subset of tests — comes from an adapter at .claude/test-audit.config.ts in the consuming repo. Arc is the reference implementation.

Usage

# One PR / working tree: what did this change do to the suite?
bun <plugin_root>/skills/test-audit/scripts/audit.ts diff [--base origin/main] [--json]

# Whole tree: which tests verify nothing? (advisory backlog, never a gate)
bun <plugin_root>/skills/test-audit/scripts/audit.ts scan [--json] [--rule <id>]
bun <plugin_root>/skills/test-audit/scripts/audit.ts scan --write-baseline

# Turn a sweep into a handful of actionable issue drafts (prints; never creates)
bun <plugin_root>/skills/test-audit/scripts/audit.ts issues [--rule <id>] [--json]

# Calibration: what would these rules have done to real merged history?
bun <plugin_root>/skills/test-audit/scripts/replay.ts [--limit 300] [--rule <id>] [--verbose]

issues groups findings by (rule × owning unit), gives each draft the four blocks a work item needs (问题 / 证据 / 方案 / 验收) and a test-audit-key marker for upsert, and rolls the long tail of one-off groups into a single per-rule list. On arc that turns 108 findings into 17 drafts. It prints and exits — creating issues is a side effect on a shared surface and stays an explicit act by whoever ran it.

When invoked — pick the mode, then read the output

There are four modes and they answer different questions. Choose from what was actually asked; do not run all of them.

Reading the output

1. Separate blocking from advisory before saying anything. They mean different things: block = this change makes the suite lie; warn = worth a glance. Reporting "12 findings" without that split is noise.
2. Quote the witness, not just the count. Every finding carries a machine-checkable reason. path:line plus the witness is the whole value — a bare count cannot be acted on or disputed.
3. A finding is a claim about a test, not a verdict on the author. Several rules are deliberately tuned for recall over precision (see the bar section below), so some advisory hits are legitimate code. Read the site before asserting it is broken.
4. Never edit .claude/test-audit-baseline.json to make a run clean. The baseline is accepted debt, rewritten only when a human decides to accept the current state (scan --write-baseline). Silencing a new finding by adding its key is indistinguishable from fixing it — and it is the one move that turns this whole tool back into decoration.
5. Never raise a severity to make a point. Promotion to block requires a replay run, and it is the repo owner's call, not the reviewing agent's.

In arc the diff mode is also wired into the verification gate as the testQuality check (.claude/verify/checks/check-test-quality.ts), so pre-pr / pre-merge carry it automatically.

The five ways a suite fails

Rows 2 and 3 are honest gaps, not oversights. They are listed so nobody reads a green run as "the suite is fine".

Rules

The three disciplines this skill is built on

1. Parse, don't regex

A regex cannot express "the try block has no guard", because [^}]* cannot cross a nested brace. A predecessor rule of this shape scored 27% precision over a 22-file hand-labelled sample; 13 of its 22 false positives came from the single fact that it never looked for a guard token at all, and it silently missed any unguarded try whose body contained a {.

catch-swallow reads the actual try block and honours five guard shapes: a dialect guard token, an assertion in the try, an assertion after the try/catch, an assertion before it naming the same callee, and the two-valid-paths idiom. Re-measured against the same hand-labelled sample: 14/14 known false positives eliminated, 6/6 known true positives retained.

2. Every rule ships an accept fixture

fixtures/<rule>/bad.fixture.ts must be flagged; fixtures/<rule>/good.fixture.ts must be flagged by no rule. Diff rules ship a before / bad-after / good-after triple. test/rules.test.ts enforces this for every registered rule, so a rule cannot be registered without one.

This is the accept-path rule pointed at the auditor. A rule that flags everything satisfies every reject-only test it will ever be given — "catches the bad fixture" and "also catches correct code" are the same colour without an accept fixture. It works: the first run of no-assertions failed its own bad fixture, because its helper escape-hatch was exempting the code under test.

When real history exposes a false positive, it goes into the good fixture. The accept fixtures currently pin: toMatch→toBe tightening, assertions removed because the feature was removed, afterEach cleanup, capability probes, sentinel-after-try, retitles, void-guard accept halves, node:assert imports, and hand-rolled throw assertions.

3. A rule may only block once measured on real history

scripts/replay.ts walks real commits, feeds each (parent, head) pair through the same code path the gate uses, and reports what the gate would have done. Run it before promoting any rule to block, and again whenever a rule changes.

The bar is two-sided, because finding a real problem and being worth blocking are separate questions. A rule that stops honest work is a tax on every honest change, and the first thing anyone does with a gate that cries wolf is route around it — at which point it protects nothing.

Measured on arc's last 300 test-touching commits:

Shipped blocking set: 0.0% of real commits. Whole-tree scan never gates.

The bar is different for warn than for block

no-assertions sits at 76.5% precision, and that is deliberate: it is the point of maximum precision reachable without giving up a single true positive. Two further exclusions were implemented, measured, and reverted, because each bought fewer false positives than it cost true ones:

• "this test is setup for later tests" (assigns a describe-scoped let that an asserting test reads) — removed 3 false positives, silenced 3 confirmed true positives. server = await createAuthServer() at the top of an ordinary test is the same shape.
• "a local helper that throws is asserting" — would catch a polling waitForTree(), but also catches every setup factory that validates its input; removed 1 false positive, silenced 2 true ones.

Both reverts are the same judgement: for a rule that only ever writes to an advisory backlog, a false negative is the more expensive error. A missed vacuous test stays hidden indefinitely; a false positive costs a reviewer thirty seconds. A rule that blocks inverts this, which is why the blocking set is held to a near-zero false-positive rate instead. Pick the operating point from what the rule is allowed to do, not from a single number.

Adapter

import { type Adapter, BUN_VITEST_DIALECT } from "<plugin_root>/skills/test-audit/scripts/adapter.ts";

const adapter: Adapter = {
  root: "",                                  // filled in by the engine
  testGlobs: ["**/*.test.ts", "**/*.test.tsx"],
  excludes: ["nested-checkouts/", "coverage.test.ts"],
  dialect: BUN_VITEST_DIALECT,               // or override per framework
  runTests: (files) => `bun test ${files.map((f) => `./${f}`).join(" ")}`,
  testsForSource: (source, all) => /* tests that plausibly cover it */ [],
  rules: { only: "block", "no-assertions": "warn" /* … */ },
  baseline: ".claude/test-audit-baseline.json",
};
export default adapter;

Scan the whole tree plus explicit excludes — never a hand-listed set of directories. A predecessor hardcoded five and, two months on, was blind to 844 of the repo's 3392 test files, including all of runtimes/. One of its rules scanned a single directory that had since been cleaned, reported 0, and looked healthy while 51 instances of the same defect sat in trees it never opened.

dialect.tracksFloatingExpects is a fact about the runner, not a preference. Bun settles floating expect(...).rejects promises and fails the test anyway; jest and vitest do not. Verify with a throwaway fixture before setting it. A predecessor rule flagged 43 unawaited sites as P0 "vacuous pass" on a premise nobody had run — measured on bun 1.3.14, all three variants (resolve-instead-of-reject, late resolve, wrong error type) go red.

Adding a rule

1. Write it in scripts/rules.ts (static) or scripts/diff.ts (diff), keyed on content, never on a line number — a finding whose identity moves when a comment is added above it re-opens as "new" on every reformat.
2. Emit a witness: machine-checkable evidence for the claim. A finding without one cannot be audited later without re-auditing the rule.
3. Ship the fixture pair. The harness fails without it.
4. bun test ./test/rules.test.ts.
5. bun scripts/replay.ts --limit 300 --rule <id> --verbose, and read the hits. Default to warn. Promote to block only with a demonstrated true positive and a near-zero false-positive rate on real history.

Cost