# ADR-057: Test Remediation Post-API Versioning and Frontend Rework

**Date**: 2026-01-28

**Status**: Accepted

**Context**: Major test remediation effort completed after ADR-008 API versioning implementation and frontend style rework

## Context

Following the completion of ADR-008 Phase 2 (API Versioning Strategy) and a concurrent frontend style/design rework, the test suite experienced 105 test failures across unit tests and E2E tests. This ADR documents the systematic remediation effort, root cause analysis, and lessons learned to prevent similar issues in future migrations.

### Scope of Failures

| Test Type  | Failures | Total Tests | Pass Rate After Fix |
| ---------- | -------- | ----------- | ------------------- |
| Unit Tests | 69       | 3,392       | 100%                |
| E2E Tests  | 36       | 36          | 100%                |
| **Total**  | **105**  | **3,428**   | **100%**            |

### Root Causes Identified

The failures were categorized into six distinct categories:

1. **API Versioning Path Mismatches** (71 failures)
   - Test files using `/api/` instead of `/api/v1/`
   - Environment variables not set for API base URL
   - Integration and E2E tests calling unversioned endpoints

2. **Dark Mode Class Assertion Failures** (8 failures)
   - Frontend rework changed Tailwind dark mode utility classes
   - Test assertions checking for outdated class names

3. **Selected Item Styling Changes** (6 failures)
   - Component styling refactored to new design tokens
   - Test assertions expecting old CSS class combinations

4. **Admin-Only Component Visibility** (12 failures)
   - MainLayout tests not properly mocking admin role
   - ActivityLog component visibility tied to role-based access

5. **Mock Hoisting Issues** (5 failures)
   - Queue mocks not available during module initialization
   - Vitest's module hoisting order causing mock setup failures

6. **Error Log Path Hardcoding** (3 failures)
   - Route handlers logging hardcoded paths like `/api/flyers`
   - Test assertions expecting versioned paths `/api/v1/flyers`

## Decision

We implemented a systematic remediation approach addressing each failure category with targeted fixes while establishing patterns to prevent regression.

### 1. API Versioning Configuration Updates

**Files Modified**:

- `vite.config.ts`
- `vitest.config.e2e.ts`
- `vitest.config.integration.ts`

**Pattern Applied**: Centralize API base URL in Vitest environment variables

```typescript
// vite.config.ts - Unit test configuration
test: {
  env: {
    // ADR-008: Ensure API versioning is correctly set for unit tests
    VITE_API_BASE_URL: '/api/v1',
  },
  // ...
}

// vitest.config.e2e.ts - E2E test configuration
test: {
  env: {
    // ADR-008: API versioning - all routes use /api/v1 prefix
    VITE_API_BASE_URL: 'http://localhost:3098/api/v1',
  },
  // ...
}

// vitest.config.integration.ts - Integration test configuration
test: {
  env: {
    // ADR-008: API versioning - all routes use /api/v1 prefix
    VITE_API_BASE_URL: 'http://localhost:3099/api/v1',
  },
  // ...
}
```

### 2. E2E Test URL Path Updates

**Files Modified** (7 files, 31 URL occurrences):

- `src/tests/e2e/budget-journey.e2e.test.ts`
- `src/tests/e2e/deals-journey.e2e.test.ts`
- `src/tests/e2e/flyer-upload.e2e.test.ts`
- `src/tests/e2e/inventory-journey.e2e.test.ts`
- `src/tests/e2e/receipt-journey.e2e.test.ts`
- `src/tests/e2e/upc-journey.e2e.test.ts`
- `src/tests/e2e/user-journey.e2e.test.ts`

**Pattern Applied**: Update all hardcoded API paths to versioned endpoints

```typescript
// Before
const response = await getRequest().post('/api/auth/register').send({...});

// After
const response = await getRequest().post('/api/v1/auth/register').send({...});
```

### 3. Unit Test Assertion Updates for UI Changes

**Files Modified**:

- `src/features/flyer/FlyerDisplay.test.tsx`
- `src/features/flyer/FlyerList.test.tsx`

**Pattern Applied**: Update CSS class assertions to match new design system

```typescript
// FlyerDisplay.test.tsx - Dark mode class update
// Before
expect(image).toHaveClass('dark:brightness-75');
// After
expect(image).toHaveClass('dark:brightness-90');

// FlyerList.test.tsx - Selected item styling update
// Before
expect(selectedItem).toHaveClass('ring-2', 'ring-brand-primary');
// After
expect(selectedItem).toHaveClass('border-brand-primary', 'bg-teal-50/50', 'dark:bg-teal-900/10');
```

### 4. Admin-Only Component Test Separation

**File Modified**: `src/layouts/MainLayout.test.tsx`

**Pattern Applied**: Separate test cases for admin vs. regular user visibility

```typescript
describe('for authenticated users', () => {
  beforeEach(() => {
    mockedUseAuth.mockReturnValue({
      ...defaultUseAuthReturn,
      authStatus: 'AUTHENTICATED',
      userProfile: createMockUserProfile({ user: mockUser }),
    });
  });

  it('renders auth-gated components for regular users (PriceHistoryChart, Leaderboard)', () => {
    renderWithRouter(<MainLayout {...defaultProps} />);
    expect(screen.getByTestId('price-history-chart')).toBeInTheDocument();
    expect(screen.getByTestId('leaderboard')).toBeInTheDocument();
    // ActivityLog is admin-only, should NOT be present for regular users
    expect(screen.queryByTestId('activity-log')).not.toBeInTheDocument();
  });

  it('renders ActivityLog for admin users', () => {
    mockedUseAuth.mockReturnValue({
      ...defaultUseAuthReturn,
      authStatus: 'AUTHENTICATED',
      userProfile: createMockUserProfile({ user: mockUser, role: 'admin' }),
    });
    renderWithRouter(<MainLayout {...defaultProps} />);
    expect(screen.getByTestId('activity-log')).toBeInTheDocument();
  });
});
```

### 5. vi.hoisted() Pattern for Queue Mocks

**File Modified**: `src/routes/health.routes.test.ts`

**Pattern Applied**: Use `vi.hoisted()` to ensure mocks are available during module hoisting

```typescript
// Use vi.hoisted to create mock queue objects that are available during vi.mock hoisting.
// This ensures the mock objects exist when the factory function runs.
const { mockQueuesModule } = vi.hoisted(() => {
  // Helper function to create a mock queue object with vi.fn()
  const createMockQueue = () => ({
    getJobCounts: vi.fn().mockResolvedValue({
      waiting: 0,
      active: 0,
      failed: 0,
      delayed: 0,
    }),
  });

  return {
    mockQueuesModule: {
      flyerQueue: createMockQueue(),
      emailQueue: createMockQueue(),
      // ... additional queues
    },
  };
});

// Mock the queues.server module BEFORE the health router imports it.
vi.mock('../services/queues.server', () => mockQueuesModule);

// Import the router AFTER all mocks are defined.
import healthRouter from './health.routes';
```

### 6. Dynamic Error Log Paths

**Pattern Applied**: Use `req.originalUrl` instead of hardcoded paths in error handlers

```typescript
// Before (INCORRECT - hardcoded path)
req.log.error({ error }, 'Error in /api/flyers/:id:');

// After (CORRECT - dynamic path)
req.log.error({ error }, `Error in ${req.originalUrl.split('?')[0]}:`);
```

## Implementation Summary

### Files Modified (14 total)

| Category             | Files | Changes                                           |
| -------------------- | ----- | ------------------------------------------------- |
| Vitest Configuration | 3     | Added `VITE_API_BASE_URL` environment variables   |
| E2E Tests            | 7     | Updated 31 API endpoint URLs                      |
| Unit Tests           | 4     | Updated assertions for UI, mocks, and admin roles |

### Verification Results

After remediation, all tests pass in the dev container environment:

```text
Unit Tests:     3,392 passing
E2E Tests:      36 passing
Integration:    345/348 passing (3 known issues, unrelated)
Type Check:     Passing
```

## Consequences

### Positive

1. **Test Suite Stability**: All tests now pass consistently in the dev container
2. **API Versioning Compliance**: Tests enforce the `/api/v1/` path requirement
3. **Pattern Documentation**: Clear patterns established for future test maintenance
4. **Separation of Concerns**: Admin vs. user test cases properly separated
5. **Mock Reliability**: `vi.hoisted()` pattern prevents mock timing issues

### Negative

1. **Maintenance Overhead**: Future API version changes will require test updates
2. **Manual Migration**: No automated tool to update test paths during versioning

### Neutral

1. **Test Execution Time**: No significant impact on test execution duration
2. **Coverage Metrics**: Coverage percentages unchanged

## Best Practices Established

### 1. API Versioning in Tests

**Always use versioned API paths in tests**:

```typescript
// Good
const response = await request.get('/api/v1/users/profile');

// Bad
const response = await request.get('/api/users/profile');
```

**Configure environment variables centrally in Vitest configs** rather than in individual test files.

### 2. vi.hoisted() for Module-Level Mocks

When mocking modules that are imported at the top level of other modules:

```typescript
// Pattern: Define mocks with vi.hoisted() BEFORE vi.mock() calls
const { mockModule } = vi.hoisted(() => ({
  mockModule: {
    someFunction: vi.fn(),
  },
}));

vi.mock('./some-module', () => mockModule);

// Import AFTER mocks
import { something } from './module-that-imports-some-module';
```

### 3. Testing Conditional Component Rendering

When testing components that render differently based on user role:

1. Create separate `describe` blocks for each role
2. Set up role-specific mocks in `beforeEach`
3. Explicitly test both presence AND absence of role-gated components

### 4. CSS Class Assertions After UI Refactors

After frontend style changes:

1. Review component implementation for new class names
2. Update test assertions to match actual CSS classes
3. Consider using partial matching for complex class combinations:

```typescript
// Flexible matching for Tailwind classes
expect(element).toHaveClass('border-brand-primary');
// vs exact matching
expect(element).toHaveClass('border-brand-primary', 'bg-teal-50/50', 'dark:bg-teal-900/10');
```

### 5. Error Logging Paths

**Always use dynamic paths in error logs**:

```typescript
// Pattern: Use req.originalUrl for request path logging
req.log.error({ error }, `Error in ${req.originalUrl.split('?')[0]}:`);
```

This ensures error logs reflect the actual request URL including version prefixes.

## Migration Checklist for Future API Version Changes

When implementing a new API version (e.g., v2), follow this checklist:

- [ ] Update `vite.config.ts` test environment `VITE_API_BASE_URL`
- [ ] Update `vitest.config.e2e.ts` test environment `VITE_API_BASE_URL`
- [ ] Update `vitest.config.integration.ts` test environment `VITE_API_BASE_URL`
- [ ] Search and replace `/api/v1/` with `/api/v2/` in E2E test files
- [ ] Search and replace `/api/v1/` with `/api/v2/` in integration test files
- [ ] Verify route handler error logs use `req.originalUrl`
- [ ] Run full test suite in dev container to verify

**Search command for finding hardcoded paths**:

```bash
grep -r "/api/v1/" src/tests/
grep -r "'/api/" src/routes/*.ts
```

## Related ADRs

- [ADR-008](./0008-api-versioning-strategy.md) - API Versioning Strategy
- [ADR-010](./0010-testing-strategy-and-standards.md) - Testing Strategy and Standards
- [ADR-014](./0014-containerization-and-deployment-strategy.md) - Platform: Linux Only
- [ADR-040](./0040-testing-economics-and-priorities.md) - Testing Economics and Priorities
- [ADR-012](./0012-frontend-component-library-and-design-system.md) - Frontend Component Library

## Key Files

| File                           | Purpose                                      |
| ------------------------------ | -------------------------------------------- |
| `vite.config.ts`               | Unit test environment configuration          |
| `vitest.config.e2e.ts`         | E2E test environment configuration           |
| `vitest.config.integration.ts` | Integration test environment configuration   |
| `src/tests/e2e/*.e2e.test.ts`  | E2E test files with versioned API paths      |
| `src/routes/*.routes.test.ts`  | Route test files with `vi.hoisted()` pattern |
| `docs/development/TESTING.md`  | Testing guide with best practices            |
