Review the Tests First, Split Big Diffs
Stop reading the code first in review. Read the tests before the implementation. The tests tell you what the change is supposed to do — intent, edge cases, and coverage — before you judge how it does it. And with AI assistants generating bigger diffs faster than ever, the second half of the tip matters just as much: watch the size, and split anything that outgrows one logical change.
Why tests first
An implementation shows you how; tests show you what and why. Reading them first gives you a spec to review against instead of reverse-engineering intent from the code:
- Intent — test names and assertions state the expected behavior in plain terms:
test_refund_rejects_expired_cardtells you more in one line than 40 lines of handler code. - Coverage — missing tests are visible immediately. If the diff touches error handling but no test exercises the failure path, you've found your first review comment before reading a single implementation line.
- Validity — tests don't test themselves. Google's reviewer guide asks: will these tests actually fail when the code is broken? Does each one make simple, useful assertions? A human has to check that, and it's easiest while the tests are your first, fresh read.
This mirrors how Google's guide recommends navigating a change anyway: read the description, then the main files first, because the big parts give context to everything else. For most changes, the tests are that context.
The size ladder
Review quality degrades with diff size — SmartBear's study of Cisco code reviews found reviewers catch 70–90% of defects when reviewing 200–400 lines over 60–90 minutes, and effectiveness drops fast beyond that. Google's own guideline: 100 lines is usually a reasonable change, 1,000 is usually too large.
| Diff size | Verdict | What to do |
|---|---|---|
| ~100 lines | Easy | Review in one sitting, tests first |
| 200–400 lines | Workable ceiling | One logical change, ≤500 lines/hour pace |
| ~1,000 lines | Too large | Split before requesting review |
Line count isn't the only axis: 200 lines in one file reads fine; the same 200 lines spread across 50 files usually doesn't.
Three ways to split
- Stack small changes. Write one small change, send it for review, and immediately start the next one based on the first. You keep moving while reviewers work, and each link in the chain stays reviewable.
- Split by file group. Ship the schema or interface change in one diff, the code that uses it in a follow-up. Reviewers can approve the contract before wading into the call sites.
- Cut vertical slices. For a feature, land one thin end-to-end path — UI to storage — then widen it. Each slice is independently testable and demoable, which also means its tests are readable as a spec.
Power-user moves
- Make the AI split for you. When an assistant proposes a 900-line change, ask it to restage the work as a stacked sequence — "schema first, then handler, then UI, each with its own tests" — before you open a single file.
- Diff the tests alone.
git diff main -- '*test*'shows you the intent layer of a branch in isolation. If that diff is empty on a behavior change, the review starts with "where are the tests?" - Reject "tests later." Tests belong in the same change as the code they cover; a follow-up promise is how coverage gaps ship.
- Budget review time, not just lines. Under an hour per session — defect detection falls off a cliff with reviewer fatigue, no matter how disciplined the author was.
Resources
- Small CLs — Google Engineering Practices
- What to look for in a code review (tests section) — Google Engineering Practices
- Navigating a CL in review — Google Engineering Practices
- Best practices for peer code review — SmartBear
Building an AI feature? Yeda AI designs, audits, and ships production LLM systems.