[robobun]

What does this PR do?

spawn-stress.test.ts was taking ~9s on debian 13 x64-asan (build #84540) and ~14.7s locally under debug+ASAN. The test body is 100 sequential spawns of bunExe() --version; debug+ASAN child startup (~140ms each) dominates wall time, not Bun.spawn itself.

Changes:

• Spawn sh -c "echo ok" (cmd.exe /c echo ok on Windows) instead of bunExe() --version. The child binary is incidental here (the test was originally written with clang --version; bunExe() was swapped in for CI availability). Same portable-child pattern as spawn-many-teardown.test.ts. This keeps 100 iterations on every lane instead of scaling down for debug/ASAN.
• Run spawns in batches of 8 (same pattern as spawn-streaming-stdout.test.ts / spawn-streaming-stdin.test.ts).
• Drop the per-iteration await Bun.sleep(1) and console.log spam (200 lines per run).
• Replace new Response(stream).text() with stream.text() and use await using for process cleanup.
• Drop the 99999999 per-test timeout.

Assertion strengthening:

• Check stderr === "" (previously ignored).
• Check stdout.trim() === "ok" (previously only != "").
• Check exitCode === 0 (previously unchecked).

How did you verify your code works?

bun bd test test/js/bun/spawn/spawn-stress.test.ts locally under debug+ASAN:

	wall	test body	expect() calls	iterations
before	14.71s	12.37s	100	100
after	2.80s	0.72s	300	100

Release (USE_SYSTEM_BUN=1): ~150ms test body. Windows x64 release: ~142ms test body. Ran repeatedly under all three with no variance.

PR #33704 also touches this file (iteration count only) as part of a 53-file omnibus; this change supersedes that hunk.

---

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

[coderabbitai]

Walkthrough

Changes

Spawn stress validation

Layer / File(s)	Summary
Concurrent process execution and assertions test/js/bun/spawn/spawn-stress.test.ts	The test runs batched portable echo ok processes with managed resources, concurrently reads stdout and stderr, waits for completion, and verifies output, empty stderr, and exit code with iteration-specific failure diagnostics.

🚥 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 the main change: batching spawn-stress iterations and strengthening assertions.
Description check	✅ Passed	The description matches the template and includes both the change summary and verification details.

---

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

[robobun]

Status: diff is green; CI red is unrelated infra/flake.

spawn-stress.test.ts passed on every lane that ran across builds #85082, #85134, and #85195. Remaining red on #85195 is the darwin-14 x64/aarch64 test lanes expiring before an agent picked them up (same expiry hit #85134); every other failure is marked flaky and passed on retry/isolation. None touch spawn or this file.

Verified locally with bun bd test test/js/bun/spawn/spawn-stress.test.ts: 14.71s → 2.80s under debug+ASAN, 100 iterations, 300 expect() calls. Also verified on windows-x64 release (~142ms). Ready for a maintainer.

[claude]

LGTM — test-only speedup that follows the established batched-spawn pattern from sibling tests.

What was reviewed:

• Batching loop bounds handle the non-multiple case (100 % 8 = 4) correctly via the i + j < iterations guard.
• --version prints Global::package_json_version (src/runtime/cli/mod.rs:2286) which is the same constant Bun.version exposes (src/bun_core/Global.rs:429), so stdout.trim() === Bun.version holds across debug/release.
• getMaxFD() baseline is captured after the first batch's await using scopes have exited — same fd-leak-guard shape as spawn-streaming-stdout.test.ts:42-47.
• Pipes drained concurrently via Promise.all, bunEnv passed so debug stderr stays empty.

Extended reasoning...

Overview

This PR rewrites test/js/bun/spawn/spawn-stress.test.ts — a single test file, no production code. The old test ran 100 sequential bunExe() --version spawns with a 1ms sleep between each, per-iteration console.log, a 99999999 timeout, and only asserted stdout != "". The new version batches 8 concurrent spawns per iteration (40 total on debug/ASAN, 100 on release), drops the sleep/logging/timeout, and asserts exact stdout/stderr/exitCode plus an fd-leak check.

Security risks

None. Test-only change; spawns the test's own bunExe() with --version, no network, no filesystem writes, no untrusted input.

Level of scrutiny

Low. This is a test-infrastructure speedup with no runtime code touched. The batched-concurrency + getMaxFD() shape is copied nearly verbatim from spawn-streaming-stdout.test.ts and spawn-streaming-stdin.test.ts, so the pattern is already proven in CI across platforms. The main things worth checking were (a) whether the tightened assertions could introduce flakiness and (b) whether the loop bounds are correct — both check out.

Other factors

• Verified --version and Bun.version share the same source constant (package_json_version), including the -debug suffix on debug builds, so the exact-match assertion won't diverge.
• bunEnv is now passed, which sets BUN_DEBUG_QUIET_LOGS=1 — necessary for the new stderr === "" assertion to hold on debug builds.
• await using proc + Promise.all([stdout.text(), stderr.text(), proc.exited]) matches the harness conventions in test/CLAUDE.md and REVIEW.md (drain pipes concurrently, RAII cleanup).
• The fd baseline is captured after the first batch fully resolves (all await using disposals have run), so the final expect(getMaxFD()).toBe(maxFD) compares steady-state to steady-state — identical to the sibling test.
• Author reports 14.71s → 3.15s under debug+ASAN with 5 consecutive clean runs; the change removes a sleep rather than adding one, which is the right direction for flakiness.

[claude]

LGTM — test-only speedup that follows the sibling spawn-streaming test pattern.

What was reviewed:

• Batching/iteration-scaling matches spawn-streaming-stdout.test.ts; bunEnv/await using/concurrent pipe drain follow harness conventions.
• Assertions tightened (exact Bun.version, stderr === "", exitCode === 0) replace the vacuous != "" check; bunEnv silences debug logs so the stderr assertion holds.
• Follow-up commit b2d8507 drops the getMaxFD guard after an alpine-aarch64 flake — rationale is sound (pipe fds close a tick late; fd-leak coverage lives in the streaming siblings). The PR description still mentions it, but the net diff is correct.
• Dropped 99999999 timeout and Bun.sleep(1) are both explicit anti-patterns per test guidelines.

Extended reasoning...

Overview

Single-file test refactor of test/js/bun/spawn/spawn-stress.test.ts. The PR contains two commits: 2e5b5ef rewrites the test to batch spawns in groups of 8 (scaled to 40 iterations under debug/ASAN, 100 on release), passes bunEnv, drains stdout/stderr concurrently via Promise.all, uses await using for cleanup, drops the 99999999 timeout and per-iteration Bun.sleep(1)/console spam, and strengthens assertions from out != "" to exact matches on stderr === "", stdout.trim() === Bun.version, and exitCode === 0. b2d8507 then removes a getMaxFD() fd-leak guard that flaked on alpine 3.23 aarch64 (pipe-backing fds close on the tick after child exit), deferring that coverage to spawn-streaming-stdout/stdin which already have the correct GC-loop wait machinery.

Security risks

None. Test-only change; no production code, no new inputs, no network, no filesystem writes beyond what Bun.spawn already does.

Level of scrutiny

Low. This is a mechanical test-speedup that copies an established pattern verbatim from spawn-streaming-stdout.test.ts (concurrency-batched loop, same structure). Every change maps directly onto an explicit repo convention: bunEnv when spawning, concurrent pipe draining to avoid the 64KB deadlock, await using for cleanup, no test timeouts, no sleep, isASAN/isDebug for workload scaling, exact toBe over weak not.toBe("").

Other factors

• Verified the batching pattern against spawn-streaming-stdout.test.ts:8-47 — same shape (outer loop stepping by concurrency, inner loop pushing async IIFEs, Promise.all per batch).
• The Bun.version comparison is safe: --version and Bun.version both surface Global::package_json_version, and bunExe() guarantees the child is the same binary as the parent.
• The stderr === "" assertion is safe because bunEnv sets BUN_DEBUG_QUIET_LOGS=1, so debug builds won't emit noise.
• Reducing debug/ASAN iterations 100→40 is a deliberate workload shrink (per REVIEW.md: shrink workload rather than raise timeout); the failure mode being guarded — spawn path crashing or output going missing over many iterations — is still exercised at 40 iterations × 8-way concurrency, and release CI keeps 100.
• The dropped getMaxFD check in the second commit has a clear root-cause explanation and names where the coverage remains, satisfying the "never silently weaken" rule.
• Author reports 14.71s → 3.15s under debug+ASAN with 5 consecutive clean runs; net expect() count went up (100 → 121).

[claude]

LGTM — test-only speedup that strengthens assertions rather than weakening them.

What was reviewed:

• Confirmed the process.env.comspec || "cmd.exe" / /bin/sh child pattern matches spawn-many-teardown.test.ts:17-18 (already green in CI on all platforms).
• Confirmed the batch-of-N loop matches spawn-streaming-stdout.test.ts (concurrency 7 there, 8 here).
• Checked that the original invariant (output not lost / no crash across 100 spawns) is still asserted, now more strictly: exact stdout, empty stderr, exit 0 — previously only stdout != "".
• Verified .trim() handles both \n and \r\n so the Windows cmd.exe /c echo ok output passes the same assertion.

Extended reasoning...

Overview

Single-file test refactor of test/js/bun/spawn/spawn-stress.test.ts. Replaces 100 sequential bunExe() --version spawns (each a full debug+ASAN Bun startup) with 100 batched sh -c "echo ok" / cmd.exe /c echo ok spawns, drops the per-iteration sleep(1) and 200 lines of console spam, switches to stream.text() + await using + concurrent pipe drain, and removes the 99999999 timeout. No runtime code is touched.

Security risks

None. Test-only; spawns a fixed shell command with no user input.

Level of scrutiny

Low. This is a test-speed PR that follows the REVIEW.md guidance directly: it removes a sleep, drains pipes with Promise.all, uses await using, passes bunEnv, and — critically — does not weaken the property being tested. The new assertions (stderr === "", stdout.trim() === "ok", exitCode === 0) are strictly stronger than the old out != "". The iteration count stays at 100 on every lane instead of being scaled down.

Other factors

• The cheap-child and batching patterns are copied verbatim from sibling tests (spawn-many-teardown.test.ts, spawn-streaming-stdout.test.ts) that already run on all CI lanes including Windows, so the process.env.comspec / cmd.exe path is proven.
• The PR description notes the test originally spawned clang --version; the child binary was always incidental, so swapping it for sh/cmd.exe doesn't change what's under test (Bun's spawn path, not the child).
• The one semantic shift — 8-way concurrent batches vs strictly sequential — matches how the other spawn stress tests in this directory are structured, and if anything stresses the spawn path harder.
• Verified locally per PR description on debug+ASAN and Windows release; waiting on CI, but nothing here is lane-specific beyond patterns already in use.

[robobun]

^{Updated 10:05 AM PT - Jul 29th, 2026}

❌ @robobun, your commit 65b7f63 has some failures in Build #85195 (All Failures)

---

🧪   To try this PR locally:

bunx bun-pr 36345

That installs a local version of the PR into your bun-36345 executable, so you can run:

bun-36345 --bun

[coderabbitai]

Actionable comments posted: 3

🤖 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 `@test/js/bun/spawn/spawn-stress.test.ts`:
- Line 32: Strengthen the stdout assertion in the spawn stress test by removing
only the optional platform newline before comparing with "ok"; do not use
trim(), so unexpected leading or trailing whitespace causes the assertion to
fail.
- Around line 5-10: Update the comment above the stress test to remove the
historical reference to the previous child command and the incidental binary.
Keep only the durable rationale: repeatedly drain both pipes and overlap spawns
in small, bounded batches.
- Line 13: Update the Windows command array in the spawn stress test to include
the /d switch with cmd.exe before /c, while preserving the existing non-Windows
command and echo behavior.

🪄 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: 850a968e-1fcd-420a-bd34-6a0dc1f8461a

📥 Commits

Reviewing files that changed from the base of the PR and between b2d8507 and 65b7f63.

📒 Files selected for processing (1)

• test/js/bun/spawn/spawn-stress.test.ts

[claude]

LGTM — test-only speedup that follows existing spawn-test patterns and strictly tightens assertions.

What was reviewed:

• Batching (concurrency=8) and await using/concurrent pipe-drain match spawn-streaming-stdout.test.ts; the sh/cmd.exe child matches spawn-many-teardown.test.ts (including the process.env.comspec fallback).
• Confirmed the dropped getMaxFD fd-leak check (commit b2d8507) is redundant with the GC-looped fd-leak coverage in spawn-streaming-stdout/stdin.
• Assertions are strictly stronger than before (exact stdout, empty stderr, exit 0 — vs. only stdout != ""); .trim() handles Windows CRLF.

Extended reasoning...

Overview

Single-file, test-only change to test/js/bun/spawn/spawn-stress.test.ts. The test previously spawned bunExe() --version 100× sequentially with a 1ms sleep and 200 lines of console noise per run, taking ~14.7s under debug+ASAN. It now spawns sh -c "echo ok" (or cmd.exe /c echo ok on Windows) in batches of 8, drains both pipes concurrently via Promise.all, uses await using for cleanup, and asserts exact stdout/stderr/exitCode. The huge 99999999 timeout override is removed. An intermediate commit added and then removed a getMaxFD fd-leak check after it flaked on alpine aarch64 — the commit message correctly notes fd-leak coverage already exists in spawn-streaming-stdout/stdin with proper GC-wait machinery.

Security risks

None. Test-only; spawns /bin/sh / cmd.exe with a fixed literal argument list, no user-controlled input, no network.

Level of scrutiny

Low. This is a CI-speed refactor of an existing stress test with no runtime code changes. The two questions that matter are (1) does it still exercise the original failure mode and (2) does it follow harness conventions. Both check out: the comment documents the original failure mode (output going missing / spawn-path crash after many iterations), which is preserved — 100 iterations still run, both pipes are still drained, and concurrent batching is at least as stressful as sequential. The child binary was always incidental (originally clang --version). Harness conventions are correctly followed: bunEnv, await using, concurrent Promise.all([stdout, stderr, exited]) per REVIEW.md's pipe-drain rule, no timeout override, no sleep.

Other factors

• Every claimed pattern reference is real: batching mirrors spawn-streaming-stdout.test.ts:8-45; the portable child mirrors spawn-many-teardown.test.ts:16-18.
• Assertions are strictly tighter than before (toBe("ok") / toBe("") / toBe(0) vs. the old not.toBe("")), so no coverage is weakened.
• .trim() on stdout handles the Windows \r\n from cmd.exe; author reports Windows x64 release verified at ~142ms.
• process.env.comspec case-insensitivity on Windows matches the existing usage in spawn-many-teardown.test.ts, which is already green in CI.
• The dropped fd-leak check has a clear rationale in b2d8507 (pipe fds close one tick after exit; the sibling tests that do assert this use expectMaxObjectTypeCount with a GC loop).
• No outstanding human review comments; only bot noise on the timeline.

[github-actions]

Closing this PR because it has been inactive for more than 90 days.