All checks were successful
Deploy to Test Environment / deploy-to-test (push) Successful in 49m39s
518 lines
18 KiB
Markdown
518 lines
18 KiB
Markdown
# 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`
|