Repository navigation
Conversation
…ip tests] Also adds a temporary measurement of the slow tests under the emulators. The next commit removes it.
|
Updated 4:42 AM PT - Oct 28th, 2026
❌ @robobun, your commit f26dbea has 3 failures in
🧪 To try this PR locally: bunx bun-pr 45530That installs a local version of the PR into your bun-45530 --bun |
|
Status
|
…fy-baseline-emulated-test-timeout
The per-test timeout for the emulated test run stays.
|
No actionable comments were generated in the recent review. ℹ️ Recent review info
WalkthroughThe SIMD baseline test invocation now sets Bun’s test timeout to 90 seconds. The live-output option remains unchanged. ChangesSIMD baseline test
Priority: Merge Risk:
 |
| Check name | Status | Explanation |
|---|---|---|
| Linked Issues check |  |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
| Title check | |
The title clearly identifies the main change: adding a per-test timeout to the emulated Bun test run. |
| Description check | |
The description covers the problem, the fix, and verification, so it fits the template's intent. It explains that the step runs an emulator (Intel SDE on Windows, QEMU on Linux) and that the 5 s defau… |
Full details: Description check
-
Explanation
The description covers the problem, the fix, and verification, so it fits the template's intent. It explains that the step runs an emulator (Intel SDE on Windows, QEMU on Linux) and that the 5 s default timeout is too tight there. It gives the fix as
--timeout=90000on the emulated run, and it names builds 127645 and 134314 as verification. The template asks for two sections: 'What does this PR do?' and 'How did you verify your code works?'. The author uses 'Problem', 'Fix', and 'Background' headings instead, and the 'How did you verify' content sits inside 'Fix' as one bullet. The description also carries a very long collapsed notes block with statistics and CI logs. The verification evidence (build numbers, pass results on Windows and four Linux lanes) is specific, so this is a minor structural gap, not a missing section. Two items need attention. The evidence block mentions a temporary probe test file (simd-baseline-probe.test.ts) that the file summary does not list as touched. The objectives say that test was removed afterward, so the final diff should not include it. The description also does not explicitly map to the template's headings.
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 @coderabbitai help to get the list of available commands.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.
LGTM — a one-argument change to a CI verification script, and the inline note about the duplicated 90000 is a nit, not a blocker.
Beyond that, I checked that the fixture sets no setDefaultTimeout or per-test timeouts of its own (so the flag is what governs), that the temporary probe test from the measurement commit is gone from the net diff, and that the hang bound is unchanged in kind: runTest had no spawn-level timeout before this PR either, so the Buildkite step limit remains the outer bound as it was.
Extended reasoning...
The diff touches only scripts/verify-baseline.ts, adding --timeout=90000 to the emulated bun test invocation of the SIMD baseline fixture plus a two-line comment; no test file or production code changes. It touches no security-sensitive surface. The value matches what scripts/runner.node.ts:1165 computes for native non-ASAN runs (Math.ceil(180000/2) = 90000), the fixture itself declares no timeouts, and the step is a CPU-instruction check where wall time is an emulator artifact rather than the property under test. No CODEOWNERS entry covers scripts/, the hunt ran dry, and the only surviving finding is a maintainability nit about the literal not being derived from runner.node.ts.
The value no longer refers to the timeout of the CI test runner. The two are equal now, but the emulated run needs its own value.
There was a problem hiding this comment.
LGTM, a small CI-script-only change; the latest commit names the constant and drops the claim that it mirrors the native runner, which resolves my earlier nit.
What was reviewed:
--timeout=<ms>spelling and flag-before-file ordering match whatscripts/runner.node.tsalready passes tobun test(lines 1189-1195), andsrc/runtime/cli/Arguments.rs:584declares--timeoutas abun testparameter.- Only the Phase 1
bun testinvocation is affected; Phase 2 fixtures run asbun <file>via the samerunTesthelper and never see the flag. - Diff touches only
scripts/verify-baseline.ts; the temporary probe test file mentioned in the description is not in the final diff.
Extended reasoning...
The diff is 4 insertions and 1 deletion in scripts/verify-baseline.ts, a CI helper that runs the Bun binary under QEMU/SDE; it adds a named 90_000 ms constant and passes --timeout to the emulated bun test run. No runtime, security, or user-facing surface is touched and scripts/ is not covered by CODEOWNERS. The flag form and ordering are the same ones runner.node.ts already uses, and the bug hunt ran dry with no findings. The one prior inline nit (unlinked literal with a mirroring claim in the comment) was addressed by the final commit, and no third-party change requests are present in the timeline.
The same change as #45530. Under Intel SDE the first JPEG test of simd-baseline.test.ts takes 2 to 5.7 s, against a default timeout of 5 s.
…fy-baseline-emulated-test-timeout # Conflicts: # scripts/verify-baseline.ts
There was a problem hiding this comment.
LGTM — a four-line change that gives the emulated bun test run its own per-test timeout; since the last pass the rationale comment was reworded and main was merged in, and the diff is otherwise unchanged.
What was reviewed:
--timeout=<ms>spelling and placement match how scripts/runner.node.ts already invokesbun test, so the flag is accepted as written.- The timeout only changes when a stuck emulated test is reported, not what the step verifies: instruction violations still surface through
isInstructionViolation, and non-zero exits still count as failures. - Checked the hang case: a single stuck test now takes 90 s instead of 5 s to fail, well inside the step limit; only many simultaneous hangs would reach the outer 10-minute kill, which is a pre-existing property of the step.
Extended reasoning...
The diff touches only scripts/verify-baseline.ts, adding a named 90_000 ms constant and passing --timeout= to the Phase 1 emulated bun test invocation; the static scan, AOT phases, and the runTest helper are untouched. It is CI tooling with no security-sensitive surface (no auth, crypto, or user input). The change is small, self-contained, and mirrors an existing invocation pattern in scripts/runner.node.ts, no CODEOWNERS entry covers scripts/, the bug hunt ran dry with no findings, and the only outstanding thread was my own nit, which a later commit addressed by rewording the comment so it no longer makes a claim about another file.
|
Update on 2026-10-28.
The description has the numbers. |
Problem
windows x64 - verify-baselinefails when a test oftest/js/bun/jsc-stress/fixtures/simd-baseline.test.tstakes more than 5 s. Build 134210:gradient round-trips through baseline and progressive encoders [5010.54ms],this test timed out after 5000ms. From 2026-10-23 to 2026-10-28 this failed about 4% of the runs. Until 2026-10-17 it was 7 of 8353.scripts/verify-baseline.ts:240runsbun testwith no--timeout. The 5 s default assumes native speed. Under Intel SDE the file takes 27.13 s, on the Windows test lane of the same build 57 ms.Fix
--timeout=90000to the emulatedbun testrun. The test file does not change.scripts/runner.node.tspasses to every nativebun testrun in CI.Background
fetchround trip takes 1.65 s the first time and 12 to 32 ms the next time.bun build --compile --aot#45631 both tests are fast again. Native runs did not change.--timeout.Notes
Since 2026-10-21 (builds 130413 to 134312, 2972 finished runs of the step):
:after(b as B)or<B[]>(b)is not an arrow function return type (TypeScript) #46209, react-compiler: do not compile a component where a joined function gets a wrong type #47117, bundler: give each metafile record of a build with two graphs its own key #47083, bundler: run Bun.build plugin hooks in the AsyncLocalStorage stores of the Bun.build() call #41562, Cut the RustDOMExceptionCodeto the codes Rust sends and pin them from C++ #47055, repl: declare top-level functions in front of the wrapper and accept export statements #46336.bun testuntil the first test result went from about 19 s to about 21 s. The Windows binary grew from 17.8 to 22.2 million instructions between 10-12 and 10-28.What moved the two JPEG tests. I read all 289 finished runs of the step in builds 130950 to 131329 (2026-10-22 15:35 to 2026-10-23 14:47 UTC) and split them by the commits of main that the build contains:
bun build --compile --aot#45631: in the 11 runs I read, the gradient test takes 3.6 to 5.1 s and the file 22.5 to 27.5 s. These are the builds behind the 8% of 10-27.bun build --compile --aot#45631 (merged 2026-10-27 23:50 UTC): in the 7 runs I read, flat grey takes 2.6 to 3.2 s, gradient 2.4 to 2.7 s, llhttp 1.3 to 1.5 s and the file 16.3 to 18.9 s. None timed out. So the rate will go down again while branches pick up main, until the next change moves the emulated timings.bun build --compile --aot#45631. The native run of the file took 57 to 68 ms the whole time.Until 2026-10-17
The seven timeouts (builds 115651 to 126650, 8353 finished runs of the step):
Runs that pass (227 sampled runs, builds 119840 to 126640, Windows under SDE):
The other 27 tests take less than 0.5 s each. The process needs about 19 s before the first test result. On the Linux lanes (QEMU) the same three tests take about 0.1 to 0.2 s each and the file takes 1.1 to 1.4 s.
Measurement (build 126658, commit eb39398, a temporary test file that ran each step of the three slow tests three times in one process):
Bun.serve+fetch+text()on localhostzlib.deflatecalls (new threads)What changes for a hang. A test that hangs under the emulator now fails after 90 s, not after 5 s. The step limit is 10 minutes. A test that fails makes the step fail, and the step has no automatic retry for that.
Not part of this change. Most Windows runs of this step print a crash report after the test summary (
panic(main thread): Segmentation fault). The exit code stays 0 and the step passes. It is a different defect: the stack shows a wake of the mimalloc scavenger thread duringExitProcess. It happens after the last test, so it does not cause the timeouts.[auto-merge] gate passed · iteration 0 · 2 files touched
passes on PR (with fix)
diff hotspot
gate history · 1 passed · 0 rejected · iteration 0
evidence per changed file