# ADR-0042: Browser Test Performance Optimization

**Status**: Accepted
**Date**: 2026-02-10
**Authors**: Claude Code AI Agent

## Context

### Current State

The stock-alert project has 64 Playwright browser tests across 5 spec files taking approximately 240 seconds (~4 minutes) to execute. Analysis reveals three major performance bottlenecks:

| Metric                              | Count | Impact                                       |
| ----------------------------------- | ----- | -------------------------------------------- |
| Hardcoded `waitForTimeout()` calls  | 66    | ~120s cumulative wait time                   |
| Redundant login calls per test      | 43    | ~2-3s each = 86-129s overhead                |
| Visual regression tests blocking CI | 4     | Cannot run in parallel with functional tests |

### Test Distribution

| File                | Tests  | `waitForTimeout` Calls | `login()` Calls          |
| ------------------- | ------ | ---------------------- | ------------------------ |
| `dashboard.spec.js` | 10     | 8                      | 10                       |
| `alerts.spec.js`    | 14     | 25                     | 1 (beforeEach)           |
| `gaps.spec.js`      | 20     | 29                     | 1 (beforeEach)           |
| `login.spec.js`     | 11     | 4                      | 0 (tests login itself)   |
| `visual.spec.js`    | 4      | 0                      | 4 (via navigateWithAuth) |
| **Total**           | **59** | **66**                 | **16 patterns**          |

### Root Causes

1. **Anti-Pattern: Hardcoded Timeouts**
   - `waitForTimeout(2000)` used to "wait for data to load"
   - Unnecessarily slow on fast systems, flaky on slow systems
   - No correlation to actual page readiness

2. **Anti-Pattern: Per-Test Authentication**
   - Each test navigates to `/login`, enters password, submits
   - Session cookie persists across requests but not across tests
   - `beforeEach` login adds 2-3 seconds per test

3. **Architecture: Mixed Test Types**
   - Visual regression tests require different infrastructure (baseline images)
   - Functional tests and visual tests compete for worker slots
   - Cannot optimize CI parallelization

### Requirements

1. Reduce test suite runtime by 40-55%
2. Improve test determinism (eliminate flakiness)
3. Maintain test coverage and reliability
4. Enable parallel CI execution where possible
5. Document patterns for other projects

## Decision

Implement three optimization phases:

### Phase 1: Event-Based Wait Replacement (Primary Impact: ~50% of time savings)

Replace all 66 `waitForTimeout()` calls with Playwright's event-based waiting APIs.

**Replacement Patterns:**

| Current Pattern                         | Replacement                                       | Rationale                     |
| --------------------------------------- | ------------------------------------------------- | ----------------------------- |
| `waitForTimeout(2000)` after navigation | `waitForLoadState('networkidle')`                 | Waits for network quiescence  |
| `waitForTimeout(1000)` after click      | `waitForSelector('.result')`                      | Waits for specific DOM change |
| `waitForTimeout(3000)` for charts       | `waitForSelector('canvas', { state: 'visible' })` | Waits for chart render        |
| `waitForTimeout(500)` for viewport      | `waitForFunction(() => ...)`                      | Waits for layout reflow       |

**Implementation Examples:**

```javascript
// BEFORE: Hardcoded timeout
await page.goto('/alerts');
await page.waitForTimeout(2000);
const rows = await page.locator('tbody tr').count();

// AFTER: Event-based wait
await page.goto('/alerts');
await page.waitForLoadState('networkidle');
await page.waitForSelector('tbody tr', { state: 'attached' });
const rows = await page.locator('tbody tr').count();
```

```javascript
// BEFORE: Hardcoded timeout after action
await page.click('#runCheckBtn');
await page.waitForTimeout(2000);

// AFTER: Wait for response
const [response] = await Promise.all([
  page.waitForResponse((resp) => resp.url().includes('/api/check')),
  page.click('#runCheckBtn'),
]);
```

**Helper Function Addition to `helpers.js`:**

```javascript
/**
 * Waits for page to be fully loaded with data.
 * Replaces hardcoded waitForTimeout calls.
 */
async function waitForPageReady(page, options = {}) {
  const { dataSelector = null, networkIdle = true, minTime = 0 } = options;

  const promises = [];

  if (networkIdle) {
    promises.push(page.waitForLoadState('networkidle'));
  }

  if (dataSelector) {
    promises.push(page.waitForSelector(dataSelector, { state: 'visible' }));
  }

  if (minTime > 0) {
    promises.push(page.waitForTimeout(minTime)); // Escape hatch for animations
  }

  await Promise.all(promises);
}
```

**Estimated Time Savings:** 60-80 seconds (eliminates ~120s of cumulative waits, but event waits have overhead)

### Phase 2: Global Authentication Setup (Primary Impact: ~35% of time savings)

Share authenticated session across all tests using Playwright's global setup feature.

**Architecture:**

```
                         ┌──────────────────┐
                         │  global-setup.js │
                         │                  │
                         │ 1. Login once    │
                         │ 2. Save storage  │
                         └────────┬─────────┘
                                  │
           ┌──────────────────────┼──────────────────────┐
           │                      │                      │
           ▼                      ▼                      ▼
  ┌─────────────────┐   ┌─────────────────┐   ┌─────────────────┐
  │ dashboard.spec  │   │  alerts.spec    │   │   gaps.spec     │
  │ (reuses auth)   │   │ (reuses auth)   │   │ (reuses auth)   │
  └─────────────────┘   └─────────────────┘   └─────────────────┘
```

**Implementation Files:**

**`tests/browser/global-setup.js`:**

```javascript
const { chromium } = require('@playwright/test');
const path = require('path');

const authFile = path.join(__dirname, '.auth', 'user.json');

module.exports = async function globalSetup() {
  const browser = await chromium.launch();
  const page = await browser.newPage();

  // Only perform login if authentication is enabled
  if (process.env.DASHBOARD_PASSWORD) {
    await page.goto(process.env.PLAYWRIGHT_BASE_URL || 'http://localhost:8980');

    // Perform login
    await page.goto('/login');
    await page.fill('#password', process.env.DASHBOARD_PASSWORD);
    await page.click('button[type="submit"]');
    await page.waitForURL('/');

    // Save authentication state
    await page.context().storageState({ path: authFile });
  }

  await browser.close();
};
```

**`playwright.config.js` Updates:**

```javascript
module.exports = defineConfig({
  // ... existing config ...

  // Global setup runs once before all tests
  globalSetup: require.resolve('./tests/browser/global-setup.js'),

  projects: [
    {
      name: 'chromium',
      use: {
        ...devices['Desktop Chrome'],
        // Reuse authentication state from global setup
        storageState: './tests/browser/.auth/user.json',
      },
    },
  ],
});
```

**Test File Updates:**

```javascript
// BEFORE: Login in beforeEach
test.beforeEach(async ({ page }) => {
  page.consoleErrors = captureConsoleErrors(page);
  if (isAuthEnabled()) {
    await login(page);
  }
});

// AFTER: Remove login (handled by global setup)
test.beforeEach(async ({ page }) => {
  page.consoleErrors = captureConsoleErrors(page);
  // Authentication already applied via storageState
});
```

**Estimated Time Savings:** 80-100 seconds (43 logins x ~2-3s each, minus 3s for global setup)

### Phase 3: Visual Test Separation (Primary Impact: CI parallelization)

Separate visual regression tests into a dedicated project for parallel CI execution.

**Project Configuration:**

```javascript
// playwright.config.js
module.exports = defineConfig({
  projects: [
    // Functional tests - fast, event-based
    {
      name: 'functional',
      testMatch: /^(?!.*visual).*\.spec\.js$/,
      use: {
        ...devices['Desktop Chrome'],
        storageState: './tests/browser/.auth/user.json',
      },
    },
    // Visual tests - separate baseline management
    {
      name: 'visual',
      testMatch: '**/visual.spec.js',
      use: {
        ...devices['Desktop Chrome'],
        storageState: './tests/browser/.auth/user.json',
      },
      // Different snapshot handling
      snapshotPathTemplate: '{testDir}/__screenshots__/{projectName}/{testFilePath}/{arg}{ext}',
    },
  ],
});
```

**CI Pipeline Updates:**

```yaml
# .gitea/workflows/test.yml
jobs:
  browser-functional:
    runs-on: ubuntu-latest
    steps:
      - run: npx playwright test --project=functional

  browser-visual:
    runs-on: ubuntu-latest
    steps:
      - run: npx playwright test --project=visual
```

**Estimated Time Savings:** 30-45 seconds (parallel execution vs sequential)

## Implementation Schedule

### Critical Path (Estimated 8-12 hours)

```
Phase 1 (Event Waits)      ████████████████ [4-6h]
                                          │
Phase 2 (Global Auth)                     ████████ [2-3h]
                                                  │
Phase 3 (Visual Separation)                       ████ [2-3h]
```

### Effort Summary

| Phase                    | Min Hours | Max Hours | Expected Savings      |
| ------------------------ | --------- | --------- | --------------------- |
| 1. Event-Based Waits     | 4         | 6         | 60-80s (25-33%)       |
| 2. Global Authentication | 2         | 3         | 80-100s (33-42%)      |
| 3. Visual Separation     | 2         | 3         | 30-45s (CI parallel)  |
| **Total**                | **8**     | **12**    | **170-225s (70-94%)** |

### Expected Results

| Metric             | Before | After     | Improvement   |
| ------------------ | ------ | --------- | ------------- |
| Total Runtime      | 240s   | 110-140s  | 42-54% faster |
| Flaky Test Rate    | ~5%    | <1%       | 80% reduction |
| CI Parallelization | None   | 2 workers | 2x throughput |
| Login Operations   | 43     | 1         | 98% reduction |
| Hardcoded Waits    | 66     | <5        | 92% reduction |

## Consequences

### Positive

1. **Performance**: 40-55% reduction in test runtime
2. **Reliability**: Event-based waits eliminate timing flakiness
3. **Scalability**: Global setup pattern scales to N tests with O(1) login cost
4. **CI Efficiency**: Parallel visual tests enable faster feedback loops
5. **Maintainability**: Centralized auth logic reduces code duplication
6. **Transferable Knowledge**: Patterns applicable to any Playwright project

### Negative

1. **Initial Migration Effort**: 8-12 hours of refactoring
2. **Learning Curve**: Team must understand Playwright wait APIs
3. **Global Setup Complexity**: Adds shared state between tests
4. **Debugging Harder**: Shared auth can mask test isolation issues

### Mitigations

| Risk               | Mitigation                                                    |
| ------------------ | ------------------------------------------------------------- |
| Global setup fails | Add retry logic; fallback to per-test login                   |
| Event waits flaky  | Keep small timeout buffer (100ms) as escape hatch             |
| Visual tests drift | Separate baseline management per environment                  |
| Test isolation     | Run `--project=functional` without auth for isolation testing |

### Neutral

- Test count unchanged (59 tests)
- Coverage unchanged
- Visual baselines unchanged (path changes only)

## Alternatives Considered

### Alternative 1: Reduce Test Count

**Rejected:** Sacrifices coverage for speed. Tests exist for a reason.

### Alternative 2: Increase Worker Parallelism

**Rejected:** Server cannot handle >2 concurrent sessions reliably; creates resource contention.

### Alternative 3: Use `page.waitForTimeout()` with Shorter Durations

**Rejected:** Addresses symptom, not root cause. Still creates timing-dependent tests.

### Alternative 4: Cookie Injection Instead of Login

**Rejected:** Requires reverse-engineering session format; brittle if auth changes.

### Alternative 5: HTTP API Authentication (No Browser)

**Rejected:** Loses browser session behavior validation; tests login flow.

## Implementation Details

### Wait Replacement Mapping

| File                | Current Timeouts       | Replacement Strategy                                                   |
| ------------------- | ---------------------- | ---------------------------------------------------------------------- |
| `dashboard.spec.js` | 1000ms, 2000ms, 3000ms | `waitForSelector` for charts, `waitForLoadState` for navigation        |
| `alerts.spec.js`    | 500ms, 1000ms, 2000ms  | `waitForResponse` for API calls, `waitForSelector` for table rows      |
| `gaps.spec.js`      | 500ms, 1000ms, 2000ms  | `waitForResponse` for `/api/gaps`, `waitForSelector` for summary cards |
| `login.spec.js`     | 500ms, 2000ms          | `waitForURL` for redirects, `waitForSelector` for error messages       |

### Common Wait Patterns for This Codebase

| Scenario              | Recommended Pattern                               | Example                   |
| --------------------- | ------------------------------------------------- | ------------------------- |
| After page navigation | `waitForLoadState('networkidle')`                 | Loading dashboard data    |
| After button click    | `waitForResponse()` + `waitForSelector()`         | Run Check button          |
| After filter change   | `waitForResponse(/api\/.*/)`                      | Status filter dropdown    |
| For chart rendering   | `waitForSelector('canvas', { state: 'visible' })` | Chart cards               |
| For modal appearance  | `waitForSelector('.modal', { state: 'visible' })` | Confirmation dialogs      |
| For layout change     | `waitForFunction()`                               | Responsive viewport tests |

### Auth Storage Structure

```
tests/browser/
├── .auth/
│   └── user.json          # Generated by global-setup, gitignored
├── global-setup.js        # Creates user.json
├── dashboard.spec.js      # Uses storageState
├── alerts.spec.js
├── gaps.spec.js
├── login.spec.js          # Tests login itself, may need special handling
└── visual.spec.js
```

**`.gitignore` Addition:**

```
tests/browser/.auth/
```

### Login.spec.js Special Handling

`login.spec.js` tests the login flow itself and must NOT use the shared auth state:

```javascript
// playwright.config.js
projects: [
  {
    name: 'functional',
    testMatch: /^(?!.*login).*\.spec\.js$/,
    use: { storageState: './tests/browser/.auth/user.json' },
  },
  {
    name: 'login',
    testMatch: '**/login.spec.js',
    use: { storageState: undefined }, // No auth - tests login flow
  },
];
```

## Testing the Optimization

### Baseline Measurement

```bash
# Before optimization: establish baseline
time npm run test:browser 2>&1 | tee baseline-timing.log
grep -E "passed|failed|skipped" baseline-timing.log
```

### Incremental Verification

```bash
# After Phase 1: verify wait replacement
npm run test:browser -- --reporter=list 2>&1 | grep -E "passed|failed|slow"

# After Phase 2: verify global auth
npm run test:browser -- --trace on
# Check trace for login occurrences (should be 1)

# After Phase 3: verify parallel execution
npm run test:browser -- --project=functional &
npm run test:browser -- --project=visual &
wait
```

### Success Criteria

| Metric                 | Target | Measurement                            |
| ---------------------- | ------ | -------------------------------------- |
| Total runtime          | <150s  | `time npm run test:browser`            |
| Login count            | 1      | Grep traces for `/login` navigation    |
| Flaky rate             | <2%    | 50 consecutive CI runs                 |
| `waitForTimeout` count | <5     | `grep -r waitForTimeout tests/browser` |

## Lessons Learned / Patterns for Other Projects

### Pattern 1: Always Prefer Event-Based Waits

```javascript
// Bad
await page.click('#submit');
await page.waitForTimeout(2000);
expect(await page.title()).toBe('Success');

// Good
await Promise.all([page.waitForNavigation(), page.click('#submit')]);
expect(await page.title()).toBe('Success');
```

### Pattern 2: Global Setup for Authentication

Playwright's `storageState` feature should be the default for any authenticated app:

1. Create `global-setup.js` that performs login once
2. Save cookies/storage to JSON file
3. Configure `storageState` in `playwright.config.js`
4. Tests start authenticated with zero overhead

### Pattern 3: Separate Test Types by Execution Characteristics

| Test Type  | Characteristics          | Strategy                          |
| ---------- | ------------------------ | --------------------------------- |
| Functional | Fast, deterministic      | Run first, gate deployment        |
| Visual     | Slow, baseline-dependent | Run in parallel, separate project |
| E2E        | Cross-service, slow      | Run nightly, separate workflow    |

### Pattern 4: Measure Before and After

Always establish baseline metrics before optimization:

```bash
# Essential metrics to capture
time npm run test:browser       # Total runtime
grep -c waitForTimeout *.js     # Hardcoded wait count
grep -c 'await login' *.js      # Login call count
```

## Related ADRs

- [ADR-0031](0031-quality-gates-eslint-playwright.md): Quality Gates - ESLint, Pre-commit Hooks, and Playwright Browser Testing
- [ADR-0035](0035-browser-test-selector-fixes.md): Browser Test Selector Fixes
- [ADR-0008](0008-testing-strategy.md): Testing Strategy

## References

- Playwright Best Practices: https://playwright.dev/docs/best-practices
- Playwright Authentication: https://playwright.dev/docs/auth
- Playwright Wait Strategies: https://playwright.dev/docs/actionability
- Test Files: `tests/browser/*.spec.js`
- Helper Module: `tests/browser/helpers.js`
- Configuration: `playwright.config.js`
