test: strip seconds-formatted summary timings, run diffexample fixture from its own dir

[robobun]

Problem

bun bd test test/js/bun/test/printing/diffexample.test.ts fails the "no color" test on a debug (ASAN) build at current main, with no source changes. The only difference in the ~700 line inline snapshot is the summary line:

- Ran 19 tests across 1 file.
+ Ran 19 tests across 1 file. [2.41s]

Cause

Two things combine here.

Output::print_elapsed (src/bun_core/output.rs) prints the total as [N.NNms] up to 1.5s and as [N.NNs] above that; the timer starts at process start. The test's cleanOutput only stripped / \[[0-9\.]+ms\]/, so the snapshot and the later colorStderr/noColorStderr equality only hold while the child finishes in under 1.5s. Per-test timings are unaffected (ElapsedFormatter always prints ms); only the summary line switches units.

The reason this particular child takes ~2.5s on a debug build is mostly not the diffs: the test spawns bun test without a cwd, so the child inherits the repo root and loads the repo bunfig.toml, whose [test] preload = "./test/preload.ts" imports harness.ts into the child. On the debug build that import is ~1.8s of the ~2.5s (the same fixture run from its own directory prints [~720ms]; the 19 diffs themselves are under 0.5s). It also means the two sequential children in "no color" sit right at the test's 5s default timeout on a debug build (runs here landed between 4.9s and 5.0s), so widening the regex alone would only have traded the snapshot mismatch for an intermittent timeout in the same file.

The same ms-only strip is used against the same summary lines (from bun test, and from bun install / bun link, which share the formatter) in a few other files. Those files already spawn from a bunfig-free cwd, so they only cross 1.5s on a slow machine, but the normalizer has the same hole and is updated the same way:

• test/regression/issue/19850/19850.test.ts (the second test also counted [..ms] occurrences and expected the total to be one of them; it now accepts either unit and converts to ms)
• test/js/bun/test/test-failing.test.ts
• test/js/bun/test/test-test.test.ts
• test/cli/test/bun-test.test.ts
• test/cli/install/bun-link.test.ts (line 463 in the same file already accepted both units)

Fix

• Accept both units in the normalizers (m?s), which is what normalizeBunSnapshot in harness.ts and the other install tests already do. The seconds format is intended output, so this is test-only.
• In diffexample.test.ts, run the fixture with cwd: import.meta.dir, as the other touched tests already do for their fixtures, and start both children before waiting on either. The snapshot header becomes the bare diffexample.fixture.ts:, which also makes the Windows backslash rewrite in cleanOutput unnecessary. No other snapshot content changes.

Verification

• bun bd test test/js/bun/test/printing/diffexample.test.ts: failed before with the diff above; after, "no color" passes in 3 of 3 runs at 0.8s to 1.1s (it was 4.9s to 5.0s with only the regex change).
• USE_SYSTEM_BUN=1 bun test on the six files: all pass.
• bun bd test on the other five files: all touched tests pass. bun-link.test.ts "should link dependency without crashing" times out on the debug build with or without this change; that is a separate, pre-existing debug-only issue (the install failure path dumps a stack trace in debug builds) and is tracked separately.

---

no test proof · iteration 1 · Platform-specific test-only change; deferring to CI.

[coderabbitai]

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 42 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: edeadb33-80cd-48ff-a4c8-4430da55da2d

📥 Commits

Reviewing files that changed from the base of the PR and between c63dc64 and 06b6d55.

📒 Files selected for processing (6)

• test/cli/install/bun-link.test.ts
• test/cli/test/bun-test.test.ts
• test/js/bun/test/printing/diffexample.test.ts
• test/js/bun/test/test-failing.test.ts
• test/js/bun/test/test-test.test.ts
• test/regression/issue/19850/19850.test.ts

---

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

[claude]

LGTM — mechanical test-normalizer widening from ms to m?s.

What was reviewed:

• Confirmed print_elapsed in src/bun_core/output.rs:1200 switches to seconds above 1500ms while ElapsedFormatter (per-test timings) always prints ms, so the 19850 test's 2-match expectation and unit conversion are correct.
• Checked that harness.ts (normalizeBunSnapshot, runBunInstall) already uses the same m?s pattern this PR adopts.
• The two \\[.*\\ms\\] → \\[[\\d.]+m?s\\] rewrites in bun-test.test.ts and test-test.test.ts also tighten the regex (drop greedy .* and the no-op \\m), which is fine — the snapshots are unchanged.

Extended reasoning...

Overview

Test-only change touching six test files. Each edit widens a timing-strip regex from matching [N.NNms] to also match [N.NNs], because Output::print_elapsed switches units above 1.5s and debug/loaded-CI runs cross that threshold. One site (19850.test.ts second test) additionally captures the unit and multiplies by 1000 when it's s so the numeric bounds check stays in milliseconds. No production code, no snapshot content, and no assertion semantics change.

Security risks

None. Regex edits inside test normalizers only; no user-facing surface.

Level of scrutiny

Low. This is a de-flake following the exact pattern already established in test/harness.ts (lines 1540, 2138, 2148-2149) and elsewhere in the same file (bun-link.test.ts:463). I verified against src/bun_core/output.rs that the summary formatter (print_elapsed, line 1198-1203) does switch to s above 1500ms while the per-test ElapsedFormatter (line 1145-1183) always emits ms — so the 19850 "times reported are reasonable" test still finds exactly 2 matches (one per-test + one summary) under either unit, and the unit === "s" ? *1000 : ... conversion is correct.

Other factors

The two sites that previously used /\\[.*\\ms\\]/ (bun-test.test.ts:1304, test-test.test.ts:303) were rewritten to /\\[[\\d.]+m?s\\]/ rather than just adding ? — this drops a greedy .* and a redundant \\m escape, making them consistent with the neighboring patterns. The inline snapshots they normalize are unchanged, so this is a strict improvement. The PR description documents verification on both the ms branch (USE_SYSTEM_BUN=1) and the seconds branch (debug build), and notes the one pre-existing unrelated debug-only timeout in bun-link.test.ts. No outstanding reviewer comments.

[claude]

LGTM — test-only flake fix widening timing normalizers to accept both ms and s.

What was reviewed:

• Confirmed print_elapsed in src/bun_core/output.rs switches units at 1500ms and that normalizeBunSnapshot in harness.ts already uses the m?s pattern this PR adopts.
• Checked the 19850 unit-conversion logic (s → ×1000) keeps the same > 45 / < 2*elapsed bounds and match count.
• Traced the diffexample refactor: cwd: import.meta.dir makes the printed header path just diffexample.fixture.ts:, so the removed Windows path normalizer and one-line snapshot edit are consistent; concurrent spawn + await using preserves the prior await-then-read order.

Extended reasoning...

Overview

Test-only PR touching six test files. The core change replaces \[[0-9.]+ms\] timing-strip regexes with \[[0-9.]+m?s\] so that summary lines printed as [N.NNs] (which Output::print_elapsed emits above 1.5s) are normalized the same way as [N.NNms]. A secondary commit refactors diffexample.test.ts to pin the fixture's cwd to its own directory, extract a runFixture() helper, spawn both fixtures concurrently under await using, and drop a now-unneeded Windows path normalizer plus a one-line snapshot update.

Security risks

None. No runtime code is touched; all changes are to test normalizers, one snapshot line, and spawn setup for a test fixture.

Level of scrutiny

Low. This is a mechanical de-flake that aligns ad-hoc timing regexes with the pattern normalizeBunSnapshot in test/harness.ts already uses (\[[\d.]+\s?m?s\]). No assertions are weakened — the stripped timing was never part of the asserted content, and the 19850 second test still asserts the same numeric bounds after converting seconds to ms. The two regexes that changed shape beyond m?s (/\[.*\ms\]/ and /\[(.*)\ms\]/g) were tightened from .* to [\d.]+, which is strictly safer against over-matching.

Other factors

The diffexample refactor is the only non-trivial hunk. Pinning cwd makes the printed file header diffexample.fixture.ts: regardless of where the outer test is invoked from and regardless of platform path separators, which is why the Windows replaceAll and the full-path snapshot line both go away together. The concurrent spawn keeps the same await exited → read pattern as before, so pipe-buffer behavior is unchanged, and await using adds cleanup on failure. The PR description documents verification on both debug (bun bd test, exercises the s branch) and release (USE_SYSTEM_BUN=1, exercises the ms branch), which covers both sides of the widened regex.

[claude]

LGTM — test-only de-flaking that widens timing-strip regexes to accept both [N.NNms] and [N.NNs].

What was reviewed:

• The ms → m?s regex widening across all six files matches what normalizeBunSnapshot already does; no assertions weakened.
• diffexample.test.ts: pinning cwd: import.meta.dir (no bunfig.toml there) and the resulting snapshot header change to bare diffexample.fixture.ts:, which correctly makes the Windows-backslash rewrite dead.
• 19850.test.ts: the unit-capturing match and seconds→ms conversion preserve the original bounds check.

Extended reasoning...

Overview

Test-only PR touching six files under test/. It (1) widens elapsed-time normalizer regexes from \[[0-9.]+ms\] to \[[0-9.]+m?s\] so snapshot/normalizer assertions survive when Output::print_elapsed switches to seconds above 1.5s, and (2) refactors diffexample.test.ts to spawn its fixture with cwd: import.meta.dir (avoiding the repo bunfig.toml preload of harness.ts), spawn both children concurrently under await using, and update the one snapshot line whose path prefix depends on cwd. The Windows path rewrite in cleanOutput is dropped since the header is now separator-free.

Security risks

None. No production code touched; changes are regex tweaks in test normalizers and a spawn cwd argument.

Level of scrutiny

Low. This is a mechanical de-flaking change to test normalizers, mirroring the pattern already used by normalizeBunSnapshot in harness.ts and by line 463 of the same bun-link.test.ts. The one non-trivial edit — the 19850.test.ts second test — now captures the unit and multiplies by 1000 for s, preserving the exact same > 45 / < 2*elapsed bounds. No existing assertions are loosened beyond accepting the alternate unit spelling, which is intended runtime output.

Other factors

Verified the printing/ directory has no bunfig.toml, so the cwd pin achieves its stated goal without side effects. The pre-existing await proc.exited before reading pipes in diffexample.test.ts is unchanged by this PR (fixture output is well under the 64KB pipe buffer). The PR description documents debug-build verification and the USE_SYSTEM_BUN=1 check across all six files.