Torben Sorensen
0d4b028a66
All checks were successful
Deploy to Test Environment / deploy-to-test (push) Successful in 21m49s
162 lines
5.4 KiB
Markdown
162 lines
5.4 KiB
Markdown
# Unit Test Fix Plan: Error Log Path Mismatches
|
|
|
|
**Date**: 2026-01-27
|
|
**Type**: Technical Implementation Plan
|
|
**Related**: [ADR-008: API Versioning Strategy](../adr/0008-api-versioning-strategy.md)
|
|
**Status**: Ready for Implementation
|
|
|
|
---
|
|
|
|
## Problem Statement
|
|
|
|
16 unit tests fail due to error log message assertions expecting versioned paths (`/api/v1/`) while route handlers emit hardcoded unversioned paths (`/api/`).
|
|
|
|
**Failure Pattern**:
|
|
|
|
```text
|
|
AssertionError: expected "Error PUT /api/users/profile" to contain "/api/v1/users/profile"
|
|
```
|
|
|
|
**Scope**: All failures are `toContain` assertions on `logger.error()` call arguments.
|
|
|
|
---
|
|
|
|
## Root Cause Analysis
|
|
|
|
| Layer | Behavior | Issue |
|
|
| ------------------ | ----------------------------------------------------- | ------------------- |
|
|
| Route Registration | `server.ts` mounts at `/api/v1/` | Correct |
|
|
| Request Path | `req.path` returns `/users/profile` (router-relative) | No version info |
|
|
| Error Handlers | Hardcode `"Error PUT /api/users/profile"` | Version mismatch |
|
|
| Test Assertions | Expect `"/api/v1/users/profile"` | Correct expectation |
|
|
|
|
**Root Cause**: Error log statements use template literals with hardcoded `/api/` prefix instead of `req.originalUrl` which contains the full versioned path.
|
|
|
|
**Example**:
|
|
|
|
```typescript
|
|
// Current (broken)
|
|
logger.error(`Error PUT /api/users/profile: ${err}`);
|
|
|
|
// Expected
|
|
logger.error(`Error PUT ${req.originalUrl}: ${err}`);
|
|
// Output: "Error PUT /api/v1/users/profile: ..."
|
|
```
|
|
|
|
---
|
|
|
|
## Solution Approach
|
|
|
|
Replace hardcoded path strings with `req.originalUrl` in all error log statements.
|
|
|
|
### Express Request Properties Reference
|
|
|
|
| Property | Example Value | Use Case |
|
|
| ----------------- | ------------------------------- | ----------------------------- |
|
|
| `req.originalUrl` | `/api/v1/users/profile?foo=bar` | Full URL with version + query |
|
|
| `req.path` | `/profile` | Router-relative path only |
|
|
| `req.baseUrl` | `/api/v1/users` | Mount point |
|
|
|
|
**Decision**: Use `req.originalUrl` for error logging to capture complete request context.
|
|
|
|
---
|
|
|
|
## Implementation Plan
|
|
|
|
### Affected Files
|
|
|
|
| File | Error Statements | Methods |
|
|
| ------------------------------- | ---------------- | ---------------------------------------------------- |
|
|
| `src/routes/users.routes.ts` | 3 | `PUT /profile`, `POST /profile/password`, `DELETE /` |
|
|
| `src/routes/recipe.routes.ts` | 2 | `POST /import`, `POST /:id/fork` |
|
|
| `src/routes/receipts.routes.ts` | 2 | `POST /`, `PATCH /:id` |
|
|
| `src/routes/flyers.routes.ts` | 2 | `POST /`, `PUT /:id` |
|
|
|
|
**Total**: 9 error log statements across 4 route files
|
|
|
|
### Parallel Implementation Tasks
|
|
|
|
All 4 files can be modified independently:
|
|
|
|
**Task 1**: `users.routes.ts`
|
|
|
|
- Line patterns: `Error PUT /api/users/profile`, `Error POST /api/users/profile/password`, `Error DELETE /api/users`
|
|
- Change: Replace with `Error ${req.method} ${req.originalUrl}`
|
|
|
|
**Task 2**: `recipe.routes.ts`
|
|
|
|
- Line patterns: `Error POST /api/recipes/import`, `Error POST /api/recipes/:id/fork`
|
|
- Change: Replace with `Error ${req.method} ${req.originalUrl}`
|
|
|
|
**Task 3**: `receipts.routes.ts`
|
|
|
|
- Line patterns: `Error POST /api/receipts`, `Error PATCH /api/receipts/:id`
|
|
- Change: Replace with `Error ${req.method} ${req.originalUrl}`
|
|
|
|
**Task 4**: `flyers.routes.ts`
|
|
|
|
- Line patterns: `Error POST /api/flyers`, `Error PUT /api/flyers/:id`
|
|
- Change: Replace with `Error ${req.method} ${req.originalUrl}`
|
|
|
|
### Verification
|
|
|
|
```bash
|
|
podman exec -it flyer-crawler-dev npm run test:unit
|
|
```
|
|
|
|
**Expected**: 16 failures → 0 failures (3,391/3,391 passing)
|
|
|
|
---
|
|
|
|
## Test Files Affected
|
|
|
|
Tests that will pass after fix:
|
|
|
|
| Test File | Failing Tests |
|
|
| ------------------------- | ------------- |
|
|
| `users.routes.test.ts` | 6 |
|
|
| `recipe.routes.test.ts` | 4 |
|
|
| `receipts.routes.test.ts` | 3 |
|
|
| `flyers.routes.test.ts` | 3 |
|
|
|
|
---
|
|
|
|
## Expected Outcomes
|
|
|
|
| Metric | Before | After |
|
|
| ------------------ | ----------- | ------------------- |
|
|
| Unit test failures | 16 | 0 |
|
|
| Unit tests passing | 3,375/3,391 | 3,391/3,391 |
|
|
| Integration tests | 345/348 | 345/348 (unchanged) |
|
|
|
|
### Benefits
|
|
|
|
1. **Version-agnostic logging**: Error messages automatically reflect actual request URL
|
|
2. **Future-proof**: No changes needed when v2 API is introduced
|
|
3. **Debugging clarity**: Logs show exact URL including query parameters
|
|
4. **Consistency**: All error handlers follow same pattern
|
|
|
|
---
|
|
|
|
## Implementation Notes
|
|
|
|
### Pattern to Apply
|
|
|
|
**Before**:
|
|
|
|
```typescript
|
|
logger.error(`Error PUT /api/users/profile: ${error.message}`);
|
|
```
|
|
|
|
**After**:
|
|
|
|
```typescript
|
|
logger.error(`Error ${req.method} ${req.originalUrl}: ${error.message}`);
|
|
```
|
|
|
|
### Edge Cases
|
|
|
|
- `req.originalUrl` includes query string if present (acceptable for debugging)
|
|
- No sanitization needed as URL is from Express parsed request
|
|
- Works correctly with route parameters (`:id` becomes actual value)
|