Test Reviewer
Audience: Developers reviewing test suites for quality and completeness.
Goal: Analyze automated tests for coverage completeness, assertion quality, and adherence to testing best practices.
Review Steps
1. Identify Test Context
- Run
git statusandgit diffto identify changed files and their corresponding test files - Use
GrepandGlobto locate test files related to the changed code - Identify the testing framework (RSpec, Minitest, Jest, Playwright)
2. Map Implementation to Tests
- Read the implementation files being tested
- Identify all public methods, endpoints, and behaviors that should have test coverage
- Create a coverage matrix mapping implementation features to test cases
3. Analyze Test Coverage
For each implementation feature, check if tests exist for:
- Happy Path: Normal successful execution flows
- Sad Path: Error conditions, validation failures, edge cases
- Boundary Conditions: Empty values, nil/null, maximum limits
- State Transitions: Before/after states, side effects
- Authorization: Permission checks (if applicable)
- Integration Points: External service calls, database operations
4. Evaluate Test Quality
- Assertion Quality:
- Are assertions specific and meaningful?
- Do tests verify behavior, not implementation?
- Are error messages helpful for debugging?
- Test Isolation:
- Do tests depend on execution order?
- Are external services properly mocked?
- Is test data properly set up and torn down?
- AAA Pattern:
- Clear Arrange (setup) section
- Single Act (execution)
- Focused Assert (verification)
- Test Names:
- Do names describe expected behavior?
- Would someone understand the test without reading code?
5. Check Test Patterns
- Framework Compliance:
- RSpec: Proper use of
describe,context,it,let,before - Minitest: Proper use of
setup,test, fixtures - Jest: Proper use of
describe,it,beforeEach, mocks
- RSpec: Proper use of
- Project Conventions:
- Do tests follow existing patterns in the codebase?
- Are shared examples/contexts used where appropriate?
- Is test data handled consistently (fixtures vs factories)?
6. Identify Missing Scenarios
Look for common gaps:
- Nil/Empty Handling: What happens with nil, empty strings, empty arrays?
- Error Cases: Network failures, database errors, validation failures
- Concurrency: Race conditions, parallel execution
- Edge Cases: First/last items, exactly at limits, off-by-one
- Security: Input sanitization, authorization bypasses
- Performance: Large data sets, timeout scenarios
7. Check for Test Anti-Patterns
- Skipped/pending tests without justification
- Tests with
sleepor hardcoded delays (flaky test smell) - Tests with non-deterministic data (random values without seeds)
- Over-reliance on
allow_any_instance_ofor similar broad mocks
Note: This skill performs static analysis only. Test execution is handled separately to avoid redundant test runs.
Red Flags to Watch For:
- Tests that always pass (no real assertions)
- Tests that test Rails/framework behavior, not application code
- Overly complex test setup indicating design issues
- Mocking too much (testing mocks, not real behavior)
- No error case coverage
- Tests tightly coupled to implementation details
- Missing database state verification
- Frozen fixture assertions: Exact collection comparisons (
assert_equal [a, b], scopeorexpect(scope).to eq([a, b])) that break when unrelated fixtures are added. Recommendassert_includes/expect(...).to include(...)instead.
Test Quality Review Report
Provide your findings in this structure:
Test Coverage Summary
- Implementation Files Reviewed: [count]
- Test Files Reviewed: [count]
- Coverage Gaps Identified: [count]
- Quality Issues Found: [count]
Coverage Matrix
| Feature/Method | Happy Path | Sad Path | Edge Cases | Status | |----------------|------------|----------|------------|--------| | [method_name] | pass/fail | pass/fail| pass/fail | [Complete/Gaps] |
Critical Coverage Gaps
[List features/methods with missing test coverage, prioritized by risk]
- [Feature/Method Name] - [file:line]
- Missing: [what scenarios are not tested]
- Risk: [why this gap matters]
- Recommended: [specific test to add]
Test Quality Issues
High Priority:
- [Issue description with file:line reference]
- Recommendation: [how to fix]
Medium Priority:
- [Issue description]
- Recommendation: [how to fix]
Low Priority:
- [Issue description]
- Recommendation: [how to fix]
Missing Edge Cases
[List specific edge case scenarios that should be tested]
- [Scenario]: [why it matters] → [file to add test]
Pattern Violations
[Tests that don't follow project conventions or best practices]
Recommendations
- [Prioritized actionable recommendations]
- [Include specific test examples where helpful]
- [Reference project patterns to follow]
Overall Assessment
- Test Suite Health: [Excellent/Good/Needs Work/Critical Gaps]
- Confidence Level: [High/Medium/Low] - Can we ship with current tests?
- Priority Actions: [Top 3 things to fix before merging]
Note: All file paths should be absolute. Focus on actionable feedback that improves test quality and coverage.