Code Review for Tests: What to Pay Special Attention To

1. Why test code often gets less attention in review

Reviewers naturally focus first on a pull request's production logic, since that's presumably where the actual business risk lies, while the accompanying test code tends to get perceived as confirming evidence that the production logic already works correctly, rather than as a standalone artifact that needs to be scrutinized just as carefully for correctness and meaningfulness.

This perception is deceptive, since a faulty or meaningless test is in a sense more dangerous than no test at all, since it creates the appearance of coverage without actually delivering it, causing a team to falsely feel safe and skip important, manual additional checks it would probably have performed had the missing test coverage been visibly apparent.

2. Substantively checking assertions instead of just rubber-stamping structure

A common, superficial review mistake is checking only whether a test formally exists, is named, and runs, without actually reading what the contained assertions concretely claim and whether that claim actually matches the behavior under test in business terms. A test can be syntactically completely correct and still make a far too weak or simply wrong claim, say by only checking that a function doesn't throw instead of checking that it returns the business-correct result.

A proven review question is therefore: "Would this test actually fail if the production logic contained the described bug?" This question forces the reviewer to mentally run the assertion against a concrete, plausible failure scenario, instead of settling for the mere existence of an assertion.

// WEAK ASSERTION: only checks that something comes back at all
test('calculates shipping cost', () => {
  const result = calculateShipping(cart);
  expect(result).toBeDefined();
});

// MEANINGFUL ASSERTION: checks the actual expected value
test('calculates shipping cost for standard shipping below the free threshold', () => {
  const cart = { items: [{ price: 30 }], weight: 2 };
  const result = calculateShipping(cart);
  expect(result).toEqual({ method: 'standard', cost: 4.99 });
});

3. Typical review mistakes with test code at a glance

Besides overly weak assertions, overlooking missing negative cases is one of the most common review mistakes: a pull request testing only the successful path of a new function, without a single test for invalid input, empty result sets, or edge cases, often still gets accepted in review, because the existing, green test already creates visual confidence.

Another common mistake is failing to critically question test names: a test name like `testWorks()` or `testCase1()` conveys no information at all about which behavior is actually being checked, and should be consistently rejected in review, in favor of a name that states the checked condition and expected outcome in understandable language, say `testThrowsExceptionOnNegativeAmount()`.

Insufficient test isolation also often gets overlooked, say when a test relies on global, shared state or depends on the execution order of other tests, which only becomes apparent in review if the reviewer deliberately scans the test for dependencies on outside state, instead of relying solely on a successful CI run.

4. Spotting flakiness risks already during review

An attentive reviewer can spot many later-occurring flaky tests already during review by deliberately looking for certain warning signs: fixed wait times (`sleep(2000)`) instead of conditional waiting for a concrete state, assertions on exact timestamps or on the order of asynchronously running operations, and tests accessing external resources not controlled by the test itself, like the current date or a random generator without a fixed seed.

Another flakiness risk detectable in review is insufficient cleanup logic: a test that creates data in a shared test database but doesn't reliably remove it at the end can pollute subsequent, actually independent tests, causing seemingly random, hard-to-reproduce failures in completely different test files. Reviewers should therefore deliberately check whether a new test either runs inside a fully isolated transaction that gets rolled back at the end, or removes all self-created data in an explicit teardown step.

5. A practical checklist for test code review

A short, team-agreed checklist helps not to overlook the risks mentioned above even under time pressure: checking the assertion against a concrete failure scenario, checking for missing negative and edge cases, checking the test name for actual meaningfulness, checking for fixed wait times or uncontrolled time/random dependencies, and checking test isolation and cleanup logic.

This checklist shouldn't be understood as a rigid, bureaucratic ritual, but as a memory aid for exactly the aspects most likely to be overlooked under time pressure, and can be directly stored as a PR template in many projects' Git hosting system, so it automatically appears as a reminder on every new pull request.

6. Particularities when reviewing Magento and Hyvä tests

In a Magento project with a Hyvä frontend, reviewers should additionally check, for PHPUnit integration tests, whether a test actually runs against the correct fixture store configuration, say via `#[DataFixture]` or `#[ConfigFixture]` attributes, since a test accidentally running against the default store configuration instead of a deliberately prepared test configuration can unknowingly make business-wrong assumptions.

For Playwright or Cypress tests targeting the Hyvä frontend, it's also worth deliberately checking whether selectors reliably use `data-testid` attributes instead of Tailwind class names, since the latter can change with every CSS refactoring, making tests unnecessarily brittle against purely visual adjustments that don't represent any business-relevant change at all.

7. Establishing a culture where test code comments are taken seriously

For reviewers to actually invest time in a thorough test code review, this diligence needs to be visibly valued within the team, say by explicitly highlighting an especially precisely written test case positively during review, instead of implicitly treating test code comments as less important than comments on production logic.

It's also helpful to record recurring test code problems, say a particular flakiness pattern that came up multiple times in review, in a short, shared team document, so new colleagues benefit from already-gathered experience instead of making the same mistakes again and needing correction in review repeatedly.

8. Automated pre-checks before human review

A large share of the review mistakes mentioned above can already get caught in an automated fashion before the actual human review, letting the reviewer focus their limited attention on substantively assessing assertions instead of spending it on mechanically checkable formalities. A dedicated PHPCS sniff rejecting generic test names like `test1` or `testWorks` already in the CI pipeline reliably prevents such names from ever entering a pull request in the first place, instead of relying on every individual reviewer's attention.

Automated flakiness detection can also run ahead of human review: a CI system that runs every new test multiple times in a row and automatically leaves a comment on the pull request on inconsistent results makes a potential flakiness risk visible before the reviewer even starts reading the content, preventing a subtle time or ordering problem from getting overlooked amid the noise of a large pull request.

For particularly critical modules, it additionally pays off to make an automated mutation score a merge prerequisite: if a new pull request drops an affected module's mutation score below a previously defined threshold, the pipeline automatically blocks the merge until either stronger assertions get added or the deviation gets deliberately and documentedly accepted, so weak assertions no longer depend solely on the human reviewer's vigilance.

9. Review priorities at a glance

The table below summarizes the most important priorities for a thorough test code review.

10. Summary

11. FAQ: Code Review for Tests: The Essentials at a Glance