test-reviewer

# Test Reviewer **Audience:** Developers reviewing test suites for quality and completeness. **Goal:** Assess whether tests prove intended user workflows and independent behavior contracts, with meaningful assertions and failure detection rather than test count or method coverage. ## Review Steps ### 1. Identify Test Context - Establish the review scope from the request, changed files, or provided diff - Locate test files related to the changed behavior - Identify the testing framework and repository conventions ### 2. Map Behavior Contracts to Tests - Start from user workflows, acceptance criteria, documented contracts, and known failure traces, not the implementation's assumptions - State the expected outcomes independently of the code; mark missing or ambiguous expectations as evidence gaps - Read the implementation and relevant callers to trace how each behavior is exercised - Create a coverage matrix mapping user workflows and behavior contracts to test cases, including the failure each case would detect - Prefer end-to-end checks that prove a user can complete the workflow. Keep focused unit and integration tests when they detect additional real failures or isolate a failure that a broader check would miss - Do not require a separate unit test for every public method or enforce a testing pyramid by habit ### 3. Analyze Test Coverage For each user workflow or behavior contract, 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 against an independent expectation, not mirror implementation details? - Would a plausible regression in the expected outcome make the test fail? - Do end-to-end checks exercise the real workflow rather than mock away the behavior they claim to verify? - 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 - **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 `sleep` or hardcoded delays (flaky test smell) - Tests with non-deterministic data (random values without seeds) - Over-reliance on `allow_any_instance_of` or 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 - Agent-written tests that copy the implementation's assumptions without an independent contract - Many passing unit tests with no check that the user can complete the workflow - Redundant tests that add maintenance work without detecting additional failures - Missing database state verification - **Frozen fixture assertions**: Exact collection comparisons (`assert_equal [a, b], scope` or `expect(scope).to eq([a, b])`) that break when unrelated fixtures are added. Recommend `assert_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 | Workflow/Contract | Expected Outcome Source | Test Level | Failure Detected | Coverage Gaps | |-------------------|-------------------------|------------|------------------|---------------| | [workflow] | [specification or failure trace] | [e2e/integration/unit] | [regression] | [missing or unverified behavior] | Record coverage as present, missing, or unknown. This static review cannot establish that tests passed; report execution evidence separately when supplied. ### Critical Coverage Gaps [List features/methods with missing test coverage, prioritized by risk] 1. **[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] 1. [Scenario]: [why it matters] → [file to add test] ### Pattern Violations [Tests that don't follow project conventions or best practices] ### Recommendations 1. [Prioritized actionable recommendations] 2. [Include specific test examples where helpful] 3. [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**: Use clear repository-relative file paths and line numbers. Focus on actionable feedback that improves test quality and coverage.