test(proxy-stress-protocol): share proxy pair across concurrent tests - #33987
Conversation
102 test.concurrent cases each created a fresh adversarial proxy and origin, so ~200 listen(0, "127.0.0.1") calls per file under the test runner's rolling concurrency window. On the persistent darwin runners this has been flaking since #32635 added the suite: the proxy's net.connect(port, "localhost") occasionally lands on an unrelated server instead of the test's origin. Share one {http, https} proxy pair from beforeAll across the 68 tests that do not inspect proxy.connections. The 32 tests that assert on the proxy's connection log (multi-hop redirect, IPv6, many proxy.headers, redirect manual/error) keep dedicated proxies. Same approach as #33975 (proxy-stress-headers) and #33984 (proxy-stress-matrix). 102 tests / 508 expect() calls, identical to main.
|
Updated 11:43 AM PT - Jul 11th, 2026
❌ @robobun, your commit e926c35 has 4 failures in
🧪 To try this PR locally: bunx bun-pr 33987That installs a local version of the PR into your bun-33987 --bun |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThe proxy stress tests now share HTTP and HTTPS adversarial proxies across eligible tests, with lifecycle hooks for setup and cleanup. Tests inspecting connection state continue using dedicated proxies and document that requirement. ChangesProxy stress test reuse
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — test-only de-flake following the same pattern as #33975 and #33984.
Checked that every describe block switched to sharedProxy(...) (early-reply, large request/response headers, 1xx, HTTP/1.0, body consumers) only reads proxy.url and never touches proxy.connections, so sharing across test.concurrent is safe. The five blocks that assert on proxy.connections correctly retain dedicated await using proxies. afterAll uses ?.close() so a beforeAll failure won't throw during teardown.
Extended reasoning...
Overview
Test-only change to test/js/bun/http/proxy-stress-protocol.test.ts. Hoists two AdversarialProxy instances (one HTTP, one HTTPS) into beforeAll/afterAll and reuses them across the 68 concurrent test cases that only need proxy.url, instead of creating a fresh proxy per test. Tests that assert on the proxy's connections log keep dedicated per-test proxies with an explanatory comment. No production code touched.
Security risks
None. Test infrastructure only; no auth, crypto, permissions, or user-facing behavior changed.
Level of scrutiny
Low. This is a mechanical test refactor to reduce ephemeral-port churn causing darwin CI flakes, and it follows an already-established pattern from two sibling PRs (#33975 for proxy-stress-headers, #33984 for proxy-stress-matrix). The diff is small (~30 lines net), the intent is clear, and the PR description documents 15 debug+ASAN + 30 release verification runs with unchanged test/expect counts.
Other factors
- Verified against
proxy-stress-helpers.tsthatAdversarialProxyis an exported interface withclose(): Promise<void>, so the new import andawait sharedHttpProxy?.close()are correct. - The shared proxy handles each client connection independently (
net.createServerper-socket handler), so concurrent tests hitting the same proxy is safe as long as nothing reads the sharedconnectionsarray — confirmed none of the switched tests do. - The
sharedProxyarrow'stlsparameter shadows the module-leveltlsimport, but only within that one-line function — harmless. - Coverage is preserved: same 102 tests, same assertions; only the proxy lifetime changed.
|
CI build 71946:
Ready for review. |
2 participants
Problem
test/js/bun/http/proxy-stress-protocol.test.tshas been flaking on darwin since it was added in #32635. Always exactly 1 of 102 tests, across darwin 14 x64 and 26 aarch64:build 71945 (darwin 14 x64): the proxy's upstream dial reached an unrelated Express server instead of the test origin:
build 71690 (darwin 14 x64): reached a Verdaccio registry from another job on the same box:
build 71696 (darwin 26 aarch64): 200 response from a server that was not the test origin:
build 71727, build 71795:
ConnectionRefusedon the origin URL. All failed on the single CI retry.Cause
Same mechanism #33975 diagnosed for
proxy-stress-headers.test.tsand #33984 forproxy-stress-matrix.test.ts: 102test.concurrentcases each create a fresh adversarial proxy + origin, issuing ~200listen(0, "127.0.0.1")calls per file under the test runner's rolling concurrency window. As early tests dispose their servers, a later test'slisten(0)can be handed a just-freed port while a sibling is still mid-dial (the proxy'snet.connect(port, "localhost")goes through autoSelectFamily, adding async hops between port capture and connect). On the persistent darwin runners the"localhost"dial can additionally land on an unrelated IPv6 listener (Verdaccio / Express from another job), which is why builds 71690 and 71945 received recognisable third-party HTML.Fix
Share one
{http, https}proxy pair frombeforeAllacross the 68 tests that do not inspectproxy.connections(early-reply, large-headers, 1xx, HTTP/1.0, body-consumer matrices). The 32 tests that assert on the proxy's connection log (multi-hop redirect, IPv6,many proxy.headers, redirect manual/error) keep dedicated proxies. Per-filelisten(0)churn drops from ~200 to ~134.Coverage is unchanged: 102 tests, 508
expect()calls, identical to main. Same approach as the two open sibling PRs.Introduced by #32635; sibling fixes are #33975 and #33984.
Verification
15 consecutive debug+ASAN runs and 30 release runs on linux, all clean.
[stamp-90s] gate passed · iteration 0 · 1 files touched
passes on PR (with fix)
diff hotspot
gate history · 1 passed · 0 rejected · iteration 0
evidence per changed file