The QA Problem: Green Builds, Low Confidence
Many teams treat automated tests in pull requests (PRs) as a formality: if the pipeline passes, the change is considered “safe.” The underlying claim worth taking seriously is that “passing” is not the same as “useful.” A test can execute and still provide little additional confidence, add maintenance burden, or even mislead reviewers about coverage.
This gap becomes more visible when generating tests is easy (including via AI tools). When the cost of writing tests drops, the cost of keeping tests—reviewing, understanding, debugging, and maintaining them—becomes the constraint. QA leaders then face a concrete decision: should PR review explicitly evaluate test quality, not just test pass/fail? In most organizations, the practical answer should be “yes,” but it requires structure.
This article lays out a pragmatic, reviewer-friendly approach Anbosoft teams can apply to close the “green build, low confidence” gap without turning PRs into week-long test debates.
A Practical Goal for PR Test Review
Instead of “add more tests” or “raise coverage,” set a PR-level goal:
- Every new or changed test should earn its place by contributing meaningful, maintainable confidence about behavior that matters.
That goal gives reviewers permission to ask: “Should this test exist?”—not merely “Does it pass?”
Define What “Good Tests” Mean in Your PRs
Create a small checklist that reviewers can apply quickly. Keep it behavior- and risk-oriented rather than style-oriented.
1) Value: Does the test protect meaningful behavior?
A test is valuable if it would fail for a real defect that matters. In review, look for:
- Clear connection to a requirement, bug fix, or risk.
- Assertions that would catch an incorrect outcome (not just that code ran).
- Coverage of boundary conditions where defects cluster (validation, authorization, rounding, state transitions).
Anti-patterns to flag
- Assertions that only check “not null,” “no exception,” or “method called,” without verifying an outcome users care about.
- Tests that execute lines but don’t check observable results.
2) Distinctness: Is it truly new coverage or duplicate noise?
Redundant tests increase suite size and review load without increasing confidence. In PR review, ask:
- Does an existing test already cover this scenario?
- Are multiple new tests variants of the same path with trivial input changes?
- Could these be collapsed into a parameterized test or a smaller set of representative cases?
This isn’t about minimizing the count at all costs; it’s about keeping the suite understandable and purposeful.
3) Isolation and determinism: Will it be stable in CI?
A frequent source of flakiness is hidden dependency on external state. During review, scan for:
- Reliance on system time (e.g., “now”), time zones, or locale formatting.
- Filesystem reads/writes outside controlled temp directories.
- Network calls, real databases, or shared services.
- Environment variables that vary by machine.
Prefer designs that inject a clock, use in-memory fakes, or isolate I/O behind interfaces. Even if a test passes today, reviewers should push back on nondeterminism that will later become “flaky test debt.”
4) Coupling: Is it testing behavior or the implementation?
A core PR review question is whether the test is resilient to refactoring:
- Does it assert outcomes and externally visible effects?
- Or does it assert internal calls, private structure, or incidental ordering that may change without breaking behavior?
Some interaction-based testing is valid, but reviewers should require a rationale when the test is heavily coupled to internal mechanics.
5) Maintainability: Is the test readable and cheap to modify?
Tests are code. Review them like code:
- Can a new engineer understand intent quickly?
- Is setup minimal and relevant?
- Are helper methods and fixtures used to clarify, not obscure?
Overbuilt test scaffolding can make small product changes expensive.
Make Test Review a First-Class PR Activity
Test review fails when it is nobody’s job. A lightweight process change often works better than adding more tooling.
Update the PR template
Add a “Tests added/changed” section with prompts such as:
- What risk or requirement does each new test address?
- Why is this test not redundant with existing coverage?
- What makes the test deterministic in CI?
- If mocks are used, what behavior are we validating?
The goal is not bureaucracy; it’s to force the author to articulate value and design intent.
Assign explicit test reviewers
If your workflow supports it, set an expectation:
- Any PR that adds or significantly modifies tests requires at least one reviewer to comment on test quality, not only product code.
This prevents the common failure mode where reviewers focus on implementation and treat tests as an afterthought.
Triage Rules: What Reviewers Should Push Back On
Review time is limited. Use “stop-the-line” criteria for the worst offenders:
- No meaningful assertions (tests that only execute code).
- Flakiness risk (time, randomness, concurrency, shared state) without control mechanisms.
- Excessive mocking that makes the test harder to understand than the feature.
- Overlapping tests that bloat the suite in the same PR.
Everything else can be improvement suggestions rather than blockers.
Hypothetical Example: A PR Adds Many Passing Tests, But Confidence Doesn’t Increase
(Hypothetical example) A PR introduces a new discount rule in a checkout service and adds 25 unit tests. CI is green. Coverage increases.
During test review, a reviewer notices:
- 15 tests differ only by minor input variations but assert the same output category.
- Several tests assert internal helper method calls rather than final price or invoice lines.
- A few tests depend on the current date to determine promotional eligibility.
Next-step decisions (hypothetical):
- Collapse similar cases into a parameterized test with a short table of representative inputs.
- Replace internal-call assertions with outcome assertions (final amount, applied discount code).
- Inject a controllable clock into the service or freeze time in the test harness.
The result isn’t “fewer tests at all costs,” but a smaller, clearer set that fails for the right reasons and won’t start flaking next month.
Introduce Lightweight Automation That Assists Review (Not Replaces It)
While humans must judge intent, you can still automate cues that help reviewers focus:
- Flag new tests that have no assertions (or only trivial assertions).
- Flag tests that access filesystem/network/time APIs directly.
- Flag unusually long tests or heavy setup blocks (a proxy for complexity).
- Detect suspicious repetition (e.g., many tests differing only by literal values).
Use these as PR annotations or checklist warnings, not as absolute gates. Automation should guide attention, not “approve” test quality.
Operational Next Steps for QA Leads
- Define “test quality” in one page using the five dimensions above (value, distinctness, isolation, coupling, maintainability).
- Update PR templates to require a short explanation of what each new/changed test proves.
- Train reviewers with 2–3 internal examples of “passing but low-value” tests and how to improve them.
- Add a “test review” checkbox to your PR process and make it socially expected.
- Pilot a small set of automated flags (assertion presence, time/network access, duplication heuristics) to reduce reviewer burden.
The Outcome You Should Aim For
A mature PR doesn’t only answer:
- Did the code change compile and pass tests?
It also answers:
- Do the tests added or modified by this PR deliver durable confidence at an acceptable maintenance cost?
When teams make that second question routine, the test suite becomes an asset that scales—rather than a growing pile of “green” noise that slows delivery and erodes trust.
