Skip to content

Reviewing AI-Written Tests

An agent that wrote the code and then wrote the tests has graded its own homework. The suite will be green, coverage will look respectable, and the assertions will encode whatever the implementation already does — including the bugs. Reviewing AI tests is not "are there tests?" It is "would this suite fail if the behavior were wrong in a way that matters?"

The review question

For every non-trivial test file the agent added or changed, pick one assertion and ask:

If I broke the production code in the way this test claims to protect against, would this assertion fail — or would the mocks still smile?

If you cannot answer "fail," the test is documentation of the current call graph, not a safety net. Reject or rewrite it before you trust the PR.

Mock-only tests

Agents default to mocking everything they can import. The result is a suite that proves the test author wired mocks correctly.

Smell:

typescript
it("calculates proration", async () => {
  db.query.mockResolvedValue({ rows: [{ amount: 100 }] });
  await calculateProration(sub, changeDate);
  expect(db.query).toHaveBeenCalledWith(expect.stringContaining("SELECT"));
  expect(billing.charge).toHaveBeenCalledWith(userId, 42);
});

Nothing here asserts that 42 is the right charge. Swap the formula for return 0 and, if the mock still returns what the test planted, you may still get green depending on how the code is structured — or the test only fails because a call count changed, not because money was wrong.

What you want instead: assert on the observable outcome (returned value, DB row, HTTP response body, emitted event payload). Mock only at the true boundary: third-party HTTP, clock, email provider — and list those boundaries in CLAUDE.md so the agent stops inventing mock policies per session. See Quality Prompts for the mock-policy pattern.

SignalLikely meaning
toHaveBeenCalled / toHaveBeenCalledWith as the only assertionTesting the mock, not the behavior
Every collaborator mocked, including in-process modulesIntegration was avoided; bug will appear in staging
Fixture data identical to hardcoded expected output with no derivationCircular: test restates the stub
Snapshot of an entire response with no named fields checkedDrift will be accepted as "update snapshot"

Assertion weakness

Even without mocks, agents write soft assertions:

WeakStrongWhy the delta matters
expect(result).toBeDefined()expect(result.amountCents).toBe(1337)Defined ≠ correct
expect(res.status).toBe(200)Status and body shape and side effectHappy status hides wrong payload
expect(errors).toHaveLength(0)Named invariant: "duplicate invoice id rejected"Empty array is not a property
expect(fn).not.toThrow()Specific error code / message for the negative case"Doesn't throw" ignores wrong success

Force specificity in the prompt: "assert exact amounts, IDs, and error codes from the spec — no toBeTruthy on money."

Characterization vs behavior

Two legitimate test intents get conflated:

KindPurposeWhen to use with agents
Behavior / acceptanceSpec says X must happen; test fails if X doesn'tNew features, clear acceptance criteria
CharacterizationPin current behavior so a refactor can't drift silentlyLegacy you don't yet understand; bug you haven't decided to fix

Agents love characterization dressed as behavior: they observe what the code does and assert it. That is useful for a legacy refactor and poisonous for a new feature — it freezes the agent's improvisation as "requirements."

Rule: if the PR claims to implement a spec, every new test must trace to a spec line. If it can't, either the test is characterization (label it) or the agent invented a requirement (delete it or promote it into the spec). See Keeping Specs Alive.

Review procedure (10 minutes)

  1. File list — which test files changed? Any production-only change with zero test delta on a behavior PR is incomplete.
  2. Mock inventory — grep for mock, stub, fake, vi.fn, jest.fn. Every mock needs a one-line justification against the repo mock policy.
  3. Edge-case grep — take the edge cases named in the spec; search the test files for each. Missing name = missing coverage, regardless of % lines covered.
  4. Mutation thought experiment — for the highest-risk behavior, mentally invert it. Does a test fail?
  5. Builder contamination check — if the same session wrote code and tests, schedule a second-agent attack or rewrite tests in a fresh session from the spec only.

Weak vs strong test-review prompt

Weak:

text
Are the tests good?

Strong:

text
Audit test/ for the diff on branch feat/proration against
docs/specs/invoice-proration.md.

For each new or changed test:
1. Name the spec criterion it protects (or mark CHARACTERIZATION).
2. List mocks used and whether each is an approved boundary.
3. State the strongest assertion (exact value / error code).
4. Propose one code mutation that should fail the test — if none
   exists, mark the test as WEAK and rewrite it.

Do not praise coverage percentages. Output a table:
file · verdict (keep/rewrite/delete) · reason.

Reject criteria

Reject the PR (or the test portion) when any of these hold:

  • [ ] Primary assertions are mock call counts / call args only
  • [ ] Spec-named edge cases absent from the suite
  • [ ] Agent deleted or weakened an existing regression to get green
  • [ ] New feature tests were written after reading the implementation in the same session, with no independent acceptance suite
  • [ ] Snapshots updated with no explanation of what behavioral change they encode

Green coverage that fails these checks is worse than no tests: it creates false confidence. That failure mode is catalogued under Hallucination and False Confidence.

A field manual for AI-native software engineering.