# Test Audit

> Ruthlessly audit a repo's tests — cut the ones that prove nothing, rewrite the ones checking the wrong thing, keep only what earns its place.

- Skill: `caneff/test-audit` (Agent Skill, multi-file: 13 files)
- Install (CLI): `npx skillmds@latest add caneff/test-audit`
- Raw SKILL.md: https://api.skillmd.com/api/skills/caneff/test-audit/raw
- Safety review: pending
- Works with: Claude Code, Claude.ai, OpenAI Codex
- Category: Security
- Author: caneff (https://skillmd.com/u/caneff)
- Updated: 2026-09-22
- Page: https://skillmd.com/skills/caneff/test-audit

---


Sweep the tests in scope and sort each one into Cut, Rewrite, or Keep. A suite
full of tests that can't fail, or that fail for reasons no user cares about,
trains the reader to distrust the suite — when half the tests go red for
nothing, a real failure gets waved through. Ruthless sorting is what buys the
survivors their authority.

The default deliverable is a **report**, not applied edits. The sweep writes a
machine-readable findings log and a grouped HTML summary; it touches no test.
Applying the changes is a separate, opt-in step the user asks for by name.

This is the judgment pass: the smells only reading can find. `audit.py` and
`audit.mjs` in this skill's directory are pass one — a mechanical scan for
the syntactically detectable smells (assertion-free tests, tautologies,
mock-the-world, interaction-only assertions, empty/skipped tests). `audit.py`
covers pytest; `audit.mjs` covers vitest and node:test. Run both first; their
combined candidate list feeds the judgment sweep below instead of starting
from a blank page.

**JS/TS reach.** `audit.mjs` recognizes vitest and node:test — nothing else.
A file identifies as **vitest** by importing `vitest`, or by naming tests
(`describe`/`it`/`test`) and calling `expect`. A file identifies as
**node:test** by importing `node:test`, and only that way — `assert.*` alone
is not a signal, so a node:test file that reaches its runner some other way is
out of reach. A file it can't identify as one of those two is skipped, never
flagged, so a homegrown or non-standard harness doesn't flood pass one with
false assertion-free findings. Playwright
`.spec` files are out of scope — they're e2e/visual specs, not unit tests, and
auditing them by this yardstick would misjudge them. jest, mocha, ava, and
chai are likewise out of scope. A missing `@babel/parser` prints a `run npm
ci` message and exits — bootstrap with `npm ci` in
`~/.agents/skills/test-audit/` once.

**Gate mode.** `audit.py --gate [path]` and `audit.mjs --gate [path]` are the
build-gate form of pass one: they report **only** smell 1 and skip anything
under a `fixtures/` directory. Smell 1 alone gates because an assertion-free
test cannot fail at all — a machine can call that a defect without reading
anything. The other four still run and still fail when the behavior breaks, so
blocking a merge on one costs more than it buys; they stay report-only.

Three exit statuses: **0** clean, **1** hollow tests found, **2** unable to
check. Only gate mode ever returns 1 — a report never fails a build — but 0 and
2 mean the same thing in both modes. A path that does not exist gets 2, never 0
— a gate that reports success while it scanned nothing is a lie, and a typo in
a repo's wiring would otherwise make that repo's gate permanently green (#685).
`audit.mjs` also exits 2 when `@babel/parser` is missing.

The `fixtures/` exemption matches a directory of that name at any depth, so the
gate prints how many findings it suppressed (`N assertion-free finding(s)
suppressed under fixtures/.`) even when it passes — otherwise a repo could park
hollow tests under a directory it named `fixtures` and never notice.

Wire it into a repo by adding it to that repo's local gate (`git config
land.testcmd`). In this repo that is `test-audit/assertion-free-gate.test.sh`,
picked up automatically by `tests/all.sh`'s `*.test.sh` discovery.

**The JS gate's remaining blind spot.** A file whose every test is
assertion-free has no assertion to be recognized by, so the runner *import* is
what identifies it (#685) — which leaves one case uncovered: a vitest file run
in globals mode, importing nothing, whose every test is assertion-free. It is
invisible to both signals. `audit.py` has no equivalent limit; it audits any
`test_*.py` outright.

**Name collision.** This is `test-audit` — test-*file* quality. It is not
`ponytail-audit` (production-code over-engineering) or `skill-audit`
(skill-*file* quality).

## The one test

**A good test fails for exactly one real reason — a behavior a human would
be upset to see break. A crap test either cannot fail when the behavior
breaks, or fails when nothing a user cares about changed.**

Three buckets, not two — a deleted test can lose the only guard on a real
behavior, so the burden of proof splits differently than a deleted comment:

- **Cut** — proves nothing. Deleting it loses no coverage.
- **Rewrite** — the behavior is real, but the test checks the wrong thing.
  Replace it, don't delete it.
- **Keep** — earns its place.

Default when unsure is **Rewrite, not Cut.** A comment you wrongly cut just
needs re-adding; a test you wrongly cut can open a silent coverage gap that
nobody notices until the behavior it was guarding actually breaks. Only cut
when you're sure the test proves nothing at all.

**The only-test warning.** Before you cut, check whether anything else in
scope touches the same code path. If the test you're about to cut is the
*only* one that exercises that path, it is not a Cut — it's a Rewrite. The
smell that makes it a Cut candidate (mismatch, sensitive equality, whatever)
still needs fixing; it just can't be fixed by deletion.

This warning doesn't rescue a test that structurally cannot fail — a
conditional-logic test with an escape-valve assertion in every branch, or an
assertion-free test, proves nothing under any circumstance. There is no real
coverage there to lose, only-test or not, so it stays a Cut. The warning is
for tests that *can* fail, just for the wrong reason or on the wrong
grounds — a real signal aimed badly, which is worth salvaging.

## Documented intent — the author already answered

Before you Cut or Rewrite a test, read its own comments and docstring. When a
test carries an in-code comment or docstring stating *why* it exists — "a
worked example the property already owns," "a tripwire / deliberate
change-detector," an ADR reference — that is a documented-intent signal, and
evidence the test earns its place. A finding whose objection the test's own
comment already answers is arguing with the author, not naming a defect.

Default such a test to **Keep**. You may still disagree — but then surface it
as the `documented-intent` category, "author declares intent — confirm before
acting," naming the comment you would override, never a cold Cut or Rewrite.

One carve-out: a comment cannot rescue a test that *structurally* cannot
fail — the same limit the only-test warning draws above. A tripwire that *does* go
red when its guarded line changes is not this case; that is
change-detection-by-design, which documented intent moves to Keep. Documented
intent downgrades a judgment-call smell — duplicate coverage,
change-detection-by-design, an equality the author calls deliberate; it does
not resurrect coverage that was never there.

## Load-bearing — never touch

Some files aren't tests, they're scaffolding the tests run on. Leave these
exactly as they are:

- `conftest.py` and other shared fixture/setup files.
- Markers, parametrize tables, and test-config (`pytest.ini`, `tox.ini`,
  `jest.config.*`) — unless a specific marker or parametrize case is itself
  the thing under judgment.
- CI wiring that invokes the suite (workflow YAML, Makefile test targets).

If cutting or rewriting a test would mean touching one of these to keep the
suite green, stop and flag it instead of editing scaffolding to make a
judgment call fit.

## Cut — judgment-pass smells

Each of these needs a human eye on the code beside the test; none is
reliably greppable. For every one, name the *concrete* failure — never a bare
label. "Cannot fail when X breaks" or "fails when Y is refactored though
nothing broke," not "this is a mystery guest."

- **Duplicate coverage across layers.** The same behavior asserted at a unit
  test and again at an integration or e2e test, where the higher layer adds
  nothing the lower layer doesn't already prove faster. Keep the lowest
  layer that owns the behavior; cut the redundant restatement — but only
  when some *other* test in scope already covers the wiring this layer
  uniquely owns. If the duplicate assertion is the only thing proving that
  wiring anywhere in scope, it's the only-test case above — rewrite it down
  to what it actually owns instead.
- **Mystery guest / resource optimism.** The test depends on state it
  doesn't build or show — an ambient fixture, a database row left by another
  test, a file assumed to exist, a service assumed to be up. It fails when
  run order or environment changes, not when the behavior breaks; and it
  can't reliably fail when the behavior *does* break, because the shared
  state may mask it.
- **Eager test.** One test body drives several unrelated behaviors and
  asserts on all of them. A failure tells you "something in here broke," not
  which behavior — collapsing several real reasons to fail into one.
- **Sensitive equality.** Asserting a full object, dict, or blob equal
  because equality was easy to write, when only one or two fields are the
  actual behavior under test. Fails whenever an unrelated field changes
  (an id, a timestamp, a field nobody asked this test to guard).
- **Name/behavior mismatch.** The test's name promises one behavior; its
  body checks something else. Readers trust the name over the assertions —
  so this doesn't just fail to prove the named behavior, it actively hides
  that nothing proves it.
- **Library-default test.** The assertion only proves a language or library
  default works (a dataclass default argument, an ORM's auto-increment id,
  a framework's built-in validation). Nothing this codebase wrote can break
  it.
- **Conditional test logic.** An `if`/`else` or `try`/`except` inside the
  test body that changes what gets asserted depending on a runtime
  condition. Whichever branch runs, the test finds a way to pass — it can't
  fail no matter which branch the real behavior takes.
- **Flakiness-by-construction.** Real `sleep`, unseeded randomness, or
  wall-clock time decides the outcome run to run. It can fail when the
  behavior is correct (bad luck) and pass when the behavior is broken (good
  luck) — the opposite of "fails for exactly one real reason."

## Rewrite

Everything in the Cut list above lands here instead of Cut whenever the
behavior is real and worth guarding — which is most of them. A test earns
Rewrite, not Cut, when:

- it is the only test on the code path it (badly) covers, or
- the smell is fixable by changing *what* the test asserts or *how* it
  builds its inputs, without abandoning the behavior.

Rewrite the assertion, the setup, or the isolation — not the intent. If you
cannot state what the rewritten test should assert instead, you have not
finished judging it; don't apply a placeholder rewrite.

## Keep

A keeper earns its place against Khorikov's four pillars — regression
protection, refactoring resistance, fast feedback, maintainability — by:

- asserting an observable outcome, not an implementation detail
- failing for exactly one reason
- surviving an internal refactor of the code it covers
- covering the behavior at the lowest layer that owns it

For what a good test looks like when you're writing one rather than judging
one, see `python-testing-patterns`'s Test Quality Gate — this skill applies
that same standard as a sweep instead of restating its positive patterns.

**The highest-value, lowest-visibility find** is the interaction-only test
that is green today: it asserts a mock was called with certain arguments and
nothing about the outcome. It looks like coverage and destroys refactoring
resistance — the moment the implementation changes shape while behavior
holds, it goes red for a reason no user cares about. Call these out first.

## The audit, worked

```python
def test_create_user_returns_expected_user():
    service = UserService()

    user = service.create_user(name="Ada", email="ada@example.com")

    assert user == {
        "id": 42,
        "name": "Ada",
        "email": "ada@example.com",
        "created_at": "2026-08-13T00:00:00Z",
    }
```

Sensitive equality: `id` and `created_at` aren't this test's business, but
locking them into a literal means the test fails the moment the id sequence
advances or a millisecond of clock drift shows up — for a reason no user
cares about. The behavior worth guarding — a created user carries the name
and email you gave it — is real. Rewrite:

```python
def test_create_user_returns_expected_user():
    service = UserService()

    user = service.create_user(name="Ada", email="ada@example.com")

    assert user.name == "Ada"
    assert user.email == "ada@example.com"
```

Two fields instead of four. `id` and `created_at` are still generated —
just no longer this test's problem.

## Verify against the fixture

`~/.agents/skills/test-audit/fixtures/` carries five files spanning the Cut/
Rewrite/Keep buckets and the mechanical smells above (pytest and vitest).
`~/.agents/skills/test-audit/fixtures/answer-key.md` has the expected pass-one
candidate list and the pass-two bucket for each test. Running this skill over
`~/.agents/skills/test-audit/fixtures/` should reproduce that table.

## Run

1. **Start clean, scope.** Confirm a clean working tree first (`git status`).
   Then scope: audit `$ARGUMENTS` if given; with no argument, scope defaults
   per `~/.agents/skills/all-audits/SKILL.md`'s Scope section. Either way, skip vendored, generated, and dependency
   trees (`node_modules`, `dist`, `.venv`, build output, lockfiles) and any
   `worktrees/` tree — a git worktree mirrors the whole repo, so scanning it
   multiplies every finding once per worktree — and leave load-bearing
   scaffolding alone (see above). `audit.py` prunes these directory names
   itself; the same skip applies to the judgment sweep.

2. **Pass one — run both mechanical scanners.** `python3
   ~/.agents/skills/test-audit/audit.py <scope>` scans pytest files; `node
   ~/.agents/skills/test-audit/audit.mjs <scope>` scans
   vitest and node:test files. Run both and concatenate their output into one
   `file:line: <smell>` candidate list for the five mechanically detectable
   smells (assertion-free, tautology, mock-the-world, interaction-only
   assertion, empty/skipped). This is a candidate list, not a verdict — every
   line still needs the judgment pass below to confirm it and assign a
   bucket.

3. **Pass two — sweep every test, not a sample.** Walk the test files in
   scope and read each test beside the code it covers; judgment needs both,
   pass one's candidates tell you where to look first, but the sweep still
   covers every test in scope, not just the flagged lines. On a large tree,
   fan the sweep across subagents by directory — but every test in scope
   gets judged, never sampled.

4. **Judge each into one bucket** against the one test, checking the
   only-test warning and the documented-intent rule before every Cut or
   Rewrite. For each Cut or Rewrite, write the one-line concrete failure it
   names — "cannot fail when X breaks" or "fails when Y is refactored though
   nothing broke." If you can't name it concretely, you haven't finished
   judging — don't bucket it yet.

5. **Write the findings log and render the summary — the default deliverable.**
   Every judged Cut/Rewrite/Keep goes in the log (see below). Touch no test.
   The mechanical scanners' `file:line: <smell>` candidate list from pass one
   still prints to the terminal — it is a scan, not the log.

## Write the log and render the summary

See `~/.agents/skills/all-audits/harness/AUDIT-RUN.md` for the shared
write-and-deliver step (tmpdir resolution, `findings.jsonl` + `report.html`,
opening, and the final print). This skill's own bucket names, category
vocabulary, and metabar:

- **Log** — one JSONL line per judged test. `bucket` is `cut` / `rewrite` /
  `keep`. `category` is the smell that named it — `duplicate-coverage`,
  `mystery-guest`, `eager`, `sensitive-equality`, `name-mismatch`,
  `library-default`, `conditional-logic`, `flaky-by-construction`, `tautology`,
  `interaction-only`, `documented-intent`. A rewrite carries `before`/`after`;
  a duplicate-coverage cut carries `owner` (the stronger test's `file:line`);
  a `documented-intent` row carries the comment it defers to in
  `extra.author_intent`.
- **Summary** — the verdict, the `N judged · C cut · R rewrite · K kept`
  metabar, and the findings grouped by bucket then category with counts. No
  per-test cards. Call out the standouts in a `vt-callout`: the interaction-only
  tests that are green today, the highest-value find.

## Applying the changes (opt-in)

Only when the user asks to apply — see `~/.agents/skills/all-audits/SKILL.md`'s
"Opt-in edits" section for the shared opt-in contract
(reviewable PR on its own branch, never a direct commit). Start from a clean
working tree.

Apply: delete the cuts. Rewrite each Rewrite to assert the real behavior
correctly — new assertions, isolated setup, mocked time, whatever the smell
called for. Leave every Keep untouched.

Verify before the PR: run the repo's own test command. A test you misjudged
as crap should turn the loop red here, before a human ever reviews the diff —
not after it merges.