fix(test, frontend): raise Vitest timeouts for CI stalls - #7623
fix(test, frontend): raise Vitest timeouts for CI stalls#7623aglinxinyuan wants to merge 2 commits into
Conversation
The macOS leg of `build / frontend` goes red on a different unit test every few days -- always a timeout, never the same spec, always green on rerun. Three occurrences in the last four days: | Run | Test | Error | | --- | --- | --- | | 31665399757 | UserDatasetVersionCreatorComponent > onClickCreate ... | Test timed out in 5000ms | | 31630884042 | AdminUserComponent > sortByAffiliation ... | Hook timed out in 10000ms | | 31411656559 | WorkflowRuntimeStatisticsComponent > should create | Test timed out in 5000ms | The tests are not the problem: the runner stalls, and the stall lands on whichever test is executing. In run 31665399757 the offending spec file took 11727ms on macos-latest and 240ms on ubuntu-latest for the same commit; in an earlier run the same file took 219ms on macOS. Suite totals from that run show the same picture -- 252.88s wall on macOS vs 89.85s on ubuntu, with a cumulative test time of 307.69s vs 182.34s. macos-latest gives 3 cores and 7 GB against ubuntu's 4 and 16, so the jsdom + v8-coverage workers run under real memory pressure there. Raise testTimeout and hookTimeout to 30s in both Vitest configs, which absorbs a stall an order of magnitude worse than any observed so far. A spec that legitimately needs 30s is broken, and the job's own timeout still bounds a true hang. Per-test timeouts would be whack-a-mole: the next stall picks a different test. Also opt the frontend matrix out of fail-fast, as every other multi-leg matrix in build.yml already does. Today one flaky OS cancels the other two legs, which destroys exactly the evidence needed to tell a runner flake from a real break. Before: macOS stalls 5s -> that test fails -> ubuntu + windows cancelled After: macOS stalls 5s -> absorbed; a real break still fails all legs
github-actions
Bot
added
fix
frontend
Backport auto-label reportThis
|
Automated Reviewer SuggestionsBased on the
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7623 +/- ##
============================================
+ Coverage 88.96% 90.65% +1.68%
- Complexity 4338 4442 +104
============================================
Files 1178 1175 -3
Lines 46835 47140 +305
Branches 5226 5284 +58
============================================
+ Hits 41667 42733 +1066
+ Misses 3422 2695 -727
+ Partials 1746 1712 -34
☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
| config | throughput | MB/s | latency | max Δ latest / 7d | |
|---|---|---|---|---|---|
| 🟢 | bs=10 sw=10 sl=64 | 537 | 0.327 | 17,522/25,415/25,415 us | 🟢 -12.7% / 🔴 +55.5% |
| 🔴 | bs=100 sw=10 sl=64 | 1,204 | 0.735 | 82,463/107,002/107,002 us | 🔴 +7.1% / 🟢 +18.4% |
| ⚪ | bs=1000 sw=10 sl=64 | 1,391 | 0.849 | 718,547/768,727/768,727 us | ⚪ within ±5% / 🟢 +32.8% |
Baseline details
Latest main 310ab88 from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---|---|---|---|---|
| bs=10 sw=10 sl=64 | throughput | 537 tuples/sec | 532 tuples/sec | 779.28 tuples/sec | +0.9% | -31.1% |
| bs=10 sw=10 sl=64 | MB/s | 0.327 MB/s | 0.325 MB/s | 0.476 MB/s | +0.6% | -31.2% |
| bs=10 sw=10 sl=64 | p50 | 17,522 us | 17,756 us | 12,712 us | -1.3% | +37.8% |
| bs=10 sw=10 sl=64 | p95 | 25,415 us | 29,103 us | 16,345 us | -12.7% | +55.5% |
| bs=10 sw=10 sl=64 | p99 | 25,415 us | 29,103 us | 19,050 us | -12.7% | +33.4% |
| bs=100 sw=10 sl=64 | throughput | 1,204 tuples/sec | 1,232 tuples/sec | 1,017 tuples/sec | -2.3% | +18.3% |
| bs=100 sw=10 sl=64 | MB/s | 0.735 MB/s | 0.752 MB/s | 0.621 MB/s | -2.3% | +18.4% |
| bs=100 sw=10 sl=64 | p50 | 82,463 us | 80,237 us | 100,048 us | +2.8% | -17.6% |
| bs=100 sw=10 sl=64 | p95 | 107,002 us | 99,910 us | 106,477 us | +7.1% | +0.5% |
| bs=100 sw=10 sl=64 | p99 | 107,002 us | 99,910 us | 114,739 us | +7.1% | -6.7% |
| bs=1000 sw=10 sl=64 | throughput | 1,391 tuples/sec | 1,423 tuples/sec | 1,048 tuples/sec | -2.2% | +32.8% |
| bs=1000 sw=10 sl=64 | MB/s | 0.849 MB/s | 0.868 MB/s | 0.639 MB/s | -2.2% | +32.8% |
| bs=1000 sw=10 sl=64 | p50 | 718,547 us | 701,466 us | 976,350 us | +2.4% | -26.4% |
| bs=1000 sw=10 sl=64 | p95 | 768,727 us | 775,164 us | 1,022,084 us | -0.8% | -24.8% |
| bs=1000 sw=10 sl=64 | p99 | 768,727 us | 775,164 us | 1,053,520 us | -0.8% | -27.0% |
Raw CSV
config_idx,batch_size,schema_width,string_len,num_batches,total_ms,total_tuples,total_bytes,tuples_per_sec,mb_per_sec,lat_p50_us,lat_p95_us,lat_p99_us
0,10,10,64,20,372.77,200,128000,537,0.327,17521.94,25414.83,25414.83
1,100,10,64,20,1661.33,2000,1280000,1204,0.735,82462.69,107002.09,107002.09
2,1000,10,64,20,14378.99,20000,12800000,1391,0.849,718547.05,768726.96,768726.96
Yicong-Huang
left a comment
Yicong-Huang
left a comment
There was a problem hiding this comment.
🔴 1 must-fix · 2 advisory · 1 polish — well-diagnosed, correctly scoped CI-stability fix; the only blocker is the missing Closes linkage, the rest are accuracy fixes to comments and docs that would otherwise ship permanently.
Simplifications (2)
frontend/vitest.browser.config.ts:71— underbrowser.enabled: trueVitest's defaults are already 15s/30s, so this line is a no-op and TESTING.md's baseline is wrong here (advisory, see inline)frontend/vitest.config.ts:44— the "job's own timeout bounds a true hang" reassurance has no backing; the frontend job sets notimeout-minutes(advisory, see inline)
Conventions (1)
- Description: promote the
#6073mention toCloses #6073— no closing link now (linkage_ok: false), and #6073's follow-ups are exactly this PR's two changes, so it ships untracked (must-fix)
Polish: 1 quick touch-up (see inline comments).
Review follow-ups: the frontend job had no `timeout-minutes`, so the claim that "the job's own timeout bounds a true hang" rested on GitHub's implicit 6h cap; browser mode already resolves a 30s `hookTimeout` from `browser.enabled`, so setting it there restated the default; and the fail-fast rationale named "every other multi-leg matrix", which `amber-integration` and `pyamber` refute.
|
All four addressed in fb5ec5a, plus a description edit.
Description updated to match: per-config timeout rows, the new job cap, and the ceiling split (6x under jsdom, 2x in browser mode). |
Yicong-Huang
left a comment
Yicong-Huang
left a comment
There was a problem hiding this comment.
🟡 4 resolved · 0 open · 1 new (1 new = 1 newly introduced · 0 late catches)
All four round-1 findings verified fixed against the tree, not taken from the replies. The one new item arrived with the fix itself and blocks nothing.
Simplifications (1)
.github/workflows/build.yml:92— the job cap's sizing argument reads a step budget as an observed duration, and leaves no headroom (advisory, see inline)
Verification trace
Each resolved thread was re-checked against the working tree rather than the author's reply. grep -n hookTimeout frontend/vitest.browser.config.ts now returns only the comment line explaining the key's absence, so the no-op is genuinely gone. timeout-minutes: 30 sits at 4-space (job-level) indent at build.yml:94, which is what makes the vitest.config.ts:44 reassurance point at a real bound. Mapping each fail-fast: false line to its owning job confirms the opt-out set is exactly the four now named at :100-101.
TESTING.md's four baseline numbers were re-read from the pinned runtime, not from the docs: node_modules/vitest/dist/chunks/coverage.DM_a_rWm.js:538-539 in vitest 4.1.10 resolves testTimeout to 15s and hookTimeout to 30s under browser.enabled, and 5s / 10s otherwise — so the per-config row is correct on all four counts.
The timings behind the one new finding come from this PR's own run 31869822969 via the Actions API, per step: frontend totals 518s / 601s / 641s on ubuntu / macOS / windows, of which Install dependency is 37s / 49s / 74s.
| # 10x on the macOS leg. Recent green runs take 4-12 minutes and install | ||
| # alone may take 20, so 30 bounds a hang while still absorbing a cold | ||
| # yarn cache on a slow runner. |
There was a problem hiding this comment.
The 20 here is the Install dependency step's own budget (:142), not an observed duration — install measured 37-74s across this PR's three legs.
The conclusion then doesn't follow: if install did approach 20, the remaining steps still need 8-9.5 minutes (measured), putting the job at 28-29.5 against the 30 cap. No absorption is left in exactly the cold-cache-on-a-slow-runner case the sentence names.
The value is fine at 2.8x the worst leg; sizing it from observed runs like amber-integration (:375-376) makes the sentence carry it.
| # 10x on the macOS leg. Recent green runs take 4-12 minutes and install | |
| # alone may take 20, so 30 bounds a hang while still absorbing a cold | |
| # yarn cache on a slow runner. | |
| # 10x on the macOS leg. Recent green legs run 8-11 minutes end to end, so | |
| # 30 leaves roughly 3x headroom for a cold yarn cache on a slow runner | |
| # without masking a hang. |
What changes were proposed in this PR?
The macOS leg of
build / frontendgoes red on a different unit test every few days — always a timeout, never the same spec, always green on rerun. Three occurrences in the last four days:UserDatasetVersionCreatorComponent > onClickCreate creates a dataset with a sanitized name …Test timed out in 5000msAdminUserComponent > sortByAffiliation compares affiliations …Hook timed out in 10000msWorkflowRuntimeStatisticsComponent > should createTest timed out in 5000msRoot cause: the runner stalls, not the test. The stall lands on whichever test happens to be executing. From run
31665399757— one commit, one matrix, two OSes:That file is not systematically slow — it took 219 ms on macOS in an earlier run, and 443 ms locally. The 11.7 s is a stall. jsdom + v8-coverage workers on 3 cores / 7 GB run under real memory pressure, which is where multi-second pauses come from.
testTimeout5s → 30s,hookTimeout10s → 30sfrontend/vitest.config.tstestTimeout15s → 30s (hookTimeoutleft alone — browser mode already resolves 30s)frontend/vitest.browser.config.tsfail-fast: falseandtimeout-minutes: 30on the frontend job.github/workflows/build.ymlfrontend/TESTING.mdBumping the one test's timeout would be whack-a-mole — the next stall picks a different spec. 30 s absorbs a stall an order of magnitude worse than any observed; a spec that legitimately needs 30 s is broken, and the job now carries
timeout-minutes: 30to bound a true hang. Without it the job inherited GitHub's implicit 6h cap — billed at 10× on the macOS leg. Green legs run 4–12 minutes and the "Install dependency" step is already allowed 20 on its own, so 30 leaves room for a cold yarn cache without masking a hang.The
fail-fastopt-out followsplatform,platform-integration,agent-serviceandinfra, the jobs that already set it. Today one flaky OS cancels the other two legs, so the run no longer says whether the failure reproduces off that OS — exactly the evidence needed to tell a runner flake from a real break.If macOS keeps flaking after this, the next lever is capping
maxWorkerson that leg to cut memory pressure. Left out here: it trades wall clock for stability and can't be measured from a non-macOS box.amber-integrationandpyamberare also multi-leg without afail-fastopt-out; left alone to keep this PR to the frontend job.Any related issues, documentation, discussions?
Closes #6073
How was this PR tested?
No production code is touched; the change is to the test harness and CI config.
yarn test:ci(full jsdom suite, new config)beforeEach+ 8 s body passes (14 026 ms); the 8 s body fails on the old 5 s default. Removed before commitvitest@4.1.10(dist/chunks/coverage.DM_a_rWm.js:538-539):testTimeout ??= browser.enabled ? 15e3 : 5e3,hookTimeout ??= browser.enabled ? 3e4 : 1e4— so the droppedhookTimeoutline was setting the value it already resolved tobuild.ymlparsesyaml.safe_load→jobs.frontend['timeout-minutes'] === 30,strategy['fail-fast'] === false; the same pass over every job confirmsplatform,platform-integration,agent-service,infraare the ones opting outprettier-eslint --list-differentcleanThe flake itself can't be reproduced on demand — that's the nature of a runner stall. What this PR asserts is verifiable: the per-test ceiling the stalls blow past is 6× higher under jsdom (5 s → 30 s) and 2× in browser mode (15 s → 30 s), the surviving matrix legs still report their own results, and a genuine hang now ends at 30 minutes instead of 6 hours.
Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 5)