Test Code Review Standards Workflow
Apply review standards to tests specifically: assertion strength, isolation, determinism, and readability, with a checklist and anti-pattern list.
Objective
Raise the diagnostic value of tests through review standards that catch hollow assertions, shared state, and hidden nondeterminism before merge.
Environment
- PR review process including test files
- A written test-review checklist
- Reviewer training or pairing capacity
Steps
1. Write the checklist from real failures
Derive checklist items from actual bad tests that shipped: hollow assertions, order dependence, sleep-based waits, shared fixtures, ignored cleanup.
Expected state: A checklist exists where every item cites the failure class it prevents.
2. Review assertions for strength
In review, verify each test asserts the behavior that motivated it: state outcomes over call sequences, specific values over truthiness, error specifics over catch-all.
Expected state: Hollow assertions (expect(true).toBe(true) equivalents) fail review.
3. Review for isolation and determinism
Check the test for hidden shared state, wall-clock dependence, random without seed, and network reach-outs, requiring seeds and fakes.
Expected state: Tests with nondeterminism sources are flagged with the specific fix required.
4. Require failure to be diagnosable
Evaluate what the failure message teaches: when this test fails at 2 a.m., will the diff between got and expected point at the cause?
Expected state: Assertions are written so failure output is actionable without reading the test body.
5. Track the anti-pattern trend
Count checklist rejections per category over time, targeting training at the most frequent offenders.
Expected state: Rejection categories trend downward as the standards internalize.
Expected outcome
Test code held to review standards that prevent hollow and flaky tests at merge, with measurable anti-pattern decline over time.
Verification
Sampled merged tests pass an independent standards audit; the anti-pattern rejection counts decline quarter over quarter.
Tests are code that fails or lies
A weak test is worse than no test: it provides confidence without protection. Reviewing assertions for strength, isolation, and diagnosable failure turns the test suite from a ritual into an instrument.
checklist (item -> failure it prevents):
A1 asserts outcome state, not call log -> refactor-brittle
A2 specific values, not truthiness -> false green
A3 error case asserts message/type -> swallowed bugs
I1 no shared mutable fixtures -> order flakes
I2 no wall-clock/sleep waits -> timing flakes
I3 randomness seeded + logged -> unreproducible
D1 failure message shows got vs want -> 2am debugging
review catch example:
expect(result).toBeTruthy() <- A2: rejects
-> expect(result.totalCents).toBe(850)Assertion strength is the core review question
The single highest-value review pass asks: if the behavior broke, would this test fail, and would its failure name the break? Truthiness checks, call-sequence verification, and catch-all error assertions all pass today while lying about tomorrow.
Diagnosability is a property you review, not hope for
A test that fails with got 850, want 925 teaches the on-call engineer the cause in one glance; a test that fails with assertion failed sends them reading fixture setup at 2 a.m. Writing the failure message you would want to receive is a reviewable standard.