All checks were successful
Deploy to Test Environment / deploy-to-test (push) Successful in 15m11s
215 lines
8.0 KiB
Markdown
215 lines
8.0 KiB
Markdown
# ADR-040: Testing Economics and Priorities
|
|
|
|
**Date**: 2026-01-09
|
|
|
|
**Status**: Accepted
|
|
|
|
## Context
|
|
|
|
ADR-010 established the testing strategy and standards. However, it does not address the economic trade-offs of testing: when the cost of writing and maintaining tests exceeds their value. This document provides practical guidance on where to invest testing effort for maximum return.
|
|
|
|
## Decision
|
|
|
|
We adopt a **value-based testing approach** that prioritizes tests based on:
|
|
|
|
1. Risk of the code path (what breaks if this fails?)
|
|
2. Stability of the code (how often does this change?)
|
|
3. Complexity of the logic (can a human easily verify correctness?)
|
|
4. Cost of the test (setup complexity, execution time, maintenance burden)
|
|
|
|
## Testing Investment Matrix
|
|
|
|
| Test Type | Investment Level | When to Write | When to Skip |
|
|
| --------------- | ------------------- | ------------------------------- | --------------------------------- |
|
|
| **E2E** | Minimal (5 tests) | Critical user flows only | Everything else |
|
|
| **Integration** | Moderate (17 tests) | API contracts, auth, DB queries | Internal service wiring |
|
|
| **Unit** | High (185+ tests) | Business logic, utilities | Defensive fallbacks, trivial code |
|
|
|
|
## High-Value Tests (Always Write)
|
|
|
|
### E2E Tests (Budget: 5-10 tests total)
|
|
|
|
Write E2E tests for flows where failure means:
|
|
|
|
- Users cannot sign up or log in
|
|
- Users cannot complete the core value proposition (upload flyer → see deals)
|
|
- Money or data is at risk
|
|
|
|
**Current E2E coverage is appropriate:**
|
|
|
|
- `auth.e2e.test.ts` - Registration, login, password reset
|
|
- `flyer-upload.e2e.test.ts` - Complete upload pipeline
|
|
- `user-journey.e2e.test.ts` - Full user workflow
|
|
- `admin-authorization.e2e.test.ts` - Admin access control
|
|
- `admin-dashboard.e2e.test.ts` - Admin operations
|
|
|
|
**Do NOT add E2E tests for:**
|
|
|
|
- UI variations or styling
|
|
- Edge cases (handle in unit tests)
|
|
- Features that can be tested faster at a lower level
|
|
|
|
### Integration Tests (Budget: 15-25 tests)
|
|
|
|
Write integration tests for:
|
|
|
|
- Every public API endpoint (contract testing)
|
|
- Authentication and authorization flows
|
|
- Database queries that involve joins or complex logic
|
|
- Middleware behavior (rate limiting, validation)
|
|
|
|
**Current integration coverage is appropriate:**
|
|
|
|
- Auth, admin, user routes
|
|
- Flyer processing pipeline
|
|
- Shopping lists, budgets, recipes
|
|
- Gamification and notifications
|
|
|
|
**Do NOT add integration tests for:**
|
|
|
|
- Internal service-to-service calls (mock at boundaries)
|
|
- Simple CRUD operations (test the repository pattern once)
|
|
- UI components (use unit tests)
|
|
|
|
### Unit Tests (Budget: Proportional to complexity)
|
|
|
|
Write unit tests for:
|
|
|
|
- **Pure functions and utilities** - High value, easy to test
|
|
- **Business logic in services** - Medium-high value
|
|
- **React components** - Rendering, user interactions, state changes
|
|
- **Custom hooks** - Data transformation, side effects
|
|
- **Validators and parsers** - Edge cases matter here
|
|
|
|
## Low-Value Tests (Skip or Defer)
|
|
|
|
### Tests That Cost More Than They're Worth
|
|
|
|
1. **Defensive fallback code protected by types**
|
|
|
|
```typescript
|
|
// This fallback can never execute if types are correct
|
|
const name = store.name || 'Unknown'; // store.name is required
|
|
```
|
|
|
|
- If you need `as any` to test it, the type system already prevents it
|
|
- Either remove the fallback or accept the coverage gap
|
|
|
|
2. **Switch/case default branches for exhaustive enums**
|
|
|
|
```typescript
|
|
switch (status) {
|
|
case 'pending':
|
|
return 'yellow';
|
|
case 'complete':
|
|
return 'green';
|
|
default:
|
|
return ''; // TypeScript prevents this
|
|
}
|
|
```
|
|
|
|
- The default exists for safety, not for execution
|
|
- Don't test impossible states
|
|
|
|
3. **Trivial component variations**
|
|
- Testing every tab in a tab panel when they share logic
|
|
- Testing loading states that just show a spinner
|
|
- Testing disabled button states (test the logic that disables, not the disabled state)
|
|
|
|
4. **Tests requiring excessive mock setup**
|
|
- If test setup is longer than test assertions, reconsider
|
|
- Per ADR-010: "Excessive mock setup" is a code smell
|
|
|
|
5. **Framework behavior verification**
|
|
- React rendering, React Query caching, Router navigation
|
|
- Trust the framework; test your code
|
|
|
|
### Coverage Gaps to Accept
|
|
|
|
The following coverage gaps are acceptable and should NOT be closed with tests:
|
|
|
|
| Pattern | Reason | Alternative |
|
|
| ------------------------------------------ | ------------------------- | ----------------------------- |
|
|
| `value \|\| 'default'` for required fields | Type system prevents | Remove fallback or accept gap |
|
|
| `catch (error) { ... }` for typed APIs | Error types are known | Test the expected error types |
|
|
| `default:` in exhaustive switches | TypeScript exhaustiveness | Accept gap |
|
|
| Logging statements | Observability, not logic | No test needed |
|
|
| Feature flags / environment checks | Tested by deployment | Config tests if complex |
|
|
|
|
## Time Budget Guidelines
|
|
|
|
For a typical feature (new API endpoint + UI):
|
|
|
|
| Activity | Time Budget | Notes |
|
|
| --------------------------------------- | ----------- | ------------------------------------- |
|
|
| Unit tests (component + hook + utility) | 30-45 min | Write alongside code |
|
|
| Integration test (API contract) | 15-20 min | One test per endpoint |
|
|
| E2E test | 0 min | Only for critical paths |
|
|
| Total testing overhead | ~1 hour | Should not exceed implementation time |
|
|
|
|
**Rule of thumb**: If testing takes longer than implementation, you're either:
|
|
|
|
1. Testing too much
|
|
2. Writing tests that are too complex
|
|
3. Testing code that should be refactored
|
|
|
|
## Coverage Targets
|
|
|
|
We explicitly reject arbitrary coverage percentage targets. Instead:
|
|
|
|
| Metric | Target | Rationale |
|
|
| ---------------------- | --------------- | -------------------------------------- |
|
|
| Statement coverage | No target | High coverage ≠ quality tests |
|
|
| Branch coverage | No target | Many branches are defensive/impossible |
|
|
| E2E test count | 5-10 | Critical paths only |
|
|
| Integration test count | 15-25 | API contracts |
|
|
| Unit test files | 1:1 with source | Colocated, proportional |
|
|
|
|
## When to Add Tests to Existing Code
|
|
|
|
Add tests when:
|
|
|
|
1. **Fixing a bug** - Add a test that would have caught it
|
|
2. **Refactoring** - Add tests before changing behavior
|
|
3. **Code review feedback** - Reviewer identifies risk
|
|
4. **Production incident** - Prevent recurrence
|
|
|
|
Do NOT add tests:
|
|
|
|
1. To increase coverage percentages
|
|
2. For code that hasn't changed in 6+ months
|
|
3. For code scheduled for deletion/replacement
|
|
|
|
## Consequences
|
|
|
|
**Positive:**
|
|
|
|
- Testing effort focuses on high-risk, high-value code
|
|
- Developers spend less time on low-value tests
|
|
- Test suite runs faster (fewer unnecessary tests)
|
|
- Maintenance burden decreases
|
|
|
|
**Negative:**
|
|
|
|
- Some defensive code paths remain untested
|
|
- Coverage percentages may not satisfy external audits
|
|
- Requires judgment calls that may be inconsistent
|
|
|
|
## Key Files
|
|
|
|
- `docs/adr/0010-testing-strategy-and-standards.md` - Testing mechanics
|
|
- `vitest.config.ts` - Coverage configuration
|
|
- `src/tests/` - Test utilities and setup
|
|
|
|
## Review Checklist
|
|
|
|
Before adding a new test, ask:
|
|
|
|
1. [ ] What user-visible behavior does this test protect?
|
|
2. [ ] Can this be tested at a lower level (unit vs integration)?
|
|
3. [ ] Does this test require `as any` or mock gymnastics?
|
|
4. [ ] Will this test break when implementation changes (brittle)?
|
|
5. [ ] Is the test setup simpler than the code being tested?
|
|
|
|
If any answer suggests low value, skip the test or simplify.
|