Code Review and Testing Culture

Analogy categoryAnalogies

Testing culture is what a team actually does when no one is enforcing it — whether engineers write tests before being asked, whether reviewers block merges on missing coverage, and whether flaky tests get fixed or silently re-run. Code review is the primary lever for shaping this culture because it's the checkpoint where testing habits become visible and get either reinforced or quietly excused. A team's real testing standard is whatever its reviewers consistently accept, not whatever is written in a style guide.

🏏

Cricket analogy: A team's testing culture is like a fielding culture — a captain like Ricky Ponting demanded every misfield be addressed in the next net session, not excused as bad luck, and that daily reinforcement built Australia's elite fielding standard over years.

What Reviewers Should Actually Check

Effective test-focused review goes beyond 'are there tests' to specific questions: do tests assert on behavior rather than implementation details, do they cover the boundary and error cases (not just the happy path), is test naming descriptive enough to diagnose a failure without reading the test body, and are new tests meaningfully independent (no shared mutable state causing order-dependent flakiness). A reviewer who only checks that a coverage number went up misses tests that pad coverage without verifying anything meaningful.

🏏

Cricket analogy: A good reviewer checking test quality is like a bowling coach checking Jasprit Bumrah's wrist position at release, not just whether the ball reached the stumps — the mechanism matters as much as the outcome.

PR Templates and Enforced Gates

Culture scales better when it's encoded into process rather than left to individual reviewer memory: a pull request template with a checklist item for 'tests cover new behavior and at least one edge case,' a required CI status check that blocks merge below a coverage threshold, and a CODEOWNERS rule requiring a second reviewer on files with historically low test coverage. These mechanisms turn 'please remember to check tests' into something the platform enforces automatically, freeing reviewers to focus on test quality rather than test existence.

🏏

Cricket analogy: A CI coverage gate is like the third umpire's mandatory review on any close run-out — the check happens automatically every time, regardless of which on-field umpire is officiating that match.

yaml

# .github/workflows/ci.yml (excerpt) — block merges under a coverage threshold
name: CI
on: [pull_request]
jobs:
  test:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v4
      - run: npm ci
      - run: npm test -- --coverage --coverageThreshold='{"global":{"lines":85,"branches":75}}'

A coverage gate should be treated as a floor, not a target. Teams that optimize purely to keep the number above threshold end up with shallow tests that assert little; pair the gate with reviewer judgment on test quality, not just percentage.

Handling Flaky Tests Without Eroding Trust

Nothing corrodes testing culture faster than flaky tests that get re-run until green instead of fixed — within weeks, engineers stop trusting red CI and start habitually clicking 'retry,' which means real regressions slip through unnoticed. A healthy culture treats a flaky test as a bug with the same priority as a production incident: quarantine it (mark it skip or move it to a separate non-blocking suite) immediately, file a ticket, and fix the root cause (usually shared state, timing assumptions, or unmocked network calls) within an agreed SLA rather than letting it linger indefinitely.

🏏

Cricket analogy: A flaky test that's just re-run until green is like a batsman given not-out on a plumb LBW by a lenient umpire every time — the team stops trusting the umpire's calls entirely, and real dismissals start getting missed too.

Never let a flaky test sit in the main suite indefinitely with a mental note to 'fix it later.' Quarantine it into a non-blocking job the same day it's identified — an unaddressed flaky test trains the whole team to distrust CI within a few weeks.

Testing culture is defined by what reviewers actually enforce in practice, not by written style guides.
Reviewers should check for behavior-focused assertions, edge-case coverage, descriptive naming, and test independence — not just test existence.
PR templates, CI coverage gates, and CODEOWNERS rules encode culture into process so it scales beyond individual reviewer memory.
Coverage thresholds should be treated as a floor, paired with human judgment on test quality.
Flaky tests must be quarantined and fixed on an SLA, never left to accumulate silent re-runs.
Trust in CI erodes quickly once engineers habitually retry red builds instead of investigating them.
A team's real testing standard is whatever gets consistently accepted at code review, not what's documented.

Practice what you learned

1. According to the article, what actually defines a team's real testing standard?

2. Which is an example of checking test quality beyond simply confirming tests exist?

3. What is the recommended immediate action when a flaky test is identified?

4. Why should a coverage threshold gate be treated as a 'floor, not a target'?

5. What is a key benefit of encoding testing expectations into PR templates and CI gates?

Was this page helpful?

Choosing a Testing Framework

TDD Case Study Walkthrough

Related Study Notes in Software Engineering

Browse all study notes

Software Engineering

Microservices Study Notes

Software Architecture · 30 topics

Software Engineering

Design Patterns Study Notes

Software Design · 30 topics

Software Engineering

Software Engineering Study Notes

Python · 40 topics

Software Engineering

Git & Version Control Study Notes

Bash · 40 topics

Software Engineering

System Design Study Notes

Architecture · 40 topics

Topics covered

#SoftwareTesting#TestingTDDStudyNotes#SoftwareEngineering#CodeReviewAndTestingCulture#Code#Review#Testing#Culture#StudyNotes#SkillVeris

Code Review and Testing Culture