bun test: --timings for duration-balanced --shard/--parallel, docs for --parallel & --isolate

[Jarred-Sumner]

What

--timings=<path> (repeatable)	Reads { "version": 1, "files": { "rel/path.test.ts": ms } }; several files (one per CI shard from the previous run) are read as one table, missing paths skipped. --shard=i/n then cuts the path-sorted file list into n contiguous runs of ~equal total time (neighbouring files share imports → same process → module cache stays hot) instead of round-robin by count; every run gets at least one file. --parallel cuts worker chunks the same way, each worker starts its slowest file first, and an idle worker steals the slowest not-yet-started file from the chunk with the most time left.
--update-timings	Records each file's wall time and writes to the first --timings path, slowest-first so it doubles as a "what's slow" report. Under --shard it writes only the files that shard ran, so per-shard outputs are disjoint and next run just passes all of them — no merge step (docs have the Actions workflow). Without --shard it merges into what it read so a local partial run doesn't shrink the table. Atomic (tmp + rename, parent dir created), written on bail and Ctrl-C too. Missing file = fine; invalid JSON prints parser diagnostics; wrong shape = error.
FTL threshold under --isolate	thresholdForFTLOptimizeAfterWarmUp 64000 → 1000000 when each file gets a fresh global. See profile below.
docs/test/parallel.mdx	New page: --parallel (coordinator/workers, lazy scale-up, distribution, worker env, crash handling, when it helps/doesn't), --isolate, test.concurrent/--concurrent, --shard + --timings with a CI recipe and baseline-aware merge script, and a bun/jest/vitest table. Linked from nav, discovery, index.
bench/test/app	Generator + stock jest (@swc/jest) / vitest configs for the comparison.

Why the FTL change

Profiling bun test --isolate on the bench suite (128 TS files importing a zod + date-fns + lodash app, release canary, samply weighted by thread CPU):

	wall	CPU
bun test (shared global)	2.5s	3.1s
bun test --isolate	4.8s	13.7s
bun test --parallel	1.36s	17.3s
bun test --parallel, FTL threshold 1e6	0.90s	9.8s

Of the 14.8s --isolate CPU: JIT worklist threads 8.6s (DFG plan 7.2s incl. FTL::compile 2.7s + B3 2.5s), main 4.9s, GC helpers 1.2s. JSC::Parser is ~0.3s total — the SourceProvider/bytecode sharing across globals works; what's left is JIT re-tiering the same hot functions in every fresh global, and under --parallel those compiler threads compete with other workers for cores. A 200M-iteration hot-loop test is unchanged with the raised threshold (1781 → 1796 ms) and regresses with FTL off entirely (2061 ms), hence threshold rather than disable.

Benchmark (docs table)

hyperfine --warmup 1 --runs 5, M4 Max 16 cores, Bun 1.4 canary, Node 25.6, 128 files × 8 tests:

Runner	Wall	CPU
bun test --parallel	0.91s	9.9s
bun test	2.58s	3.1s
jest 30 (@swc/jest)	3.14s	29.4s
vitest 4.1	9.44s	107.6s

Tests

test/cli/test/test-shard.test.ts — 11 new: file shape/order/merge, shard writes only its own files to the first path, multi-file union in any order, empty shard writes empty table, parent-dir creation, missing/invalid/malformed file, time-balanced shards, files missing from the table, one file bigger than a shard (first and last in path order), shard update keeps the table complete for later shards, --parallel chunk cut + slowest-first dispatch + slowest-first steal. Fail on USE_SYSTEM_BUN=1, pass on the debug build; rust:check-all clean on all targets.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

Walkthrough

Changes

The test runner now supports per-file timing data for shard selection, parallel scheduling, and timing report updates. It also supports explicit isolation control. Documentation and benchmarks cover parallel execution, timing files, and isolation.

Test execution updates

Layer / File(s)	Summary
Benchmark fixture generation bench/test/...	The benchmark suite generates a shared application and compares Bun, Jest, and Vitest workloads.
Parallel execution documentation docs/docs.json, docs/test/...	The documentation describes parallel files, isolation, concurrency, sharding, timing files, and benchmark commands.
Isolation-aware JSC initialization src/jsc/lib.rs, src/jsc/bindings/ZigGlobalObject.cpp	JSC initialization accepts short_lived_globals and applies the configured FTL threshold when enabled.
Timing configuration and data model src/options_types/context.rs, src/runtime/cli/Arguments.rs, src/runtime/cli/mod.rs, src/runtime/cli/test/Timings.rs	The CLI accepts timing options. Timings validates, records, partitions, sorts, and writes timing data.
Timing-aware runner integration src/runtime/cli/test_command.rs, src/runtime/cli/test/parallel/..., test/cli/test/...	The reporter and workers use timing data for shard selection, balanced dispatch, crash accounting, report updates, and isolation behavior. Tests cover timing persistence, sharding, ordering, and --no-isolate.

Possibly related issues

• Bun Test: Firebase Cloud Functions Integration Test Fail #34486: The change adds file-level parallelism and --no-isolate controls related to cross-file concurrency and shared-state behavior.

Possibly related PRs

• oven-sh/bun#33622: Both changes cover timing-based sharding and parallel test scheduling.
• oven-sh/bun#36175: Both changes record per-file timings in parallel test execution.
• oven-sh/bun#36810: Both changes modify test isolation and parallel test execution paths.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly summarizes duration-based timing support and related parallel test documentation.
Description check	✅ Passed	The description explains the changes and verification results, although it uses different headings from the repository template.

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 6

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@bench/test/app/setup.ts`:
- Around line 8-10: Validate FILES and ITEMS immediately after parsing and
before any rmSync fixture reset, requiring finite safe integers, practical upper
bounds, and ITEMS >= 2; validate any configurable generator counts similarly.
Reject invalid values with an error containing the supplied value and permitted
range, preventing unbounded or impractical generation while preserving valid
defaults.

In `@docs/test/parallel.mdx`:
- Line 123: Update the default split description in parallel.mdx to state that
select_shard assigns each shard one contiguous, deterministic range of
neighboring files after sorting by path; remove the inaccurate “round-robin by
file count” wording while preserving the explanation that shards collectively
cover each file exactly once.

In `@src/runtime/cli/test_command.rs`:
- Line 2257: Update the reporter initialization around run_as_worker so
timings_file is not passed through Timings::load for test workers; leave
reporter.timings unset in that path while preserving timing loading for the
coordinator, which records timings from FileDone.

In `@src/runtime/cli/test/Timings.rs`:
- Around line 30-54: Update Timings::load to handle ParsedJson::parse_json
errors explicitly instead of discarding them with .ok(). When parsing fails,
print the populated log diagnostics and exit before applying the generic
version/files schema validation; preserve the existing validation path for
successfully parsed JSON.
- Around line 200-207: Update the timings-file write flow around File::create
and write_all to write the serialized output to a uniquely named temporary file
in the same directory, then atomically replace self.path with bun_sys::renameat
only after the write succeeds. Remove the temporary file when writing or
renaming fails, while preserving the existing Output::err reporting for write
failures.

In `@test/cli/test/test-shard.test.ts`:
- Around line 212-237: Move the “--update-timings writes { version, files }
sorted slowest-first and merges with existing entries” test out of the
describe.concurrent("--timings") block into a sequential describe block. Keep
its existing wall-clock assertions and test behavior unchanged, while leaving
non-timing-sensitive tests in the concurrent block.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 9f8573c4-0c41-4d99-8fde-35e1c4c6fde5

📥 Commits

Reviewing files that changed from the base of the PR and between 074656d and acf4225.

⛔ Files ignored due to path filters (1)

• bench/test/bun.lock is excluded by !**/*.lock

📒 Files selected for processing (20)

• bench/test/.gitignore
• bench/test/README.md
• bench/test/app/setup.ts
• bench/test/jest.config.cjs
• bench/test/package.json
• bench/test/vitest.config.ts
• docs/docs.json
• docs/test/discovery.mdx
• docs/test/index.mdx
• docs/test/parallel.mdx
• src/jsc/bindings/ZigGlobalObject.cpp
• src/jsc/lib.rs
• src/options_types/context.rs
• src/runtime/cli/Arguments.rs
• src/runtime/cli/mod.rs
• src/runtime/cli/test/Timings.rs
• src/runtime/cli/test/parallel/Coordinator.rs
• src/runtime/cli/test/parallel/runner.rs
• src/runtime/cli/test_command.rs
• test/cli/test/test-shard.test.ts

[claude]

Thanks — b7c1f6f addressed the abort-path nit (write_timings_if_needed() now runs before Global::exit in run_as_coordinator). No new issues found this pass, but deferring to a human for the user-facing API surface (--timings/--update-timings shape, file format version) and the thresholdForFTLOptimizeAfterWarmUp bump under --isolate.

Checked: partition() reserves ≥1 file per remaining bin (both big-first and big-last covered by tests); steal-by-remaining-time pops the victim's front (its slowest after sort_slowest_first); merged-table write keeps later --shard reads consistent; atomic tmp+rename cleans up on failure; workers skip Timings::load.

Extended reasoning...

Overview

Adds bun test --timings=<path> / --update-timings for duration-balanced --shard and --parallel distribution, a new ~225-line Timings.rs module, changes to the parallel coordinator's steal heuristic (by remaining time instead of range length when timings are present), a JSC thresholdForFTLOptimizeAfterWarmUp bump gated on --isolate, a new docs/test/parallel.mdx page, and a bench fixture generator with jest/vitest configs. 10 new tests in test-shard.test.ts.

Security risks

None identified. The only externally-read data is the user's own --timings JSON: parsed via bun_json::ParsedJson, non-finite/negative values are dropped, keys are used only as HashMap lookups (never as filesystem paths), and the file is written atomically to a same-directory tmp then renameat'd.

Level of scrutiny

Medium-high. This introduces new CLI flags and a versioned on-disk format (user-facing API per REVIEW.md's "API design" section), changes a JSC JIT-tiering threshold (perf claim backed by a profile in the description, but a knob a maintainer should ratify), and reworks the parallel work-stealing heuristic. None of it is on a hot path for users who don't opt in, and the --shard default path is unchanged, but the surface area and design choices warrant a human sign-off.

Other factors

All prior review threads (CodeRabbit + my earlier nit on the coordinator abort path) are resolved and reflected in the current diff. Test coverage is solid for the partition edge cases (giant file first/last, missing entries, shard-then-shard consistency, parallel dispatch order). The one wall-clock-sensitive test was moved to a serial describe block. The FTL threshold change is set before user env-var parsing in JSCInitialize, so BUN_JSC_thresholdForFTLOptimizeAfterWarmUp still overrides it.

[coderabbitai]

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/runtime/cli/test/parallel/Coordinator.rs (1)

551-558: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Do not record timing for a file whose worker crashed mid-execution.

reap_worker() calls self.record_timing(idx, w.dispatched_at) before self.account_crash(idx, status) when a worker dies mid-file. This records the elapsed time up to the crash as the file's duration, even though the file never finished running.

A worker crash can happen at any point in the file's execution, so the recorded duration is effectively arbitrary. With --update-timings, this value gets written back to the shared timings table and used for future --shard/--parallel balancing, corrupting the estimate for any test that occasionally crashes (native addon fault, segfault, OOM).

abort_all() in this same file already establishes the correct precedent: it does not call record_timing for interrupted in-flight files on SIGINT/SIGTERM. Apply the same rule here so a crash-truncated run does not pollute future duration estimates.

🐛 Proposed fix

             let panicked = is_panic_status(status);
             let was_bailed = self.bailed;
             if was_bailed && !panicked {
                 self.account_unfinished(idx, b"aborted: sibling worker panicked");
             } else {
-                self.record_timing(idx, w.dispatched_at);
                 self.account_crash(idx, status);
             }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/runtime/cli/test/parallel/Coordinator.rs` around lines 551 - 558, Update
the worker-reaping logic around is_panic_status, self.bailed, and account_crash
so crashed mid-execution workers never call record_timing; preserve timing
recording only for successfully completed workers, while continuing to account
for the crash and unfinished work as appropriate.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/test/parallel.mdx`:
- Line 42: Update the process-locality sentence in the parallel execution
documentation to qualify that contiguous chunks tend to keep files from the same
directory together in the initial assignment, rather than guaranteeing they run
in one process. Preserve the existing descriptions of chunk boundaries, worker
stealing, and timing-based balancing.
- Line 187: Update the timing-file write around the existing Bun.write call to
import rename from node:fs/promises, write JSON to a temporary file in the same
directory as basePath, then atomically rename that file to basePath. On any
failure, remove the temporary file before propagating the error, and do not use
Bun.rename.

---

Outside diff comments:
In `@src/runtime/cli/test/parallel/Coordinator.rs`:
- Around line 551-558: Update the worker-reaping logic around is_panic_status,
self.bailed, and account_crash so crashed mid-execution workers never call
record_timing; preserve timing recording only for successfully completed
workers, while continuing to account for the crash and unfinished work as
appropriate.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: cc6fbdcf-9f62-4286-ab37-cb4b8b80bfd4

📥 Commits

Reviewing files that changed from the base of the PR and between acf4225 and 47cc330.

📒 Files selected for processing (7)

• bench/test/app/setup.ts
• docs/test/parallel.mdx
• src/runtime/cli/test/Timings.rs
• src/runtime/cli/test/parallel/Coordinator.rs
• src/runtime/cli/test/parallel/runner.rs
• src/runtime/cli/test_command.rs
• test/cli/test/test-shard.test.ts

[claude]

Thanks for addressing the abort-path nit — write_timings_if_needed() is now on the coord.aborted exit too. I didn't find anything blocking, but this adds two user-facing flags with a versioned file format, changes the --parallel work-stealing heuristic, and overrides a JSC FTL threshold, so it's worth a human sign-off on the API shape and the 1e6 threshold choice.

What was reviewed:

• Timings::partition reserve-one-per-remaining-run guard and the big-file-first/last shard tests — every run gets ≥1 file.
• Merged-table write semantics under --shard (b7c1f6f) — later shards read the same cut; atomic tmp+rename with unlink-on-failure.
• short_lived_globals is only reachable via test_command.rs with ctx.test_options.isolate; every other jsc::initialize caller passes false, so the FTL override can't leak into bun run/bundler/repl.
• Timings write is now called on all serial/parallel exit paths (bail, coverage-threshold, coord.aborted, normal).

Extended reasoning...

Overview

Adds --timings=<path> / --update-timings to bun test: a per-file duration table ({version:1, files:{path:ms}}) that balances --shard by total time (contiguous path-sorted runs) and makes --parallel cut worker chunks by duration, dispatch each chunk slowest-first, and steal by remaining time. Also raises thresholdForFTLOptimizeAfterWarmUp to 1e6 under --isolate (via a new short_lived_globals param on JSCInitialize), adds a new docs/test/parallel.mdx page, and a bench/test/app fixture generator with jest/vitest configs. ~225-line new Timings.rs, small edits to Coordinator.rs/runner.rs/test_command.rs/Arguments.rs/context.rs, and 10 new tests in test-shard.test.ts.

Security risks

None identified. The only new file I/O is the timings JSON: read via the in-tree JSON parser (parse errors print diagnostics and exit 1), written atomically to <path>.<pid>.tmp then renameat in the same directory. Paths are user-supplied CLI args, not network/archive-derived. The FTL threshold change is a JSC Options:: numeric override applied before env-var processing, so BUN_JSC_thresholdForFTLOptimizeAfterWarmUp still wins.

Level of scrutiny

Medium-high. Nothing here is memory-unsafe or on a hot request path, but it's user-facing API surface (flag names, file format with a version field, merge semantics documented in a CI recipe) plus a JSC tuning knob whose value (1e6) was picked from one benchmark on one machine — a maintainer should confirm they're happy committing to those. The work-stealing change (find_steal_victim now sums costs[lo..hi] and the timings path steals via pop_front on the victim instead of steal_back_half) is straightforward and covered by the new deterministic-order test.

Other factors

All prior CodeRabbit findings and my earlier abort-path nit are addressed and resolved on the thread. The new tests exercise the partition edge cases (giant file first/last, files missing from the table, empty-shard write, malformed JSON, merge-under-shard) and the parallel dispatch order; the wall-clock-sensitive test was moved to a serial describe. jsc::initialize retains its old signature and delegates to initialize_with(_, false), so no other caller's behavior changes.

[robobun]

^{Updated 5:05 PM PT - Aug 3rd, 2026}

@Jarred-Sumner, your commit d313e37 is building: #88484

[coderabbitai]

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

docs/test/parallel.mdx (1)

163-185: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Add runs-on to the test job.

The test job declares strategy and steps but no runs-on. GitHub Actions requires runs-on on every job that is not a reusable-workflow call; without it, this workflow fails schema validation entirely. The sibling save-timings job added in this same block correctly includes runs-on: ubuntu-latest, so the omission on test stands out. Since this block is a copy-paste CI example that was substantially rewritten in this PR (cache restore/save, multi---timings invocation, artifact upload), a reader who copies it verbatim gets a broken workflow.

🐛 Proposed fix

 jobs:
   test:
+    runs-on: ubuntu-latest
     strategy:
       matrix:
         shard: [1, 2, 3, 4]
     steps:
       - uses: actions/checkout@v4

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/test/parallel.mdx` around lines 163 - 185, Add runs-on: ubuntu-latest to
the test job definition before its strategy block, matching the existing
save-timings job and making the GitHub Actions example schema-valid.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/test/parallel.mdx`:
- Line 158: Clarify the duplicate-entry rule in the --timings documentation by
replacing “the same test appears in several files” with terminology consistent
with the surrounding section, explicitly referring to the same test file or
timing entry appearing in multiple --timings input files. Preserve the existing
writer-measured-versus-carried-through precedence behavior.

In `@test/cli/test/test-shard.test.ts`:
- Around line 257-282: Update the two runShard calls in the merge-order test to
retain both subprocess results, then assert each result’s stderr/stdout health
before checking exitCode and ran. Follow the assertion pattern used by the
neighboring --update-timings test so failures expose subprocess diagnostics
before validating shard contents.

---

Outside diff comments:
In `@docs/test/parallel.mdx`:
- Around line 163-185: Add runs-on: ubuntu-latest to the test job definition
before its strategy block, matching the existing save-timings job and making the
GitHub Actions example schema-valid.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: edbfb226-517b-46c9-aab5-270b25a3c709

📥 Commits

Reviewing files that changed from the base of the PR and between 47cc330 and e0cd81a.

📒 Files selected for processing (6)

• docs/test/parallel.mdx
• src/options_types/context.rs
• src/runtime/cli/Arguments.rs
• src/runtime/cli/test/Timings.rs
• src/runtime/cli/test_command.rs
• test/cli/test/test-shard.test.ts

[claude]

No new issues found on this pass; all five earlier findings (partition trailing-empty-shard, empty-shard write skip, parallel-abort write, updated: [] empty-array semantics, parent-dir creation) are addressed with tests. Deferring to a human for the API-surface and JSC-tuning decisions — new --timings/--update-timings flags with a versioned on-disk format, the thresholdForFTLOptimizeAfterWarmUp bump under --isolate, and the published benchmark table.

What was reviewed:

• Timings::partition greedy cut — traced the reserve-one-per-remaining-bin guard and both giant-file test cases.
• Multi-file merge: as_property-based updated detection so [] ≠ absent; order-independence covered by two tests.
• write_timings_if_needed reached on serial/parallel bail, coverage-fail, and coord.aborted exits; atomic tmp+rename via make_open.
• Coordinator steal-victim change (u32→u64 remaining-time metric) still never aliases the caller's &mut Worker.

Extended reasoning...

Overview

Adds bun test --timings=<path>... (repeatable) and --update-timings: a per-file duration table ({version:1, files:{path:ms}, updated:[...]}) that --shard uses to cut path-sorted files into contiguous runs of ~equal total time and --parallel uses to cut worker chunks and dispatch slowest-first. New 269-line Timings.rs (load/merge/partition/write), wiring in test_command.rs/Coordinator.rs/runner.rs/Arguments.rs, a new short_lived_globals flag on JSCInitialize that raises thresholdForFTLOptimizeAfterWarmUp to 1e6 under --isolate, a 219-line docs/test/parallel.mdx with a GitHub Actions recipe and a bun/jest/vitest comparison table, a bench/test/app suite generator with jest/vitest configs, and 12 new tests in test-shard.test.ts.

Security risks

None identified. The timings file is a local JSON artifact the user opts into; parsing goes through the in-tree JSON parser with explicit shape validation (version==1, finite non-negative numbers clamped to u32). No network, no path traversal (paths are only used as map keys, never opened), and the write is tmp+rename in the same directory.

Level of scrutiny

High — this is new user-facing API surface. The --timings file format has a version field, so it's a compatibility contract; the flag names, help text, and docs page are all public. The FTL-threshold change is a performance-tuning decision backed by a profile in the PR description but affects every bun test --isolate run. The published benchmark table comparing against jest/vitest is marketing-adjacent. These are the kinds of decisions a maintainer should sign off on rather than land bot-approved.

Other factors

The PR has been through three prior bug-hunt rounds; every issue raised (by me and by CodeRabbit) is resolved with a corresponding test and the author's replies confirm each fix. Test coverage is thorough: file-format shape/order, multi-file merge order-independence including the updated: [] edge case, missing/malformed inputs, both giant-file partition edge cases, parent-dir creation, first-path write semantics, and --parallel dispatch/steal order pinned via --parallel-delay. rust:check-all is reported clean. The change is well-contained (the Timings struct is Option-gated everywhere and only touches paths already owned by the test command) and the code quality is high, but the scope — ~1000 net lines across 21 files, new CLI flags, a JSC option change, and a docs page with competitive benchmarks — is beyond what should merge without a human look.

[claude]

Both earlier findings are resolved — d345938 dropped the updated list entirely in favour of disjoint per-shard writes (simpler than the cb652c0 fix), and Timings::write now uses File::make_open with a covering test. No new issues this pass, but this adds user-facing API (--timings/--update-timings + file format), a hardcoded JSC FTL threshold under --isolate, and reworks parallel work-stealing, so it's worth a human sign-off.

Checked: the new load_one union is order-insensitive given per-shard outputs are disjoint; partition() guarantees ≥1 file per bin and the last bin drains the remainder; costs is recomputed after sort_slowest_first so indices line up with the reordered sorted; make_open + renameat keeps the write atomic and creates the parent dir.

Extended reasoning...

Overview

Adds bun test --timings=<path> (repeatable) and --update-timings for duration-balanced --shard/--parallel, a new Timings module (~240 lines Rust), a 217-line docs page, work-stealing changes in Coordinator.rs (steal-by-remaining-time when costs are known), per-file timing hooks in test_command.rs, a JSC thresholdForFTLOptimizeAfterWarmUp bump to 1,000,000 gated on --isolate, and a jest/vitest benchmark generator under bench/test/app.

Changes since my last review

My two prior inline findings (empty updated: [] treated as absent; CI recipe never bootstraps because File::create doesn't mkdir) were first patched in cb652c0, then d345938 replaced the updated-list merge with a simpler design: under --shard each run writes only the files it measured, so per-shard outputs are disjoint and reading them together is a plain union — no freshness tracking, no order dependence. load_one now just map.puts each entry; write(only_measured) picks self.measured vs self.map. This is cleaner and the tests cover empty-shard writes, multi-file union in both orders, and parent-dir creation.

Security risks

None identified. The timings file is user-supplied JSON parsed via the in-tree JSON parser; values are validated (finite, non-negative, clamped to u32) before use. Paths are only used for hashmap keys and for writing to the user-specified location via bun_sys. No network, no privilege changes.

Level of scrutiny

Medium-high. This is new user-facing CLI surface with a persisted file format (version: 1), a published CI recipe users will copy, a global JSC option override whose value (1e6) was picked from one benchmark, and a change to the parallel coordinator's steal heuristic. None of it is on a hot path for correctness of test results, but the API shape and the FTL policy are decisions a maintainer should ratify.

Other factors

12 new tests in test-shard.test.ts covering the partition edge cases (giant file first/last, missing entries, empty shard, mkdir, multi-file union order, parallel dispatch order). All prior review threads are resolved. The PR description is slightly stale (still describes the updated list) but that's not code.

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@bench/test/bunfig.toml`:
- Around line 1-2: Remove the app/preload.ts preload from the shared benchmark
configuration in bench/test/bunfig.toml and bench/test/vitest.config.ts; move it
to app-scoped configuration or restrict it exclusively to app/tests so
parallel/suite and isolate/suite run without app setup or unrelated hooks.

In `@test/cli/test/parallel.test.ts`:
- Around line 1054-1056: Update the parallel test helper’s run function to
continue returning stderr, then add expect(stderr).toBe("") assertions for both
worker runs. Keep the existing stdout, exit-code, count, and global-state
assertions unchanged while ensuring bunEnv-based executions fail on unexpected
worker diagnostics.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 83cb8fa3-76ac-4305-94b2-276f3ffcc870

📥 Commits

Reviewing files that changed from the base of the PR and between d345938 and ca704be.

📒 Files selected for processing (10)

• bench/test/.gitignore
• bench/test/README.md
• bench/test/app/setup.ts
• bench/test/bunfig.toml
• bench/test/jest.config.cjs
• bench/test/vitest.config.ts
• docs/test/parallel.mdx
• src/runtime/cli/Arguments.rs
• src/runtime/cli/test/parallel/runner.rs
• test/cli/test/parallel.test.ts

[claude]

🟡 The test: job in this GitHub Actions recipe is missing runs-on: (the sibling save-timings: job has runs-on: ubuntu-latest). GitHub Actions requires runs-on on every job with steps, so a user copy-pasting this verbatim gets an immediate "Required property is missing: runs-on" workflow validation error — add runs-on: ubuntu-latest under test:.

Extended reasoning...

What the bug is

The .github/workflows/test.yml recipe in the "Large codebases: putting it together" section defines two jobs. The save-timings: job correctly declares runs-on: ubuntu-latest, but the test: job goes straight from test: to strategy: and steps: with no runs-on: key:

jobs:
  test:
    strategy:
      matrix:
        shard: [1, 2, 3, 4]
    steps:
      - uses: actions/checkout@v4
      ...
  save-timings:
    needs: test
    runs-on: ubuntu-latest
    ...

Why it fails

GitHub Actions' workflow schema requires runs-on on every job that defines steps. The only exemption is a reusable-workflow-call job (one with uses: at the job level instead of steps:), which this is not. A matrix strategy does not supply a runner — each matrix expansion still needs to know which runner image to use.

Step-by-step proof

1. A user reads the "Keep the timings fresh automatically" section and copies the fenced YAML block into .github/workflows/test.yml in their repo.
2. They push. GitHub's workflow parser validates the file before any job is queued.
3. Validation fails on the test job with: "Invalid workflow file: .github/workflows/test.yml#L3 — Required property is missing: runs-on".
4. The workflow does not run at all; no shards execute, no timings are collected.
5. The user has to diagnose and add runs-on: ubuntu-latest themselves before the recipe does anything.

Nothing in the surrounding prose mentions that the snippet is abridged or that runs-on was elided for brevity, and the sibling save-timings: job does include it — which makes the omission look accidental rather than intentional shorthand.

Impact

The whole point of this fenced block (per the PR description: "docs have the Actions workflow") is to be a copy-paste-and-go CI recipe demonstrating the per-shard --timings/--update-timings flow. A recipe that fails validation on paste undermines that. The failure is loud, immediate, and self-explanatory — GitHub points at the exact line and names the missing key — so no one will be stuck for long, but it's a paper cut on a page that's specifically selling "here's the exact YAML".

Fix

One line — add runs-on: ubuntu-latest between test: and strategy::

jobs:
  test:
    runs-on: ubuntu-latest
    strategy:
      matrix:
        shard: [1, 2, 3, 4]
    steps:
      ...

This matches the save-timings: job and the runs-on: ubuntu-latest used in the existing GitHub Actions example at docs/test/index.mdx.

[claude]

🟡 The actions/upload-artifact@v4 step and the save-timings: job have no if:, so they default to if: success() — a shard with even one failing test skips its upload, and save-timings (needs: test) is skipped unless every matrix shard passed. --update-timings deliberately writes on failure/bail so partial runs still contribute, but the workflow discards that file; a suite with a single flaky test never populates the cache and --shard stays on file-count round-robin forever. Add if: ${{ !cancelled() }} to both (and consider fail-fast: false on the matrix).

Extended reasoning...

What's wrong

The "Keep the timings fresh automatically" recipe is meant to accumulate per-shard durations across CI runs so --shard and --parallel balance by time. But as written it only caches timings after a 100%-green run across all shards, which for its stated audience ("a suite with thousands of test files") is uncommon:

      - uses: actions/upload-artifact@v4        # no if: → implicit if: success()
        with:
          name: timings-${{ matrix.shard }}
          path: .bun-test-timings/next/${{ matrix.shard }}.json
  save-timings:
    needs: test                                 # no if: → skipped unless ALL matrix jobs succeeded
    runs-on: ubuntu-latest

GitHub Actions semantics

1. A step with no explicit if: runs under an implicit if: success(). When bun test exits non-zero (which it does on any test failure — see line 64 of the same page), every subsequent step in that job is skipped, including upload-artifact.
2. A job with needs: test and no explicit if: runs only when every matrix expansion of test has result == 'success'. One red shard skips save-timings entirely.
3. Matrix jobs default to fail-fast: true, so the first failing shard also cancels its still-running siblings — compounding the effect.

Why this matters here specifically

--update-timings is deliberately wired to write on every exit path, not just success: write_timings_if_needed() is called on the normal completion path (test_command.rs:3037), the --bail path (test_command.rs:3329), and the --parallel abort path (runner.rs:237). The runtime goes out of its way to preserve measured durations even when tests fail — but this workflow throws them away before they reach the cache.

Step-by-step

1. Suite has 2 000 test files across 4 shards; one test in shard 3 is flaky.
2. Run N: shards 1, 2, 4 pass; shard 3 fails. bun test in shard 3 exits 1 after writing .bun-test-timings/next/3.json (its ~500 files' durations).
3. Shard 3's upload-artifact step is skipped (if: success()). With fail-fast: true (the default), any shards still running are cancelled.
4. save-timings sees needs.test.result != 'success' → skipped. Nothing is written to the cache.
5. Run N+1: actions/cache/restore misses (or hits an old/empty entry). Every shard falls back to file-count round-robin.
6. Repeat. The mechanism the recipe exists to demonstrate stays inert, silently — the workflow "works" and there's no diagnostic.

The "last successful run's per-shard timings" comment on the restore step shows the author knows only green runs cache, but for the recipe's target audience that can mean never, which quietly defeats the whole --timings section above it.

Fix

Standard GitHub Actions idiom for sharded-test artifacts (matches how Jest/Vitest sharding recipes handle it):

  test:
    strategy:
      fail-fast: false
      matrix:
        shard: [1, 2, 3, 4]
    steps:
      ...
      - uses: actions/upload-artifact@v4
        if: ${{ !cancelled() }}
        with: ...
  save-timings:
    if: ${{ !cancelled() }}
    needs: test
    ...

This is a docs-recipe robustness gap rather than shipping code — it works as written on green runs and someone fluent in Actions would spot it — hence nit. Worth fixing alongside the already-noted missing runs-on: on the test: job, since the block is being edited anyway.