test(http): gate idle-WebSocket CPU check on the quietest sample, not the last - #34576
Conversation
The 'should not use 100% CPU when websocket is idle' fixture takes three 1-second CPU% samples and exits nonzero when the last one is >= 2%. That threshold is inside the noise floor on Windows, where GetProcessTimes (and so process.cpuUsage) accounts CPU time in ~15.6ms timer ticks: a couple of background-thread wakeups charged to the process during the third second read as 3-14%, and the test goes red with the event loop properly idle. On Windows 11 aarch64 CI this was failing in about 15% of runs; it also failed under the local ASAN debug build on Linux for the same reason. The regression this guards against (#25475) is a busy-spinning event loop that holds every sample near 100%, so gate on the minimum of the three samples and use the same 50% bound as test/regression/issue/21654. An idle loop always has at least one quiet sample well under that; the spin-loop has none.
|
Updated 1:59 AM PT - Jul 18th, 2026
❌ @robobun, your commit a7f9de0 has 2 failures in
🧪 To try this PR locally: bunx bun-pr 34576That installs a local version of the PR into your bun-34576 --bun |
|
Reproduced on Windows 11 aarch64 by running the fixture 100 times with the canary CI build 75187: |
WalkthroughThe WebSocket CPU fixture now records the minimum CPU usage observed across sampling intervals and uses that value, with a threshold of 50, to determine the process exit result. ChangesWebSocket CPU sampling decision
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — well-justified de-flake that keeps asserting the property the test protects.
What was reviewed:
- Confirmed the min-of-3-samples + 50% bound still catches the #25475 busy-spin regression (all samples ~100%) while absorbing Windows
GetProcessTimestick granularity and ASAN overhead. - Verified the 50% precedent cited at
test/regression/issue/21654/21654.test.ts:194for the same spin-vs-idle distinction. - Checked the parent test in
bun-server.test.ts— it only asserts on exit code, so no other call sites need updating.
Extended reasoning...
Overview
This PR touches a single test fixture, test/js/bun/http/bun-websocket-cpu-fixture.js, spawned by the "should not use 100% CPU when websocket is idle" test in bun-server.test.ts. The fixture samples process.cpuUsage() three times at 1s intervals; previously it exited nonzero if the third sample was ≥2%. The change tracks the minimum across all three samples and gates on min < 50% instead.
Security risks
None. Test-fixture-only change; no runtime, native, or user-facing code touched.
Level of scrutiny
Low. This is a targeted CI de-flake with a thorough root-cause analysis (Windows GetProcessTimes charges CPU in ~15.6ms tick multiples, so a single background wakeup can read as ~1.56–3.12%) and empirical before/after data (15/100 → 0/100 on the affected lane). The key REVIEW.md concern for de-flakes — "keep asserting the property the original assertion protected" — is satisfied: the #25475 regression is a busy-spinning event loop that pins every sample near 100%, so min < 50 still fails on it. The PR explicitly verified this with a setImmediate chain. The 50% bound matches existing precedent at test/regression/issue/21654/21654.test.ts:194 for the same spin-loop-vs-idle check.
Other factors
The threshold widening (2% → 50%) is significant but not silent: the in-code comment and PR description document the rationale, and the statistic change (last → min) is what actually eliminates the flake — the PR's own data shows the min never exceeded 3.05% across 100 runs. The parent test only inspects the fixture's exit code, so no coordinated changes elsewhere are needed. No prior reviews on the PR to address.
2 participants
test/js/bun/http/bun-server.test.ts→should not use 100% CPU when websocket is idlehas been going red on the Windows 11 aarch64 lane in roughly one build in six (e.g. 75052, 75150, 75179), always with the same shape:Cause
The fixture from #25475 samples
process.cpuUsage()three times at 1-second intervals and exits nonzero if the third sample is>= 2%. On Windows,process.cpuUsage()is backed byGetProcessTimes, which accounts CPU time at timer-tick boundaries (the default tick is ~15.625ms). A sample therefore reads as a multiple of ~1.56%: one tick is ~1.56%, two ticks ~3.12%, and so on. A couple of background-thread wakeups (mimalloc scavenger, JSC timers, TLS) that happen to straddle a tick boundary during the third second are enough to cross 2% with the event loop properly idle.Measured on a Windows 11 aarch64 box over 100 runs of the unmodified fixture: 15/100 had the third sample
>= 2%(max 13.85%), while the minimum across the three samples was0in 97 runs and never exceeded3.05%. The same assertion also fails under the local ASAN debug build on Linux (first sample ~28%, third ~3.7%) for the same overhead reason.Fix
The #25475 regression is a busy-spinning event loop that holds every sample near 100%, so the minimum across the three samples is the right statistic to test "not busy-looping": an idle loop always has at least one quiet sample, a spinning loop has none. Gate on that minimum and use the same 50% bound that
test/regression/issue/21654already uses for its spin-loop-vs-idle check. The regression would still read ~100% on all three samples and fail; scheduler/tick noise cannot reach it.Verification
bun bd test test/js/bun/http/bun-server.test.ts -t "should not use 100% CPU"on Linux now passes (was failing under ASAN).setImmediatechain) reports min ≈ 100% and the fixture exits 1.Open PR #33956 bumps the same threshold to 15% for
darwin-arm64only; this change is platform-agnostic and subsumes that hunk.no test proof · iteration 0 · docs-only change; test-proof not applicable