Skip to content

make more tests faster - #33704

Closed
robobun wants to merge 5 commits into
mainfrom
farm/4b4b4178/make-more-tests-faster
Closed

robobun wants to merge 5 commits into
mainfrom
farm/4b4b4178/make-more-tests-faster

Conversation

@robobun

@robobun robobun commented Jul 7, 2026 •

Copy link
Copy Markdown
Collaborator

What does this PR do?

Follow-up to #33622. Applies the same patterns (concurrency where tests spawn independent subprocesses, smaller workloads where the assertion is N-agnostic) to another ~53 files pulled from the top of expected-durations.json that #33622 did not already touch.

Concurrency (test.concurrent / describe.concurrent where each test owns its own tempDir / port:0 server / subprocess and reads only module-level consts):

  • http/fetch/ws: serve-syscall-fault, serve-reused-response, serve-direct-readable-stream, serve-pending-promise-abort-leak, serve-async-stream-client-abort, bun-serve-routes, proxy (base64-auth block + the two UAF subprocess tests), node-http-backpressure, node-https-checkServerIdentity, node-http-proxy-url, ws-syscall-fault, fetch-gzip, fetch-syscall-fault, fetch-redirect, client-fetch, server-response-stream-leak, websocket-server-upgrade-reentrant, test-ws-bidir-proxy
  • install/run: bun-pm-pkg (beforeEach tempDir moved into each test body), bun-audit (snapshot cases opt out via concurrent:false), symlink-path-traversal, bun-install-streaming-extract, lockfile-version-2, update_interactive_formatting, workspaces
  • bun-test.test.ts: runTest converted from spawnSync to async Bun.spawn so describe.concurrent actually overlaps the ~140 subprocess cases
  • misc: snapshot.test.ts inline-snapshots block, test_threadsafe_function/do.test.ts (await one shared build() promise + runAsync), zstd, require-esm-gc-roots, test-test, spawn-streaming-stdout, html-rewriter-leak, request-method-getter, node-stream (selective it.concurrent on subprocess tests), cp (2 subprocess tests), deno/webcrypto (describe.concurrent wrapper)
  • regression 13696/26207/26249/26360/26851/27890/32489/3613

Smaller workloads (assertion unchanged, non-ASAN lanes keep full coverage):

  • uv_stub: ASAN stride 6 -> 20 (every stub routes through the same CrashHandler__unsupportedUVFunction formatter)
  • bun-build-api: BUILDS 400 -> 200 for the thousands-of-builds crash test (regression panicked at ~build 133)
  • spawn-streaming-stdin: N 50 -> 32 (16 on ASAN); one batch of concurrency=16 still trips the fd/object-count assertions on a leak
  • spawn-stress: ASAN iteration count halved
  • sourcetextmodule-leak: 50k -> 20k iterations, thresholds scaled proportionally
  • fetch-tcp-stress: port-exhaustion threshold 2k on ASAN/debug (TIME_WAIT clears before a slower loop reaches 16k; the getMaxFD leak check is unchanged)
  • text-encoder: full 0x110000-codepoint sweep -> 0x20000 on ASAN (covers every UTF-8 width class against the same fixture bytes)
  • regression/32492: ROUNDS 16 -> 4 on ASAN/debug (96 worker-pool shutdowns)
  • bun-install-streaming-extract: gzip level 9 -> 3 for the all-zero 2.25 GiB payload (same compressed size)
  • bun-cryptohasher: ASAN-gated iteration count on the repeated-update loop

How did you verify your code works?

bun bd test on each changed file locally. ~16 additional candidates were dropped after debug-ASAN verification surfaced per-test-timeout overruns, shared-state conflicts, or RSS interference under concurrency (isolated-install after CI #70097 showed global-virtual-store conflicts on Windows; bun-patch/bun-install-patch/bun-pm-scan/run-autoinstall/26225 after CI #71171 showed Windows bun-install-cache ENOTEMPTY races; serve-body-leak, fetch-leak, express-memory-leak, import-attributes, domjit, leaks-test, node-http-syscall-fault, node-http-connect, bun-pack, filter-workspace, error-gc-test, process-args, bun-publish). Two no-op describe.concurrent wraps (esbuild/extra, wpt-webcrypto.generateKey) were also dropped. The bun-serve-static workload reduction from the original commit was dropped after #34081 restructured that file and documented why /big must stay 4MB; serve-response-stream-sink-leak dropped after #34009 rewrote the fixture measurement.

CI will be the real measurement; as with #33622, changed files sort to the front of their shard on the first run so per-file comparison is best done against a follow-up build.


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

@github-actions github-actions Bot added the claude label Jul 7, 2026
@robobun

robobun commented Jul 7, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 11:18 AM PT - Jul 14th, 2026

❌ @robobun, your commit 61d81ec has 1 failures in Build #72961 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 33704

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

bun-33704 --bun

@coderabbitai

coderabbitai Bot commented Jul 7, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

An error occurred during the review process. Please try again later.

Walkthrough

Changes

The test suite is broadly converted to concurrent execution. Shared temporary state is replaced with isolated workspaces, subprocess helpers become asynchronous, and ASAN/debug/platform-specific workloads are reduced or recalibrated across bundler, CLI, HTTP, runtime, NAPI, and regression tests.

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title is related to the change but too generic to identify the main concurrency and workload optimizations. Use a specific title like 'Speed up tests by running independent cases concurrently and shrinking ASAN workloads'.
✅ Passed checks (3 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.
Description check ✅ Passed The description matches the template and covers the change and verification at a useful level of detail.

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@test/bundler/bun-build-api.test.ts`:
- Around line 1476-1481: The stress test in the Bun build API suite is asserting
stderr is always empty, which is flaky for debug/ASAN runs. Update the test
around the existing stdout/exitCode checks to make stderr conditional on
failure: only assert on stderr when exitCode is non-zero, and keep the primary
success assertions on stdout and exitCode in place. Use the existing test block
in bun-build-api.test.ts with the BUILDS loop as the target for this change.

In `@test/napi/uv_stub.test.ts`:
- Line 16: Add a brief inline comment next to the ASAN branch in the symbol
selection logic in the test file’s symbol sampling expression, explaining why
the stride in symbols_to_test was widened from the previous value to the current
one; reference the isASAN conditional and all_symbols_to_test/filter so future
readers understand the coverage tradeoff and that the reduced sample size is
intentional.
🪄 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: d037fa13-5ec4-4b3d-8214-fed44e0a3168

📥 Commits

Reviewing files that changed from the base of the PR and between 1de77f9 and e27fbd6.

📒 Files selected for processing (29)
  • test/bundler/bun-build-api.test.ts
  • test/cli/install/bun-audit.test.ts
  • test/cli/install/bun-install-streaming-extract.test.ts
  • test/cli/install/bun-pm-pkg.test.ts
  • test/cli/install/isolated-install.test.ts
  • test/cli/install/symlink-path-traversal.test.ts
  • test/cli/test/bun-test.test.ts
  • test/js/bun/http/bun-serve-static.test.ts
  • test/js/bun/http/proxy.test.ts
  • test/js/bun/http/serve-direct-readable-stream.test.ts
  • test/js/bun/http/serve-pending-promise-abort-leak.test.ts
  • test/js/bun/http/serve-reused-response.test.ts
  • test/js/bun/http/serve-syscall-fault.test.ts
  • test/js/bun/spawn/spawn-streaming-stdin.test.ts
  • test/js/bun/test/snapshot-tests/snapshots/snapshot.test.ts
  • test/js/first_party/ws/ws-syscall-fault.test.ts
  • test/js/node/http/node-http-backpressure.test.ts
  • test/js/node/http/node-https-checkServerIdentity.test.ts
  • test/js/node/vm/sourcetextmodule-leak.test.ts
  • test/js/web/encoding/text-encoder.test.js
  • test/js/web/fetch/fetch-gzip.test.ts
  • test/js/web/fetch/fetch-tcp-stress.test.ts
  • test/napi/node-napi-tests/test/node-api/test_threadsafe_function/do.test.ts
  • test/napi/uv_stub.test.ts
  • test/regression/issue/13696.test.ts
  • test/regression/issue/26207.test.ts
  • test/regression/issue/26851.test.ts
  • test/regression/issue/27890.test.ts
  • test/regression/issue/32492.test.ts

Comment thread test/bundler/bun-build-api.test.ts Resolved
Comment thread test/napi/uv_stub.test.ts Resolved

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found — the concurrency conversions and workload reductions look sound and well-justified per file, but with 29 files each carrying its own isolation/coverage judgment call, it's worth a human skim and a CI pass before merging.

Extended reasoning...

Overview

Test-only follow-up to #33622 applying the same two patterns across 29 more slow test files: (1) test.concurrent/describe.concurrent where each case owns its own tempDir/port:0 server/subprocess, and (2) reduced iteration counts on ASAN/debug lanes where the assertion is N-agnostic. No production code is touched. Notable refactors: bun-pm-pkg.test.ts moves the shared beforeEach tempDir into per-test makeTestDir(), bun-test.test.ts converts runTest from spawnSync to async Bun.spawn, bun-audit.test.ts adds a concurrent: false opt-out for snapshot cases, and do.test.ts shares one build() promise across concurrent runs.

Security risks

None. Test infrastructure only; no production code, auth, crypto, or user-facing behavior is affected.

Level of scrutiny

Moderate. Each of the 29 files carries an independent judgment call: is this set of tests truly isolated (no shared mutable state, no snapshot-file writes, no RSS interference) under concurrency, and does the reduced workload still trip the target regression? The PR description is thorough and the author dropped 8 candidates that failed local ASAN verification, which is a good signal. But subtle races (e.g. the shared tmpdir/tmpid++ in snapshot.test.ts, filter-vs-path semantics in spawned bun test calls, or memory pressure from node-http-backpressure running two 2 GiB streams concurrently) are the kind of thing that only surfaces as intermittent CI flakiness over time.

Other factors

  • Follows the established #33622 pattern, so precedent exists for this shape of change.
  • Several changes reduce coverage on ASAN lanes only (uv_stub stride 6→20, text-encoder 0x110000→0x20000, fetch-tcp-stress 16k→2k, etc.) with non-ASAN lanes keeping full coverage — reasonable, but each is a coverage trade-off a maintainer should sign off on.
  • CI (#70097) is still building at review time; per the description, real measurement comes from CI, so a green run should be confirmed.
  • The bug hunter found nothing and I did not spot any concrete correctness issue on inspection.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't find bugs, but this touches 62 test files with many judgment calls (workload reductions, threshold recalibrations, concurrency-safety of shared servers/snapshots) that trade coverage for CI time — worth a human sanity-check, ideally after the BuildKite run lands.

Extended reasoning...

Overview

Follow-up to #33622: applies test.concurrent/describe.concurrent and ASAN/debug workload reductions across 62 test files spanning bundler, install, http/serve, fetch, spawn, napi, and regression suites. Includes a substantive refactor of bun-pm-pkg.test.ts (shared beforeEach tempDir → per-test makeTestDir()), a spawnSync→async Bun.spawn conversion of the runTest helper in bun-test.test.ts, and an incidental correctness fix in run-autoinstall.test.ts (was mutating the shared bunEnv object).

Security risks

None — test-only changes, no production code paths, no auth/crypto/network-trust surfaces touched.

Level of scrutiny

Medium-high. While each individual edit is mechanical, the PR contains many judgment calls that CLAUDE.md flags for scrutiny: iteration counts cut (400→200 builds, 50k→20k, 100→10, stride 6→20), leak-detection thresholds rescaled (sourcetextmodule-leak, html-rewriter-leak, fetch-tcp-stress), and concurrency applied to describe blocks that share a server via beforeAll (bun-serve-routes.test.ts). Each reduction needs to still fail on the original regression; the author states they verified locally and dropped 8 candidates that didn't hold up, but the safety margins are per-case judgment.

Other factors

The two CodeRabbit nits (unconditional stderr === "" and stride-comment) are marked resolved. The PR description says CI (#71171) is the real measurement and per-file comparison is best done on a follow-up build — a human should confirm the changed files stay green across the ASAN/debug/Windows lanes before merging. Given the volume and the explicit test-coverage tradeoffs, I'm deferring rather than approving.

@robobun
robobun force-pushed the farm/4b4b4178/make-more-tests-faster branch from 7c6d033 to 6b42278 Compare July 9, 2026 22:01

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found, but this touches 57 test files with ~30 independent workload-reduction judgment calls and several shared-state refactors for concurrency — worth a human skim before CI is the only gate.

Extended reasoning...

Overview

Test-only follow-up to #33622 applying two patterns across 57 files: (1) test.concurrent/describe.concurrent where each test owns its own tempDir/port:0 server/subprocess, and (2) smaller iteration counts under ASAN/debug where the assertion is N-agnostic. Notable structural changes: bun-pm-pkg.test.ts moves from a shared beforeEach tempDir to per-test makeTestDir(), bun-test.test.ts converts runTest from spawnSync to async Bun.spawn across ~140 cases, and do.test.ts shares one build() promise across concurrent napi tests.

Security risks

None. No production code, no auth/crypto/permissions paths, no external inputs. Purely test scheduling and workload sizing.

Level of scrutiny

Medium-high. While mechanically test-only, per CLAUDE.md "Never silently weaken, skip, or delete an existing test or safety net" is one of the most-blocked categories. Each of the ~30 iteration-count reductions (BUILDS 400→200, sourcetextmodule 50k→20k, uv_stub stride 6→20, spawn-stress 100→20, text-encoder 0x110000→0x20000, etc.) embeds a claim that the smaller N still clears the original regression's trip point. The author documents each rationale inline and dropped ~15 candidates after local ASAN verification, but these are per-test judgment calls that benefit from a maintainer spot-check.

Other factors

The bug hunting system found nothing; both CodeRabbit nits are resolved. The PR follows a merged predecessor (#33622) with the same patterns. The author explicitly defers to CI as the real measurement and has already reverted files that showed concurrency conflicts (isolated-install after CI #70097). Given 57 files and the density of coverage-tradeoff decisions, I'm not comfortable approving without a human reviewer confirming the reduced workloads still exceed the regression thresholds they guard.

@robobun

robobun commented Jul 9, 2026 •

Copy link
Copy Markdown
Collaborator Author

Build #72961 (latest, sha 61d81ec) finished with all 53 changed files green. The only hard error is test/js/node/test/parallel/test-net-connect-memleak.js, an upstream Node test this PR does not touch that also fails on builds #72903-#72916 from unrelated PRs. None of the flaky-then-passed entries are on files in this diff.

This is the fourth consecutive CI run (#71184, #72390, #72561, #72961) where the changed set is green and the only red is unrelated to the diff.

@robobun

robobun commented Jul 12, 2026

Copy link
Copy Markdown
Collaborator Author

The bun-serve-static.test.ts hunk from this PR is extracted into #34047, after that file timed out at the 180s file wall on a slow runner in build #72253.

@robobun
robobun force-pushed the farm/4b4b4178/make-more-tests-faster branch from 0e8d425 to a09da7d Compare July 13, 2026 02:25

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No bugs found, but this is a 57-file test-infra PR that both adds concurrency and reduces workload/iteration counts in ~15 places — each reduction is a judgment call about whether the regression it guards is still detectable, and one hunk (bun-serve-static.test.ts) already timed out on a slow runner and was extracted to #34047 while still present in this diff. Worth a human pass on the threshold/iteration reductions and the serve-static overlap.

Checked: concurrency additions only touch tests with per-test tempDir/port:0/subprocess isolation; bun-test.test.ts runTest sync→async conversion preserves semantics; bun-pm-pkg beforeEach→per-test tempDir keeps each case isolated; snapshot inline-tests block uses unique tmpfile paths so describe.concurrent doesn't race the shared source file.

Extended reasoning...

Overview

Test-only follow-up to #33622 applying two patterns across 57 files under test/: (1) test.concurrent / describe.concurrent on tests that spawn independent subprocesses or own their own port:0 server / tempDir, and (2) reduced iteration counts / payload sizes gated on isASAN/isDebug where the assertion is claimed to be N-agnostic. No production code (src/) is touched. The bun-test.test.ts runTest helper is converted from spawnSync to async Bun.spawn, and bun-pm-pkg.test.ts moves shared beforeEach tempDir setup into per-test factories.

Security risks

None. Changes are confined to test scheduling and workload sizing; no auth, crypto, install path-traversal logic, or network-facing code is modified.

Level of scrutiny

Moderate-to-high, and beyond what I should approve unilaterally. The concurrency additions are largely mechanical and CI-verified (build #71184 green on all 57 files). But the ~15 workload reductions — uv_stub stride 6→20, sourcetextmodule-leak 50k→20k with rescaled thresholds, fetch-tcp-stress 16k→2k under ASAN/debug, text-encoder 0x110000→0x20000 sweep, bun-build-api 400→200 builds, html-rewriter-leak N and threshold both halved, etc. — each embed a claim that the smaller N still crosses the regression's detection threshold. Those claims are plausible and commented, but per CLAUDE.md ("Never silently weaken… an existing test or safety net") they warrant a human sanity-check rather than bot approval.

Other factors

  • The recent commit history on this branch shows iterative correction: install tests were reverted after global-cache conflicts surfaced, and robobun notes bun-serve-static.test.ts timed out at the 180s wall in build #72253 and was "extracted into #34047" — yet the hunk is still present in this diff. A human should confirm whether this PR is meant to land with or without that hunk.
  • Two CodeRabbit nitpicks (unconditional stderr === "" in the 200-build stress test; missing rationale comment on uv_stub stride) are both marked resolved without code changes; the uv_stub file already carries a rationale comment above the changed line.
  • The one candidate the finder raised — describe.concurrent on the inline-snapshots block racing the shared test file — was verified not to apply: each InlineSnapshotTester run writes a unique tmpid++-suffixed tmpfile before spawning.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No bugs found, but this touches 57 test files with two kinds of judgment calls a human should sign off on: (1) concurrency conversions where shared-state safety depends on per-file details, and (2) reduced iteration counts / leak thresholds where the claim "still catches the original regression" needs domain review. One file (bun-serve-static.test.ts) already needed a follow-up fix (e5178ce) after timing out in CI, and its byteSize read changed semantics — previously always undefined, now the real Blob size — so the batch/iteration math is newly live.

Checked: bun-test.test.ts spawnSync→async conversion preserves stderr/exitCode semantics; bun-pm-pkg per-test tempDir isolation is complete (no leftover shared testDir); text-encoder fixture subarray keeps the final 0x110000+1 throw check unconditional; do.test.ts shared built promise correctly gates concurrent .js runs on the compile.

Extended reasoning...

Overview

Test-infrastructure-only PR applying two patterns across ~57 files: test.concurrent/describe.concurrent where each test owns its own tempDir/port:0 server/subprocess, and reduced iteration counts (mostly ASAN/debug-gated) where assertions are claimed N-agnostic. Follow-up to #33622. No production code touched.

Security risks

None — test-only, no auth/crypto/permissions/network-facing changes.

Level of scrutiny

Medium-high despite being test-only. The value of these tests is catching future regressions; green CI today doesn't validate that a halved iteration count or scaled leak threshold will still trip on the bug it was written for. Each workload reduction embeds a claim ("regression panicked at ~build 133", "leak is linear in N", "0x20000 covers every UTF-8 width class") that requires knowing the original bug to verify. The concurrency conversions similarly require per-file inspection for shared mutable state (module-level lets, beforeAll servers, snapshot files, install caches) — the PR description notes ~15 candidates were already reverted after debug-ASAN surfaced conflicts, which is evidence the author did this carefully but also that the hazard is real.

Other factors

  • CI build #71184 was green for all 57 changed files; the three hard failures are documented as pre-existing on other PRs.
  • bun-serve-static.test.ts timed out at the 180s wall in build #72253 and was extracted into #34047, then fixed in-place here (e5178ce) — the fix changes byteSize from always-undefined to the actual Blob size, which newly activates the "big route" batch-sizing branch. This interaction is subtle enough to warrant a human look.
  • Both CodeRabbit nits (unconditional stderr assert, uv_stub stride comment) are resolved and were pre-existing / already commented respectively.
  • The bun-pm-pkg.test.ts refactor removes afterEach cleanup (temp dirs now leak per-test), which is consistent with tempDirWithFiles conventions elsewhere but worth noting.

Given the breadth (57 files) and the number of per-file coverage-vs-speed tradeoffs, a human reviewer familiar with the original regressions should confirm the reduced workloads still detect them.

@robobun
robobun force-pushed the farm/4b4b4178/make-more-tests-faster branch from e5178ce to ad10451 Compare July 13, 2026 22:26
Comment thread test/js/deno/crypto/webcrypto.test.ts Resolved

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No bugs found, and CI is green for the changed set. Deferring to a human because ~half the changes are workload/threshold reductions (e.g. sourcetextmodule-leak 50k→20k with scaled thresholds, bun-build-api 400→200 builds, fetch-tcp-stress port-exhaustion 16k→2k on ASAN, html-rewriter-leak N and threshold halved) — each embeds a claim that the reduced N still trips the original regression, which is worth a maintainer sanity-check.

Checked: concurrency additions each own their own tempDir/port:0/subprocess (bun-pm-pkg's beforeEach→per-test tempDir refactor, bun-audit snapshot cases correctly opt out via concurrent:false, bun-test.test.ts runTest spawnSync→async conversion). The text-encoder fixture subarray math and serve-response-stream-sink-leak ITERATIONS env plumbing look correct.

Extended reasoning...

Overview

Test-infrastructure-only follow-up to #33622 across 55 files. Two patterns: (1) test.concurrent / describe.concurrent where each case owns its own subprocess/server/tempDir; (2) reduced iteration counts and scaled leak thresholds under ASAN/debug where the assertion is claimed N-agnostic. No production code touched.

Security risks

None — test-only, no auth/crypto/permissions logic changed.

Level of scrutiny

Moderate. The concurrency additions are mechanical and low-risk (CI already caught and reverted the ones with shared-state conflicts across builds #70097/#71171). The workload reductions are the part that needs judgment: each one asserts the reduced N still detects the original regression (e.g. "panics at ~build 133 so 200 clears it", "leak is linear in N so halved threshold preserves ratio"). These are plausible and commented, but a maintainer familiar with the specific regressions should confirm none of them cross below the detection floor.

Other factors

CI build #72390 is green for all 57 changed files with only unrelated failures. The author has already iterated substantially (dropped ~16 candidates after debug-ASAN verification, reverted several after Windows CI). My prior inline nit on the deno webcrypto GENERATED header is resolved. The remaining risk is latent flakiness or lost regression coverage that only surfaces over time, which CI green on one build doesn't rule out.

robobun and others added 5 commits July 14, 2026 16:25
Follow-up to #33622. Applies test.concurrent/describe.concurrent where
tests spawn independent subprocesses or port:0 servers with no shared
mutable state, and reduces workload sizes where the assertion is
N-agnostic.

Concurrency:
- serve-syscall-fault, serve-reused-response, serve-direct-readable-stream,
  serve-pending-promise-abort-leak, proxy (base64/UAF subprocess blocks),
  ws-syscall-fault, node-http-backpressure, node-https-checkServerIdentity,
  fetch-gzip, bun-pm-pkg (per-test tempDir instead of beforeEach),
  bun-audit (snapshot cases opt out via concurrent:false), isolated-install,
  symlink-path-traversal, bun-install-streaming-extract, bun-test.test
  (spawnSync runTest -> async Bun.spawn so describe.concurrent actually
  overlaps), snapshot.test inline-snapshots block,
  test_threadsafe_function (await shared build promise + runAsync),
  regression 13696/26207/26851/27890.

Smaller workloads (assertion unchanged):
- uv_stub: ASAN stride 6 -> 20 (all stubs hit the same CrashHandler
  formatter; full set still runs on non-ASAN lanes).
- bun-serve-static: 4MB -> 512KB big-route payload and /3 batch divisor
  on ASAN (still multi-write; 4MB existed only to stress the loop).
- bun-build-api: BUILDS 400 -> 200 for the thousands-of-builds crash
  test (regression panicked at ~build 133).
- spawn-streaming-stdin: N 50 -> 32 (16 on ASAN); fd/stream leak check
  still fires at one batch of concurrency=16.
- sourcetextmodule-leak: 50k -> 20k iterations with thresholds scaled
  proportionally.
- fetch-tcp-stress: port-exhaustion threshold 2k on ASAN/debug (TIME_WAIT
  clears before a slower loop hits 16k; fd-leak assertion is unchanged).
- text-encoder: full 0x110000 codepoint sweep -> 0x20000 on ASAN (covers
  all four UTF-8 width classes against the same fixture bytes; full sweep
  still runs on non-ASAN).
- regression/32492: ROUNDS 16 -> 4 on ASAN/debug (96 worker-pool
  shutdowns still exercise the stall path).
- bun-install-streaming-extract: gzip level 9 -> 3 for the all-zero
  2.25GiB payload (same output size, several times faster).

29 files with a combined ~767s median on linux-x64-asan per
expected-durations.json. Verified each with bun bd test locally; 8 other
candidates were dropped after debug-ASAN verification surfaced timeouts
or RSS interference under concurrency.
isolated-install.test.ts reverted: CI #70097 showed 7-13 failures on the
Windows lanes. The global-virtual-store describe shares a global cache dir
and the peer-dependency-hash tests depend on resolution order, so concurrent
installs produce different +hash suffixes and wipe each other's store entries.

Round 2 (6-13s asan tier) adds concurrency to 27 files whose tests each own
their own tempDir / port:0 server / subprocess:
- http: serve-async-stream-client-abort, bun-serve-routes,
  serve-response-stream-sink-leak, node-http-proxy-url, fetch-syscall-fault,
  fetch-redirect, client-fetch, server-response-stream-leak
- install/run: lockfile-version-2, bun-patch, bun-install-patch, bun-pm-scan,
  run-autoinstall, workspaces, update_interactive_formatting
- misc: websocket-server-upgrade-reentrant, test-ws-bidir-proxy, zstd,
  require-esm-gc-roots, test-test, spawn-streaming-stdout,
  html-rewriter-leak, request-method-getter, node-stream (selective
  it.concurrent on subprocess tests), cp (2 subprocess tests),
  deno/webcrypto (describe.concurrent wrapper)
- regression 26225/26249/26360/32489/3613

Workload reductions:
- spawn-stress: N halved under isASAN (fd-leak + object-count assertions
  are per-iteration).
- bun-cryptohasher: isASAN-gated iteration count on the repeated-update loop.
- serve-response-stream-sink-leak fixture: smaller response payload (same
  streaming path).

6 additional round-2 candidates were dropped after bun bd verification
surfaced real failures: bun-pack (incomplete per-test packageDir conversion
in the 'files' block), filter-workspace (self-referential-symlink test mutates
the shared cwd_root fixture), error-gc-test and process-args (concurrent
overruns default timeout on debug asan), bun-publish (tests write to the
shared Verdaccio registry), and the stale-edit from the reverted
isolated-install.
CI #71171 showed ENOTEMPTY races on Windows (NtSetInformationFile) when
multiple concurrent bun install subprocesses extract the same package
version to the shared cache directory: the per-version cache entry is
populated by an atomic directory rename that Windows rejects while any
other process holds a handle into the destination. Linux rename(2) is
atomic and my local bun bd verification did not surface it.

Reverted (all hit the real npm registry or autoinstall, so the shared
~/.bun/install/cache is the contention point):
- cli/install/bun-install-patch.test.ts
- cli/install/bun-patch.test.ts
- cli/install/bun-pm-scan.test.ts
- cli/run/run-autoinstall.test.ts
- regression/issue/26225.test.ts

lockfile-version-2.test.ts is kept: it uses file: deps only (no cache).
update_interactive_formatting.test.ts is kept: per-test VerdaccioRegistry
createTestDir with its own cache dir.
CI #72548 showed one retry-flake on Windows 2019 x64 where the first of the
two concurrent tests observed 'bun: command not found: node' instead of the
expected node-symlink-created path. Not seen on other recent PR builds, so
reverting to serial to avoid introducing a new flake.
@robobun
robobun force-pushed the farm/4b4b4178/make-more-tests-faster branch from a3e20c4 to 61d81ec Compare July 14, 2026 16:26

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't find correctness issues, but this touches 53 test files and makes several judgment calls about how far leak/stress-test iteration counts and thresholds can be safely reduced (e.g. sourcetextmodule-leak 50k→20k with rescaled threshold, fetch-tcp-stress 16k→2k on ASAN/debug, text-encoder full-codepoint sweep gated to ASAN). CI is green on the changed set, but the coverage-tradeoff decisions are worth a human sign-off.

Checked: concurrent blocks that share a beforeAll server (bun-serve-routes, bun-audit, update_interactive_formatting) only read from it; bun-audit snapshot cases correctly opt out via concurrent: false; bun-test.test.ts async runTest conversion preserves the try/finally rmSync; bun-pm-pkg per-test tempDir refactor covers every former testDir! reference.

Extended reasoning...

Overview

Test-infrastructure-only PR applying two mechanical patterns across 53 files: (1) test.concurrent / describe.concurrent where each test owns its own subprocess/tempDir/port:0 server, and (2) reduced iteration counts (mostly ASAN/debug-gated) with proportionally scaled thresholds on leak/stress tests. No production code is touched. Follow-up to #33622.

Security risks

None. Test-only changes with no new external inputs, network endpoints, or privilege changes.

Level of scrutiny

Medium-high. The concurrency conversions are mechanical and low-risk once isolation is verified (which CI has done — build #72561 green on all changed files, and the author already reverted ~16 candidates that surfaced shared-state conflicts). The workload reductions are the part that needs human judgment: each carries a comment justifying why the smaller N still trips the regression, but CLAUDE.md's "never silently weaken an existing test or safety net" guidance means a maintainer should confirm each tradeoff (particularly the non-ASAN-gated ones like bun-build-api 400→200, spawn-streaming-stdin 50→32, sourcetextmodule-leak 50k→20k).

Other factors

  • CI is green on the changed set per robobun's #72561 report; only unrelated files failed/flaked.
  • The PR description documents which candidates were tried and reverted, showing the author verified isolation empirically rather than assuming it.
  • I spot-checked several concurrent conversions for shared state: bun-serve-routes describe blocks share a read-only server via beforeAll (safe for concurrent GETs); bun-pm-pkg moved beforeEach tempDir into each test body so nothing is shared; snapshot.test.ts uses a sync tmpid++ at collection time so concurrent bodies get unique paths; do.test.ts correctly awaits a shared build promise before every concurrent test.
  • The one prior inline nit I left (deno webcrypto GENERATED header) is resolved and non-blocking.
  • Deferring rather than approving because 53 files with per-test coverage-reduction decisions is beyond the scope I should auto-approve, even with green CI.

Jarred-Sumner pushed a commit that referenced this pull request Jul 29, 2026
…ing assertions (#36348)

### What does this PR do?

`test/cli/update_interactive_formatting.test.ts` runs 32 tests serially,
each spawning `bun install` + `bun update -i`, for ~13s wall time on the
debian 13 x64-asan lane (build #84540).

#### Speed

Wrap the describe block in `describe.concurrent`. Every test already
creates its own `tempDir` with `cache = false` and the Verdaccio
registry is read-only after `beforeAll`, so there is no shared state.
Extract `install()` / `updateInteractive()` helpers for the boilerplate
and so that an install failure surfaces stderr instead of `Expected: 0,
Received: 1`.

Local `bun bd test` (debug+ASAN, 3/5 runs each):

| | before | after |
| --- | --- | --- |
| wall time | ~14.2s | ~7.4s |
| tests | 32 pass | 32 pass |
| lines | 2100 | 1189 |

#### Assertions

The first six tests in this file wrote a JSON blob to `bun.lockb` (which
is the *binary* lockfile format) or no lockfile at all, so `bun update
--interactive` failed at lockfile parse:

```
error: failed to parse lockfile: InvalidLockfile       # tests 1-3, 6
error: missing lockfile, nothing outdated              # tests 4-5
```

and never reached `process_multi_select` (the column-layout code the
test names refer to). The only assertions were
`expect(stderr).not.toContain("panic"|"segfault"|"overflow"|"underflow")`,
which pass for any stderr that happens not to contain those words.

Those six are rewritten to install real registry packages, reach the
selection prompt, and assert on the rendered `Current/Target/Latest`
header and row content (column alignment, section headers for each
dependency type, catalog/workspace entries listed). The remaining 18
`.not.toContain("panic")` assertions elsewhere in the file are dropped,
and tests that previously asserted only `exitCode === 0` now check
stdout (typically `"Installing updates..."`) first so a failure shows
the actual output.

Removed a stray `console.log("Checking lockfile...")` and the
`console.error` debug prints in the select-all test.

### How did you verify your code works?

`bun bd test test/cli/update_interactive_formatting.test.ts` before and
after (numbers above); after-state run 8 times with no failures.

#33704 touches this file with the minimal `describe` ->
`describe.concurrent` change as part of a wider sweep; this PR subsumes
that change for this file and additionally fixes the dead assertions.

<!-- robobun:evidence:begin -->

---

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

<!-- robobun:evidence:end -->
Jarred-Sumner pushed a commit that referenced this pull request Aug 18, 2026
…#39539)

### Problem
- `test/js/node/http/node-https-checkServerIdentity.test.ts` takes 20 to
25 s on the debian x64-asan lane (`expected-durations.json` has `asan:
25420`). On every other lane it takes 0.2 to 0.4 s.
- The file spawned one bun child per test and ran the four children one
after another. Each child starts a debug ASAN process, loads
`node:https`, and does one TLS handshake. On a local debug build one
child costs about 3.4 s: about 1.4 s for `require("https")`, about 0.8 s
for the TLS round trip, the rest is process start.
- Because of the 25 s median, `parallel-allowlist.json` lists the file
in `excludeFiles`. It runs alone in the serial phase of each shard.

### Fix
- The hostname mismatch case still runs in a child, with the same
sequence as before: the request emits `error`, the child closes the
server and exits, and the test requires an empty `stderr` and exit code
0. On the ASAN lane the child inherits `detect_leaks=1`
(`scripts/runner.node.mjs:2003-2007`), so a crash or a leak on the
reject path fails this one test. This child is how the close_notify leak
of #30368 was found (`packages/bun-usockets/src/crypto/openssl.c:1878`),
and that fix has no other test.
- The other three cases start their `https` server and make their
request in the test process. They check behavior only, so they do not
need their own process. This removes three spawns. With one spawn left,
there is no script template to share.
- The describe block is `describe.concurrent`. Each case owns its server
on port 0. The cases share no state.
- Stronger assertions:
- mismatch: the child prints one JSON object. The test compares `code`,
`reason`, `host`, the `subject.CN` of `err.cert`, and `message` with
`toEqual`. Node v26.3.0 produces the same five values for this
certificate (checked with the same script). The `stderr` check stays
first and the exit code check stays last.
- custom `checkServerIdentity`: the test records each call and expects
exactly one call with hostname `"localhost"` (the `https.request()`
default host), subject CN `agent1`, and issuer CN `ca1`. Before, it only
checked that the callback ran.
- CN fallback, and server with `ca` but no `requestCert`: the tests
compare `{ statusCode: 200, body: "ok" }` with `toEqual`. Before, they
checked the body only.
- The old header described the client path of #29829. `https.request()`
connects through `tls.connect()` since #31587
(`src/js/node/https.ts:277`), so the header now gives the reason above
for the child.
- Overlap, for the record: the vendored
`test-https-client-checkServerIdentity.js`,
`test-https-agent-servername.js`, and
`test/js/node/tls/node-tls-connect-hostname-verification.test.ts` cover
the outcome of each of the four scenarios. What only this file pins at
the `https` layer: `reason`, `host`, `cert`, and `message` of the error,
the arguments of a custom `checkServerIdentity`, and the clean exit of
the child. The task for this PR is to make the file faster without
removing tests, so it keeps the file. Deleting it is a separate
decision.
- `parallel-allowlist.json` and `expected-durations.json` are generated
from CI data by their scripts, so this PR does not edit them. The file
leaves `excludeFiles` on the next regeneration once its asan median is
under 15 s.
- Verification on one machine with the same debug build, `bun bd test
test/js/node/http/node-https-checkServerIdentity.test.ts`, three runs
each, best run:
- before: 16994 ms wall (bun reports 16.20 s), tests 3.4 to 3.7 s each,
in series
- after: 7190 ms wall (bun reports 6.40 s), the child takes 3.3 s and
the three in-process cases take 1.2 to 1.4 s and overlap with it
  - the file still has the same four tests with the same names
- a mutation run (wrong expected `reason`, a callback that returns an
error, a wrong `servername`) fails the three cases it touches
- #33704 is a 53-file sweep that only changes the `describe` of this
file to `describe.concurrent`. It has conflicts and does not change the
assertions.

### Background
- `tls.checkServerIdentity(hostname, cert)` is the Node function that
compares a hostname with the peer certificate. When the certificate has
no subjectAltName, it compares against the subject CN. On a mismatch it
returns an error with `code: "ERR_TLS_CERT_ALTNAME_INVALID"`, `reason`,
`host`, and `cert`. A `checkServerIdentity` option on the request
replaces this function.
- The fixture `agent1-cert.pem` (in `test/js/node/test/fixtures/keys`)
has CN `agent1`, no subjectAltName, and is signed by `ca1`. Every
request in the file trusts `ca1`, so the identity check alone decides
the result.
- On the ASAN lane the runner sets `ASAN_OPTIONS=detect_leaks=1` and
`BUN_DESTRUCT_VM_ON_EXIT=1` for the test process. `bunEnv` copies
`process.env`, so a spawned child gets them too, and a leak prints a
report to its `stderr` at exit.
- The CI runner runs a test file in the parallel bucket (next to other
files on the same 8 vCPU box) only when its slowest lane median is at or
under 15 s. This is why one child is better than four concurrent
children here: both give about the same wall time on an idle machine,
but one child uses about a quarter of the CPU when the file shares the
box.

<!-- robobun:evidence:begin -->

---

**[stamp-90s]** gate passed · iteration 0 · 1 files touched

<details><summary>passes on PR (with fix)</summary>

```console
Test-only change.

Debug/ASAN (expected pass):
$ bun bd test 'test/js/node/http/node-https-checkServerIdentity.test.ts'
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test test/js/node/http/node-https-checkServerIdentity.test.ts
bun test v1.4.0 (8326d1b)

test/js/node/http/node-https-checkServerIdentity.test.ts:
(pass) https.request checkServerIdentity > falls back to Subject CN when no SAN is present [1471.64ms]
(pass) https.request checkServerIdentity > custom checkServerIdentity overrides the native check [1296.33ms]
(pass) https.request checkServerIdentity > https.Server with ca but no requestCert accepts clients without a cert [1265.60ms]
(pass) https.request checkServerIdentity > hostname mismatch emits error without crashing [3413.53ms]

 4 pass
 0 fail
 7 expect() calls
Ran 4 tests across 1 file. [6.62s]
Exit: 0
```

</details>

<details><summary>diff hotspot</summary>

```
.../http/node-https-checkServerIdentity.test.ts    | 231 +++++++--------------
 1 file changed, 79 insertions(+), 152 deletions(-)
```

</details>

**gate history** · 1 passed · 0 rejected · iteration 0

<details><summary>evidence per changed file</summary>

```
file                                                      reads  edits  tests
test/js/node/http/node-https-checkServerIdentity.test.ts      3      3      0
```

</details>

<!-- robobun:evidence:end -->
Jarred-Sumner pushed a commit that referenced this pull request Aug 30, 2026
### Problem
- `test/js/bun/util/bun-cryptohasher.test.ts` took 42s on the debian 13
x64-asan lane (build #108217). It ran 55,719 `expect()` calls, most in
100-iteration loops that compared a hash result with itself. At the GC
level CI sets (`GCLevel::Mild`), every matcher call runs a collection
(`post_match` in `src/runtime/test_runner/expect.rs`).
- Three tests each spawned a child `bun -e`. The coercion fixtures ran
`Bun.gc(true)` eight times over 1 MiB buffers.

### Fix
- Replace each self-comparison loop with one pass against a literal
digest, checked against python hashlib and openssl. Buffer and Blob
inputs, four API paths, and every encoding stay covered.
- Collapse the three child processes into one fixture that prints one
JSON object. Detach with `ArrayBuffer.prototype.transfer(0)`, which
frees the backing store at once, so no `Bun.gc(true)` is needed.
- Pin the error codes and messages that a bare `toThrow()` covered
before.
- Verified: `bun bd test test/js/bun/util/bun-cryptohasher.test.ts`
(debug ASAN build). Before: 17.4s, 404 tests, 55,719 expects. After:
2.9s, 104 tests, 443 expects. With `BUN_GARBAGE_COLLECTOR_LEVEL=1` as CI
sets it: 209.8s with 12 timeouts before, 6.3s and green after. 3 runs
each way, no flakes. On the debian 13 x64-asan lane in CI (build
#108295): 569ms.

### Background
- `Bun.CryptoHasher` wraps BoringSSL EVP digests. `Bun.SHA256` and the
other static hashers are fixed-algorithm classes with the same surface.
`Bun.sha` is `Bun.SHA512_256.hash`.
- The coercion fixture tests argument order. A `toString()` hook on one
argument detaches the buffer behind another, or digests the hasher,
before the hash runs. A correct hash then sees zero input bytes, or an
output buffer that is too small.

<details><summary>Notes</summary>

Baseline per-group times (local debug ASAN, default env): "Hash is
consistent" 12 tests 10.1s, "CryptoHasher" 330 tests 3.0s, three spawn
tests 1.2s, HMAC 44 tests 0.16s. With `BUN_GARBAGE_COLLECTOR_LEVEL=1`
each "Hash is consistent" test took about 16s and hit the 5s per-test
timeout.

After: the single spawn test 0.33s, HMAC 0.13s, reference digests 0.10s,
CryptoHasher 0.09s. With GC level 1: HMAC 1.6s, CryptoHasher 0.68s,
reference digests 0.54s, spawn 0.35s. Four runs each: 2.86s to 3.03s and
6.14s to 6.48s. The file also passes on the release build (95ms).

Digest literals: 32 algorithm names (including the aliases `sha128`,
`rmd160`, `sha-1`, `sha-512/224`, and so on) hashing "hello", and four
32-byte inputs under sha1, sha256, sha512, and md5. All were checked
against python hashlib, md4 against `openssl dgst -provider legacy
-md4`. The `algorithm` getter is now checked to report the canonical
name of an alias, and `byteLength` to match the digest length.

Why one pass is enough: the original loops came from #7757, where
`encodeWithMaxSize` encoded bytes past the digest, so repeated calls
disagreed. A literal digest catches that on the first call. A pinned
value also catches a wrong algorithm mapping, which the self-comparison
never could.

Why `transfer(0)` instead of `structuredClone` plus `Bun.gc(true)`:
`arrayBufferCopyAndDetach` copies into a new zero-length buffer and
drops the old contents at once, so a stale pointer is a use-after-free
immediately, and the digest assertion fails whether or not ASAN is on.
Buffers are 64 KiB instead of 1 MiB. The fixture stays in a child
process so that fault cannot take down the runner.

Pinned messages that were bare `toThrow()` before: HMAC consumed state
(`digest`, `byteLength`, `copy`, and now `update`), `Bun.file` input,
missing `update` argument, unsupported HMAC algorithms, non-latin1
algorithm names.

Unchanged tests: odd-length hex, two-byte hex decode, `Bun.sha` argument
validation, `static hash requires the algorithm to be a string
primitive`.

Related: #33704 also touches this file (it lowers the loop count to 10
on ASAN). That PR predates the spawn tests and is conflicting with main.
This PR removes those loops.
</details>

<!-- robobun:evidence:begin -->

---

**[stamp-90s]** gate passed · iteration 0 · 1 files touched

<details><summary>passes on PR (with fix)</summary>

```console
Test-only change.

Debug/ASAN (expected pass):
$ bun bd test 'test/js/bun/util/bun-cryptohasher.test.ts'
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test test/js/bun/util/bun-cryptohasher.test.ts
bun test v1.4.1 (d578a8c)

test/js/bun/util/bun-cryptohasher.test.ts:
(pass) Bun.file in CryptoHasher is not supported yet [10.69ms]
(pass) CryptoHasher update should throw when no parameter/null/undefined is passed [6.66ms]
(pass) CryptoHasher.update(str, 'hex') rejects odd-length hex like node:crypto [33.29ms]
(pass) update(str, 'hex') decodes two-byte strings from the low byte of each code unit like node [15.43ms]
(pass) CryptoHasher throws on non-latin1 algorithm names instead of crashing [9.29ms]
(pass) static hash requires the algorithm to be a string primitive [10.11ms]
(pass) hash functions read their buffers and hasher state only after every argument has been coerced [315.66ms]
(pass) Bun.sha validates its arguments like Bun.SHA512_256.hash [22.79ms]
(pass) HMAC > sha1 (key: String) [13.68ms]
(pass) HMAC > sha256 (key: String) [3.23ms]
(pass) HMAC > sha384 (key: String) [2.51ms]
(pass) HMAC > sha512 (key: String) [2.87ms]
(pass) HMAC > blake2b512 (key: String) [3.29ms]
(pass) HMAC > md5 (key: String) [2.75ms]
(pass) HMAC > sha224 (key: String) [2.44ms]
(pass) HMAC > sha512-224 (key: String) [2.46ms]
(pass) HMAC > sha512-256 (key: String) [2.42ms]
(pass) HMAC > sha3-224 (key: String) [2.85ms]
(pass) HMAC > sha3-256 (key: String) [2.52ms]
(pass) HMAC > sha3-384 (key: String) [2.53ms]
(pass) HMAC > sha3-512 (key: String) [2.46ms]
(pass) HMAC > ripemd160 (key: String) [3.12ms]
(pass) HMAC > sha1 (key: Buffer) [2.47ms]
(pass) HMAC > sha256 (key: Buffer) [2.42ms]
(pass) HMAC > sha384 (key: Buffer) [2.93ms]
(pass) HMAC > sha512 (key: Buffer) [3.53ms]
(pass) HMAC > blake2b512 (key: Buffer) [3.18ms]
(pass) HMAC > md5 (key: Buffer) [2.93ms]
(pass) HMAC > sha224 (key: Buffer) [2.40ms]
(pass) HMAC > sha512-224 (key: Buffer) [2.37ms]
(pass) HMAC > sha512-256 (key: Buffer) [2.37ms]
(pass) HMAC > sha3-224 (key: Buffer) [2.45ms]
(pass) HMAC
... (truncated)
Exit: 0
```

</details>

<details><summary>diff hotspot</summary>

```
test/js/bun/util/bun-cryptohasher.test.ts | 588 +++++++++++++++---------------
 1 file changed, 294 insertions(+), 294 deletions(-)
```

</details>

**gate history** · 1 passed · 0 rejected · iteration 0

<details><summary>evidence per changed file</summary>

```
file                                       reads  edits  tests
test/js/bun/util/bun-cryptohasher.test.ts      7      9      0
```

</details>

<!-- robobun:evidence:end -->
Jarred-Sumner pushed a commit that referenced this pull request Sep 6, 2026
…he full report (#41423)

### Problem
- `test/regression/issue/26851.test.ts` takes up to 12s on the debian 13
x64-asan lane. It is in the slowest 5 percent of test files. Both tests
spawn a `bun test --bail` child, and they ran one after the other.
- On that lane each child exits 134, not 1. `bun test --bail` exits with
a bare `exit(1)`, so the `BUN_DESTRUCT_VM_ON_EXIT` teardown is skipped
and LeakSanitizer aborts the child after several seconds of report
symbolization (#32183, fix open in #39010). Locally each child takes
0.3s. Under the lane environment it takes 3.8s. The old
`expect(exitCode).not.toBe(0)` hid the abort.
- The old assertions only checked that the JUnit file exists and
contains four substrings. They did not check the counters, the failure
element, the bail message, or the exit code.

### Fix
- Add the file to `test/no-validate-leaksan.txt`, next to
`test/regression/issue/12250.test.ts`, which is on the list for the same
bail exit. The entry is to be removed when #39010 lands. Without
LeakSanitizer the children exit 1 in well under a second.
- Run both tests with `test.concurrent`. Each test owns its tempDir and
its outfile, so the two children overlap. A shared `runBailWithJUnit`
helper does the spawn and reads the report.
- Assert the full JUnit document with `toMatchInlineSnapshot`: the
`testsuites` root with `tests`, `assertions`, `failures` and `skipped`,
one `testsuite` per file that ran, the `testcase` names and lines, and
the `failure` element with its type and message. Only the `time` and
`hostname` attributes are normalized. Assert the child output before the
exit code: stdout is the version banner, stderr has the
`(pass)`/`(fail)` lines, the `Ran N tests across M files.` summary and
`Bailed out after 1 failure`. The exit code is exactly 1.
- Add a second test after the failing one, and a third test file
`c_never.test.ts`. Neither appears in stderr or in the report. This
proves that `--bail` stopped the run and that the report holds only what
ran. Discovery order of the root directory is sorted by name, so
`a_pass` runs before `b_fail`.
- Verified: `bun bd test test/regression/issue/26851.test.ts`. Plain, 3
runs: before 2.66s to 3.14s wall, after 2.27s to 2.44s wall. With the
ASAN lane environment (`BUN_DESTRUCT_VM_ON_EXIT=1`, `detect_leaks=1`):
before 9.53s with both children exiting 134, after the list entry that
environment is not applied. Also passes with `USE_SYSTEM_BUN=1`.

### Background
- `--bail` makes `bun test` stop after the first failing test. #26851
was about the JUnit outfile not being written in that case.
- `test/no-validate-leaksan.txt` lists test files for which
`scripts/runner.node.mjs` does not set `BUN_DESTRUCT_VM_ON_EXIT` and
`detect_leaks=1` on the ASAN lane.
- #33704 also adds `test.concurrent` to this file as part of a broad
speed pass. This PR is scoped to the one file and also strengthens the
assertions.

<details><summary>Notes</summary>

The first CI run (build 110365) failed on the x64-asan lane with exit
code 134 on both tests and 8.7s per test. That is the LeakSanitizer
abort described above. The fix for the bail exit itself is in #39010 and
out of scope here.

The stderr checks use `toContain` rather than a snapshot of the whole
stream. Open PRs #39010, #39276, #38974 and #38265 change parts of the
bail and reporter output, and a whole-stream snapshot would conflict
with them for no gain. The XML snapshot is the thing under test.

The failure body in the XML contains `at fail.test.ts:2:40`. The fixture
sources are built from fixed strings, not indented template literals, so
the column is stable. `test/js/junit-reporter/junit.test.js` snapshots
the same shape on every platform.

</details>

<!-- robobun:evidence:begin -->

---

**[auto-merge]** gate passed · iteration 1 · 2 files touched

<details><summary>passes on PR (with fix)</summary>

```console
Test-only change.

Debug/ASAN (expected pass):
$ bun bd test 'test/regression/issue/26851.test.ts'
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "test/regression/issue/26851.test.ts"
bun test v1.4.3 (e0a2b82)

test/regression/issue/26851.test.ts:
(pass) --bail writes JUnit reporter outfile [356.62ms]
(pass) --bail writes JUnit reporter outfile with multiple files [349.11ms]

 2 pass
 0 fail
 2 snapshots, 15 expect() calls
Ran 2 tests across 1 file. [2.42s]
Exit: 0
```

</details>

<details><summary>diff hotspot</summary>

```
test/no-validate-leaksan.txt        |   3 +
 test/regression/issue/26851.test.ts | 140 +++++++++++++++++++++---------------
 2 files changed, 87 insertions(+), 56 deletions(-)
```

</details>

**gate history** · 3 passed · 0 rejected · iteration 1

<details><summary>evidence per changed file</summary>

```
file                                 reads  edits  tests
test/no-validate-leaksan.txt             1      1     25
test/regression/issue/26851.test.ts      3      5     19
```

</details>

**root cause** · written by the author bot

The regression test for issue #26851 was slow because each of its five
tests spawned a separate `bun test` child one after another, and under
ASAN every child paid several seconds of startup, while the assertions
only checked that the JUnit outfile existed. The fix runs the
independent cases concurrently through a shared runner that captures
stdout, stderr, the exit code and the report once per fixture set, then
asserts the full normalized JUnit document shape, the bail message, the
summary line and the exact exit code. A follow-up strips the CI
environment variables that make the reporter …

<!-- robobun:evidence:end -->
Jarred-Sumner pushed a commit that referenced this pull request Sep 6, 2026
…1422)

### Problem
- `test/regression/issue/13696.test.ts` takes 13s on the debian 13
x64-asan lane (build 110300). Locally with a debug build it took 9.11s,
about 2.2s per test.
- Each of the three tests spawned a bun child. The child loaded
`node:http` and `node:net`, which takes about 2s in a debug build. The
tests ran one after another, and the fixture paced its body chunks with
a fixed 50ms sleep.

### Fix
- The raw `net` server and the `http.request` client now run inside the
test process. The three tests run with `test.concurrent`, each on its
own `port: 0` listener or unix socket.
- The server writes the next body chunk only after the client has
observed the previous one. No sleep and no exit timeout remain. A hang
in either direction fails on the test timeout, as before.
- Stronger assertions: the raw request line, the `Transfer-Encoding:
chunked` header, and the chunked frame of the single `write()` are
checked on the server side. After the response ends, `req.writableEnded`
must still be `false`, which proves the response arrived while the
request body was open.
- Verified: `bun bd test test/regression/issue/13696.test.ts` goes from
9.11s to 3.5s locally (10 runs, all green). The tests themselves take
about 1s. The rest is runner startup. Also green under
`USE_SYSTEM_BUN=1`.

### Background
- Issue #13696: a `ClientRequest` with one `write()` and no `end()`
never sent the request. In duplex mode the `response` event was held
back until `end()`. docker-modem does a write-once-keep-open POST for
`container.exec` stdin, so testcontainers hung. #30377 fixed it.
- Docker's exec response has no `Content-Length` and no chunked
encoding. The server writes raw frames and closes the connection, so the
client `end` event depends on the server's `sock.end()`.

<details><summary>Notes</summary>

- Before: 2444ms, 2220ms, 2073ms per test, 9.11s for the file.
`bun-debug -e 'require("http"); require("net")'` alone takes 2.0s,
against 0.3s for an empty script. That is the whole cost of the old
subprocess design.
- After: 962ms (tcp), 496ms (unix), 393ms (flushHeaders), all
concurrent, 3.5s for the file.
- The `expect()` calls run in the test body, not inside the server
callback, so a failed assertion reports against the test instead of
surfacing as an unhandled rejection.
- #33704 is an older, conflicting bulk PR that only adds
`test.concurrent` to this file. This change keeps that and removes the
subprocesses too.

</details>

<!-- robobun:evidence:begin -->

---

**[auto-merge]** gate passed · iteration 2 · 1 files touched

<details><summary>passes on PR (with fix)</summary>

```console
Test-only change.

Debug/ASAN (expected pass):
$ bun bd test 'test/regression/issue/13696.test.ts'
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "test/regression/issue/13696.test.ts"
bun test v1.4.3 (e0a2b82)

test/regression/issue/13696.test.ts:
(pass) http.request emits 'response' in duplex mode after flushHeaders() without end() [385.23ms]
(pass) http.request delivers response while request body stream is still open (tcp) [954.82ms]
(pass) http.request delivers response while request body stream is still open (unix) [486.72ms]

 3 pass
 0 fail
 10 expect() calls
Ran 3 tests across 1 file. [3.43s]
Exit: 0
```

</details>

<details><summary>diff hotspot</summary>

```
test/regression/issue/13696.test.ts | 308 +++++++++++++++++-------------------
 1 file changed, 149 insertions(+), 159 deletions(-)
```

</details>

**gate history** · 4 passed · 0 rejected · iteration 2

<details><summary>evidence per changed file</summary>

```
file                                 reads  edits  tests
test/regression/issue/13696.test.ts      3     13     12
```

</details>

<!-- robobun:evidence:end -->
@robobun

robobun commented Sep 13, 2026

Copy link
Copy Markdown
Collaborator Author

Closing as part of a cleanup of stale pull requests. This PR has had no new commits since 2026-07-14, it conflicts with main, and its last CI run failed. This is not a judgment on the fix itself. If the problem still reproduces on a current build, reopen this PR after a rebase or open a new one against main.

@robobun robobun closed this Sep 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant