ARC // OS — Test Review Process

Purpose: Ensure all tests are reviewed with code, quality is assessed, coverage is verified, and flaky tests are identified and fixed.

Status: ✅ Active Process
Last Updated: 2026-01-18

---

Overview

Every pull request that includes tests must have those tests reviewed. This process ensures:

• Tests are reviewed alongside code changes
• Test quality meets standards
• Coverage is verified and maintained
• Flaky tests are identified and fixed
• Tests serve as documentation

---

When to Review Tests

Tests must be reviewed in the following scenarios:

1. New Features - All new code must have tests
2. Bug Fixes - Bug fixes must include regression tests
3. Refactoring - Refactored code must maintain or improve test coverage
4. Test Updates - Any changes to existing tests
5. Coverage Drops - If coverage decreases, tests must be reviewed

---

Test Review Checklist

Use this checklist when reviewing tests in a pull request:

1. Test Coverage

• New code has tests - Every new function, method, component, or route has tests
• Coverage threshold met - Coverage is at least 90% (100% for critical paths)
• Coverage not decreased - Existing coverage is maintained or improved
• Critical paths covered - Authentication, validation, business rules, error handling, ownership checks have 100% coverage

2. Test Quality

• Tests are independent - No shared state between tests
• Tests clean up - Database, mocks, and resources are cleaned up after each test
• Clear test names - Test names clearly describe what is being tested
• One thing per test - Each test verifies a single behavior
• Deterministic - Tests produce consistent results (no randomness, no timing issues)
• Fast execution - Tests complete quickly (< 100ms per test ideally)

3. Test Structure

• Arrange-Act-Assert pattern - Tests follow AAA pattern
• Clear setup/teardown - Setup and cleanup are explicit
• Meaningful assertions - Assertions verify actual behavior, not implementation details
• Error cases tested - Error paths and edge cases are covered
• Business rules tested - Business logic and validation rules are verified

4. Test Content

• Happy paths tested - Normal operation is verified
• Error paths tested - Error handling is verified
• Edge cases tested - Boundary conditions and edge cases are covered
• User ownership verified - Authorization and ownership checks are tested
• Validation tested - Input validation and business rules are tested

5. Mocking Strategy

• External dependencies mocked - External APIs, email service, file system are mocked
• Database not mocked - Integration tests use real database
• Services not mocked - Services being tested are not mocked
• Mocks are realistic - Mocked data and responses are realistic

6. Test Documentation

• Tests are readable - Tests serve as documentation
• Comments when needed - Complex tests have explanatory comments
• Test organization - Related tests are grouped logically

7. Flaky Test Detection

• No timing dependencies - Tests don't rely on specific timing
• No race conditions - Tests don't have race conditions
• No random data - Tests don't use random data without seeds
• No external dependencies - Tests don't depend on external services
• No shared state - Tests don't share state that could cause flakiness

---

Review Process

Step 1: Initial Review

1. Check test coverage

   • Run pnpm test:coverage locally or check CI coverage report
   • Verify coverage meets threshold (90% minimum, 100% for critical paths)
   • Verify coverage hasn't decreased

2. Run tests locally

   • Run pnpm test to ensure all tests pass
   • Check for any warnings or errors
   • Verify tests complete in reasonable time

3. Review test files

   • Read through test files in the PR
   • Check test structure and organization
   • Verify test names are descriptive

Step 2: Quality Assessment

1. Check test quality standards

   • Use the checklist above
   • Verify tests follow docs/testing/TEST_QUALITY_STANDARDS.md
   • Check for common anti-patterns

2. Verify test content

   • Ensure happy paths are tested
   • Verify error cases are covered
   • Check edge cases are tested
   • Verify business rules are tested

3. Check mocking strategy

   • Verify external dependencies are mocked
   • Ensure database is not mocked in integration tests
   • Check mocks are realistic

Step 3: Coverage Verification

1. Check coverage report

   • Review coverage report (HTML or terminal output)
   • Identify any uncovered lines
   • Verify critical paths have 100% coverage

2. Verify coverage for new code

   • All new functions/methods have tests
   • All new components have tests
   • All new routes have tests

3. Check for coverage gaps

   • Identify any uncovered code paths
   • Verify error handling is tested
   • Check edge cases are covered

Step 4: Flaky Test Identification

1. Run tests multiple times

   • Run pnpm test 3-5 times
   • Check for inconsistent results
   • Identify any tests that fail intermittently

2. Check for flaky patterns

   • Timing dependencies (setTimeout, Date.now())
   • Race conditions (async operations without proper waiting)
   • Random data (Math.random() without seeds)
   • External dependencies (network calls, file system)
   • Shared state (global variables, singletons)

3. Review test isolation

   • Verify tests don't share state
   • Check cleanup is thorough
   • Ensure tests are independent

Step 5: Review Comments

1. Provide constructive feedback

   • Point out issues clearly
   • Suggest improvements
   • Reference standards and documentation

2. Request changes if needed

   • Block PR if tests don't meet standards
   • Request fixes for flaky tests
   • Require coverage improvements

3. Approve when ready

   • All checklist items are satisfied
   • Tests meet quality standards
   • Coverage is verified
   • No flaky tests identified

---

Identifying Flaky Tests

Common Flaky Test Patterns

1. Timing Dependencies

   // ❌ Bad: Relies on timing
   setTimeout(() => {
     expect(value).toBe(expected);
   }, 100);
   
   // ✅ Good: Uses proper waiting
   await waitFor(() => {
     expect(value).toBe(expected);
   });

2. Race Conditions

   // ❌ Bad: Race condition
   async function test() {
     await Promise.all([operation1(), operation2()]);
     expect(result).toBe(expected); // May fail if operations complete in different order
   }
   
   // ✅ Good: Proper sequencing
   async function test() {
     await operation1();
     await operation2();
     expect(result).toBe(expected);
   }

3. Random Data

   // ❌ Bad: Random data
   const randomId = Math.random().toString();
   
   // ✅ Good: Deterministic data
   const testId = 'test-id-123';

4. Shared State

   // ❌ Bad: Shared state
   let globalCounter = 0;
   
   // ✅ Good: Isolated state
   beforeEach(() => {
     const counter = 0; // Fresh for each test
   });

5. External Dependencies

   // ❌ Bad: External API call
   const response = await fetch('https://api.example.com/data');
   
   // ✅ Good: Mocked external dependency
   vi.mock('../utils/api');
   const response = await api.get('/data');

How to Identify Flaky Tests

1. Run tests multiple times

   # Run tests 5 times
   for i in {1..5}; do
     pnpm test
   done

2. Check CI history

   • Review CI test results over multiple runs
   • Look for tests that fail intermittently
   • Check for patterns in failures

3. Review test code

   • Look for timing dependencies
   • Check for race conditions
   • Verify no shared state
   • Ensure no external dependencies

4. Use test retries (temporary)

   • Add retries to identify flaky tests
   • Remove retries once tests are fixed
   • Retries should not be permanent

---

Coverage Verification Process

Automated Coverage Checks

1. CI Coverage Report

   • CI automatically generates coverage reports
   • Coverage thresholds are enforced
   • PRs are blocked if coverage drops

2. Local Coverage Report

   # Backend
   cd server
   pnpm test:coverage
   # Open coverage/index.html
   
   # Frontend
   cd web
   pnpm test:coverage
   # Open coverage/index.html

Manual Coverage Review

1. Review coverage report

   • Open HTML coverage report
   • Identify uncovered lines
   • Check for uncovered branches

2. Verify critical paths

   • Authentication code: 100% coverage
   • Data validation: 100% coverage
   • Business rules: 100% coverage
   • Error handling: 100% coverage
   • User ownership checks: 100% coverage

3. Check for coverage gaps

   • Uncovered error paths
   • Uncovered edge cases
   • Uncovered validation rules
   • Uncovered business logic

---

Test Review Questions

When reviewing tests, ask these questions:

1. Does this test actually verify the behavior?

   • Does the test verify what it claims to test?
   • Would the test fail if the behavior changed?
   • Is the assertion meaningful?

2. What happens if this test fails? Is it clear?

   • Is the error message helpful?
   • Can you identify what went wrong?
   • Is the test name descriptive?

3. Are edge cases covered?

   • Boundary conditions tested?
   • Null/undefined values handled?
   • Empty arrays/objects tested?
   • Maximum values tested?

4. Are error cases covered?

   • Error paths tested?
   • Error messages verified?
   • Error handling verified?

5. Is the test maintainable?

   • Is the test easy to understand?
   • Is the test easy to modify?
   • Is the test well-organized?

6. Could this test be flaky?

   • Does it rely on timing?
   • Does it have race conditions?
   • Does it use random data?
   • Does it depend on external services?

7. Is the test fast enough?

   • Does it complete quickly?
   • Does it avoid slow operations?
   • Could it be optimized?

---

Review Approval Criteria

A test review can be approved when:

✅ All tests pass
✅ Coverage meets threshold (90% minimum, 100% for critical paths)
✅ Coverage has not decreased
✅ Tests follow quality standards
✅ Tests are not flaky
✅ Tests serve as documentation
✅ All checklist items are satisfied

A test review should request changes when:

❌ Tests are missing for new code
❌ Coverage is below threshold
❌ Coverage has decreased
❌ Tests don't follow quality standards
❌ Tests are flaky
❌ Tests don't verify behavior
❌ Tests are hard to understand

---

Continuous Improvement

Regular Reviews

• Review test quality in sprint retrospectives
• Identify common issues and patterns
• Update standards based on learnings
• Share best practices with team

Test Quality Metrics

• Track test execution time
• Monitor flaky test rate
• Track coverage trends
• Measure test maintenance effort

Documentation Updates

• Update docs/testing/TEST_QUALITY_STANDARDS.md based on learnings
• Update docs/testing/TESTING_REQUIREMENTS.md with new patterns
• Document common issues and solutions
• Share lessons learned

---

Resources

• TESTING_REQUIREMENTS.md
• TEST_QUALITY_STANDARDS.md
• TEST_DOCUMENTATION.md

---

Remember: Tests are not overhead. They are insurance. Every test is a safety net. Every untested line is a risk.

This system controls daily life. Test quality is not optional.