Files
Charon/docs/decisions/sprint1-timeout-remediation-findings.md
akanealw eec8c28fb3
Some checks are pending
Go Benchmark / Performance Regression Check (push) Waiting to run
Cerberus Integration / Cerberus Security Stack Integration (push) Waiting to run
Upload Coverage to Codecov / Backend Codecov Upload (push) Waiting to run
Upload Coverage to Codecov / Frontend Codecov Upload (push) Waiting to run
CodeQL - Analyze / CodeQL analysis (go) (push) Waiting to run
CodeQL - Analyze / CodeQL analysis (javascript-typescript) (push) Waiting to run
CrowdSec Integration / CrowdSec Bouncer Integration (push) Waiting to run
Docker Build, Publish & Test / build-and-push (push) Waiting to run
Docker Build, Publish & Test / Security Scan PR Image (push) Blocked by required conditions
Quality Checks / Auth Route Protection Contract (push) Waiting to run
Quality Checks / Codecov Trigger/Comment Parity Guard (push) Waiting to run
Quality Checks / Backend (Go) (push) Waiting to run
Quality Checks / Frontend (React) (push) Waiting to run
Rate Limit integration / Rate Limiting Integration (push) Waiting to run
Security Scan (PR) / Trivy Binary Scan (push) Waiting to run
Supply Chain Verification (PR) / Verify Supply Chain (push) Waiting to run
WAF integration / Coraza WAF Integration (push) Waiting to run
changed perms
2026-04-22 18:19:14 +00:00

9.7 KiB
Executable File
Raw Permalink Blame History

Sprint 1 - E2E Test Timeout Remediation Findings

Date: 2026-02-02 Status: In Progress Sprint: Sprint 1 (Quick Fixes - Priority Implementation)

Implemented Changes

Fix 1.1 + Fix 1.1b: Remove beforeEach polling, add afterEach cleanup

File: tests/settings/system-settings.spec.ts

Changes Made:

  1. Removed waitForFeatureFlagPropagation() call from beforeEach hook (lines 35-46)

    • This was causing 10s × 31 tests = 310s of polling overhead per shard
    • Commented out with clear explanation linking to remediation plan
  2. Added test.afterEach() hook with direct API state restoration:

    test.afterEach(async ({ page }) => {
      await test.step('Restore default feature flag state', async () => {
        const defaultFlags = {
          'cerberus.enabled': true,
          'crowdsec.console_enrollment': false,
          'uptime.enabled': false,
        };
    
        // Direct API mutation to reset flags (no polling needed)
        await page.request.put('/api/v1/feature-flags', {
          data: defaultFlags,
        });
      });
    });
    

Rationale:

  • Tests already verify feature flag state individually after toggle actions
  • Initial state verification in beforeEach was redundant
  • Explicit cleanup in afterEach ensures test isolation without polling overhead
  • Direct API mutation for state restoration is faster than polling

Expected Impact:

  • 310s saved per shard (10s × 31 tests)
  • Elimination of inter-test dependencies
  • No state leakage between tests

Fix 1.3: Implement request coalescing with fixed cache

File: tests/utils/wait-helpers.ts

Changes Made:

  1. Added module-level cache for in-flight requests:

    // Cache for in-flight requests (per-worker isolation)
    const inflightRequests = new Map<string, Promise<Record<string, boolean>>>();
    
  2. Implemented cache key generation with sorted keys and worker isolation:

    function generateCacheKey(
      expectedFlags: Record<string, boolean>,
      workerIndex: number
    ): string {
      // Sort keys to ensure {a:true, b:false} === {b:false, a:true}
      const sortedFlags = Object.keys(expectedFlags)
        .sort()
        .reduce((acc, key) => {
          acc[key] = expectedFlags[key];
          return acc;
        }, {} as Record<string, boolean>);
    
      // Include worker index to isolate parallel processes
      return `${workerIndex}:${JSON.stringify(sortedFlags)}`;
    }
    
  3. Modified waitForFeatureFlagPropagation() to use cache:

    • Returns cached promise if request already in flight for worker
    • Logs cache hits/misses for observability
    • Removes promise from cache after completion (success or failure)
  4. Added cleanup function:

    export function clearFeatureFlagCache(): void {
      inflightRequests.clear();
      console.log('[CACHE] Cleared all cached feature flag requests');
    }
    

Why Sorted Keys?

  • {a:true, b:false} vs {b:false, a:true} are semantically identical
  • Without sorting, they generate different cache keys → cache misses
  • Sorting ensures consistent key regardless of property order

Why Worker Isolation?

  • Playwright workers run in parallel across different browser contexts
  • Each worker needs its own cache to avoid state conflicts
  • Worker index provides unique namespace per parallel process

Expected Impact:

  • 30-40% reduction in duplicate API calls (revised from original 70-80% estimate)
  • Cache hit rate should be >30% based on similar flag state checks
  • Reduced API server load during parallel test execution

Investigation: Fix 1.2 - DNS Provider Label Mismatches

Status: Partially Investigated

Issue:

  • Test: tests/dns-provider-types.spec.ts (line 260)

  • Symptom: Label locator /script.*path/i passes in Chromium, fails in Firefox/WebKit

  • Test code:

    const scriptField = page.getByLabel(/script.*path/i);
    await expect(scriptField).toBeVisible({ timeout: 10000 });
    

Investigation Steps Completed:

  1. Confirmed E2E environment is running and healthy
  2. Attempted to run DNS provider type tests in Chromium
  3. ⏸️ Further investigation deferred due to test execution issues

Investigation Steps Remaining (per spec):

  1. Run with Playwright Inspector to compare accessibility trees:

    npx playwright test tests/dns-provider-types.spec.ts --project=chromium --headed --debug
    npx playwright test tests/dns-provider-types.spec.ts --project=firefox --headed --debug
    
  2. Use await page.getByRole('textbox').all() to list all text inputs and their labels

  3. Document findings in a Decision Record if labels differ

  4. If fixable: Update component to ensure consistent aria-labels

  5. If not fixable: Use the helper function approach from Phase 2

Recommendation:

  • Complete investigation in separate session with headed browser mode
  • DO NOT add .or() chains unless investigation proves it's necessary
  • Create formal Decision Record once root cause is identified

Validation Checkpoints

Checkpoint 1: Execution Time

Status: ⏸️ In Progress

Target: <15 minutes (900s) for full test suite

Command:

time npx playwright test tests/settings/system-settings.spec.ts --project=chromium

Results:

  • Test execution interrupted during validation
  • Observed: Tests were picking up multiple spec files from security/ folder
  • Need to investigate test file patterns or run with more specific filtering

Action Required:

  • Re-run with corrected test file path or filtering
  • Ensure only system-settings tests are executed
  • Measure execution time and compare to baseline

Checkpoint 2: Test Isolation

Status: Pending

Target: All tests pass with --repeat-each=5 --workers=4

Command:

npx playwright test tests/settings/system-settings.spec.ts --project=chromium --repeat-each=5 --workers=4

Status: Not executed yet

Checkpoint 3: Cross-browser

Status: Pending

Target: Firefox/WebKit pass rate >85%

Command:

npx playwright test tests/settings/system-settings.spec.ts --project=firefox --project=webkit

Status: Not executed yet

Checkpoint 4: DNS provider tests (secondary issue)

Status: Pending

Target: Firefox tests pass or investigation complete

Command:

npx playwright test tests/dns-provider-types.spec.ts --project=firefox

Status: Investigation deferred

Technical Decisions

Decision: Use Direct API Mutation for State Restoration

Context:

  • Tests need to restore default feature flag state after modifications
  • Original approach used polling-based verification in beforeEach
  • Alternative approaches: polling in afterEach vs direct API mutation

Options Evaluated:

  1. Polling in afterEach - Verify state propagated after mutation

    • Pros: Confirms state is actually restored
    • Cons: Adds 500ms-2s per test (polling overhead)
  2. Direct API mutation without polling (chosen)

    • Pros: Fast, predictable, no overhead
    • Cons: Assumes API mutation is synchronous/immediate
    • Why chosen: Feature flag updates are synchronous in backend

Rationale:

  • Feature flag updates via PUT /api/v1/feature-flags are processed synchronously
  • Database write is immediate (SQLite WAL mode)
  • No async propagation delay in single-process test environment
  • Subsequent tests will verify state on first read, catching any issues

Impact:

  • Test runtime reduced by 15-60s per test file (31 tests × 500ms-2s polling)
  • Risk: If state restoration fails, next test will fail loudly (detectable)
  • Acceptable trade-off for 10-20% execution time improvement

Review: Re-evaluate if state restoration failures observed in CI

Decision: Cache Key Sorting for Semantic Equality

Context:

  • Multiple tests may check the same feature flag state but with different property order
  • Without normalization, {a:true, b:false} and {b:false, a:true} generate different keys

Rationale:

  • JavaScript objects have insertion order, but semantically these are identical states
  • Sorting keys ensures cache hits for semantically identical flag states
  • Minimal performance cost (~1ms for sorting 3-5 keys)

Impact:

  • Estimated 10-15% cache hit rate improvement
  • No downside - pure optimization

Next Steps

  1. Complete Fix 1.2 Investigation:

    • Run DNS provider tests in headed mode with Playwright Inspector
    • Document actual vs expected label structure in Firefox/WebKit
    • Create Decision Record with root cause and recommended fix
  2. Execute All Validation Checkpoints:

    • Fix test file selection issue (why security tests run instead of system-settings)
    • Run all 4 checkpoints sequentially
    • Document pass/fail results with screenshots if failures occur
  3. Measure Impact:

    • Baseline: Record execution time before fixes
    • Post-fix: Record execution time after fixes
    • Calculate actual time savings vs predicted 310s savings
  4. Update Spec:

    • Document actual vs predicted impact
    • Adjust estimates for Phase 2 based on Sprint 1 findings

Code Review Checklist

  • Fix 1.1: Remove beforeEach polling
  • Fix 1.1b: Add afterEach cleanup
  • Fix 1.3: Implement request coalescing
  • Add cache cleanup function
  • Document cache key generation logic
  • Fix 1.2: Complete investigation
  • Run all validation checkpoints
  • Update spec with actual findings

References

  • Remediation Plan: docs/plans/current_spec.md
  • Modified Files:
    • tests/settings/system-settings.spec.ts
    • tests/utils/wait-helpers.ts
  • Investigation Target: tests/dns-provider-types.spec.ts (line 260)

Last Updated: 2026-02-02 Author: GitHub Copilot (Playwright Dev Mode) Status: Sprint 1 implementation complete, validation checkpoints pending