fix(frontend): isolate test data dirs and remove load-sensitive sleeps (fix parallel test flakes)#416
fix(frontend): isolate test data dirs and remove load-sensitive sleeps (fix parallel test flakes)#416OBenner wants to merge 2 commits into
Conversation
Full parallel vitest runs failed 1-10 random tests per run (the failing set changed every run; each test passed in isolation). Three independent race sources: 1. setup.ts rm -rf'd the shared /tmp/auto-code-ui-tests in beforeEach while parallel workers were using it; the computed per-test testId subdir was dead code. TEST_DATA_DIR is now a unique per-test dir under a per-worker base (pid + VITEST_POOL_ID), and afterEach cleans only that dir - the shared parent is never deleted. 2. The five security-*-e2e specs all shared cwd/test-data and unlinked each other's fixture files in beforeAll/afterAll when scheduled into different workers. Each spec now uses its own tmpdir-scoped dir. 3. Fixed sleeps lost under 16-worker load: project-store.test.ts waited a flat 50ms for ProjectStore's fire-and-forget initializeAsync(), so addProject minted a fresh UUID instead of loading the pre-seeded project; window-security.test.ts waited a flat 100ms for app.whenReady() before reading captured BrowserWindow options. ProjectStore now exposes whenReady() (the init promise) which the tests await, and window-security polls until options are captured. Verified: 3 consecutive full npm run test runs green (161 files, 3898 passed / 6 skipped each). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Oleg Miagkov <mrobenner@gmail.com>
|
Warning Review limit reached
Next review available in: 43 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (9)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…porary-file) Predictable names under the shared os.tmpdir() triggered 61 CWE-377 alerts: another local user could pre-create or symlink the path. mkdtempSync creates an unpredictable directory with 0700 permissions, which also keeps the per-worker/per-file isolation this PR introduced. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Oleg Miagkov <mrobenner@gmail.com>
|
1 participant
Problem
A full
npm run testinapps/frontendfailed 1–10 random tests per run under local parallel vitest workers (window-security.test.ts, security-export-e2e.spec.ts, security-level-presets-e2e.spec.ts and others); the failing set changed every run and every test passed in isolation.Root causes (three independent races)
1.
src/__tests__/setup.tsdeleted a shared directory while parallel workers were using it.The
beforeEachhookrm -rf'd the shared/tmp/auto-code-ui-testson every test and recreated it. It even computed a unique per-testtestIdsubdirectory — but never used it (_testDirwas dead code).2. The five
security-*-e2e.spec.tsfiles sharedprocess.cwd()/test-data.Each spec's
beforeAll/afterAllunlinked.auto-claude-security.json/ audit-log fixtures andrmdir'd the directory — clobbering sibling specs scheduled into other workers. This is what actually broke the namedsecurity-*failures.3. Fixed sleeps that lose under 16-worker load.
project-store.test.ts:waitForStoreInit()slept a flat 50 ms hopingProjectStore's fire-and-forgetinitializeAsync()had finished; under load it hadn't, soaddProjectminted a random UUID instead of finding the pre-seeded project.window-security.test.ts: three tests slept a flat 100 ms after importing the main module before asserting on capturedBrowserWindowoptions.Fix
TEST_DATA_DIRis now a unique per-test directory under a per-worker base —os.tmpdir()/auto-code-ui-tests/worker-<pid>-<VITEST_POOL_ID>(pid covers the forks pool, pool id covers threads).afterEachremoves only that test's directory; the shared parent is never deleted.os.tmpdir()/auto-code-ui-tests/security-<name>-<pid>directory.whenReady()method exposing the init promise (additive, no behavior change); the test helper awaits it deterministically (all 17 call sites updated).BrowserWindowconstructor has fired (5 s cap) instead of sleeping 100 ms.Verification
npm run testruns inapps/frontend: all green (161/161 files, 3898 passed / 6 skipped each; before the fix, the first run reproduced a flake).tsc --noEmitclean; biome reports no new issues on touched files.🤖 Generated with Claude Code