test: unquarantine the three Bun-owned http2 suites by robobun · Pull Request #34307 · oven-sh/bun

robobun · 2026-07-16T07:47:32Z

The http2 rewrite (6ef5977, #31584) quarantined its own regression suites at test/expectations.txt:243-245 while the engine was still being reconciled with the legacy parser's expectations. The reconciliation is done: all three files now pass cleanly, but the entries were never removed, so the rewritten engine has been shipping with its own regression suite switched off on every lane.

What was quarantined

test/js/node/http2/node-http2.test.js [ FAIL ]
test/js/node/http2/h2-conformance.test.ts [ FAIL ]
test/js/node/http2/node-http2-invalid-padding.test.ts [ FAIL ]

No platform modifier, so getRelevantTests in scripts/runner.node.mjs spliced these out of availableTests on every platform.

Verification

3/3 consecutive runs, exit 0, on 1.4.0-canary.1+c4fad462e:

file	darwin-arm64	linux-x64	windows-x64
node-http2.test.js	294 pass / 6 skip / 0 fail	294 pass / 6 skip / 0 fail	294 pass / 6 skip / 0 fail
h2-conformance.test.ts	37 pass / 0 fail	37 pass / 0 fail	37 pass / 0 fail
node-http2-invalid-padding.test.ts	7 pass / 0 fail	7 pass / 0 fail	7 pass / 0 fail

338 tests / 12,305 expect() calls back in the run.

Out of scope

[ ASAN ] test/js/node/http2/ [ SKIP ] stays: it covers a separate socket-layer connect-race lifetime report and is ASAN-only.
The [ LINUX ]/[ WINDOWS ] entries for test/js/node/test/parallel/test-http2-* stay: platform-scoped, separate socket-layer issue, need per-platform re-triage before touching.

no test proof · iteration 0 · docs-only change; test-proof not applicable

The http2 rewrite (6ef5977) quarantined its own regression suites while the engine was still being reconciled. All three now pass 3/3 on darwin-arm64, linux-x64, and windows-x64 against 1.4.0-canary.1+c4fad462e: node-http2.test.js 294 pass / 6 skip / 0 fail / 12217 expect() h2-conformance.test.ts 37 pass / 0 fail / 66 expect() / 2 snapshots node-http2-invalid-padding.test.ts 7 pass / 0 fail / 22 expect() These entries carried no platform modifier, so getRelevantTests in scripts/runner.node.mjs removed the files from every lane, leaving the rewritten engine with its own regression suite switched off. The [ ASAN ] prefix skip for test/js/node/http2/ and the platform-scoped [ LINUX ]/[ WINDOWS ] parallel-suite entries remain; they cover separate socket-layer issues and are out of scope here.

robobun · 2026-07-16T07:47:38Z

Verified 3/3 on darwin-arm64 (reported), linux-x64, and windows-x64 against canary c4fad46. The ASAN lane still skips this directory via the existing [ ASAN ] test/js/node/http2/ prefix entry, which is intentional and unchanged here.

CI (build #73699, final): the three unquarantined http2 suites passed on every lane. No http2-related failures anywhere in the build. Remaining red, all unrelated to this diff:

test/js/web/timers/timer-heap-race.test.ts (x64-asan): ConcurrentTask leak in JSCTaskScheduler::onScheduleWorkSoon during worker VM teardown. Known main issue; JSCTaskScheduler: drop deferred-work tasks scheduled after the event loop's last tick #34293 and JSCTaskScheduler: skip DeferredWorkTimer tasks once a worker is terminating #34270 are open for it.
test-worker-message-port-transfer-terminate.js, test-net-connect-memleak.js: pre-existing on main.
bun-install-registry.test.ts, bun-install-proxy.test.ts, require-cache.test.ts, s3-list-objects.test.ts, test-http-agent-keepalive.js, 20144.test.ts: marked flaky by the failure classifier.

This diff only deletes lines from test/expectations.txt; it cannot affect any of the above. Ready for CODEOWNER review.

robobun · 2026-07-16T07:47:44Z

^{Updated 1:33 AM PT - Jul 16th, 2026}

❌ @robobun, your commit b618f43 has 3 failures in Build #73699 (All Failures):

test/js/node/test/parallel/test-net-connect-memleak.js - code 1 on 🐧 3.23 x64
test/js/node/test/parallel/test-net-connect-memleak.js - code 1 on 🐧 3.23 x64-baseline
test/js/node/test/parallel/test-worker-message-port-transfer-terminate.js - SIGABRT on 🐧 13 x64-asan
test/js/web/timers/timer-heap-race.test.ts - pid 8794 sigabrt on 🐧 13 x64-asan

🧪 To try this PR locally:

bunx bun-pr 34307

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

bun-34307 --bun

coderabbitai · 2026-07-16T07:48:30Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 8a537984-a269-4d98-8e51-974ca977ec63

📥 Commits

Reviewing files that changed from the base of the PR and between adbaf41 and b618f43.

📒 Files selected for processing (1)

test/expectations.txt

💤 Files with no reviewable changes (1)

test/expectations.txt

Walkthrough

Changes

HTTP/2 test expectations

Layer / File(s)	Summary
Remove HTTP/2 failure quarantines `test/expectations.txt`	Removes the reconciliation comment and `FAIL` expectations for three HTTP/2 tests.

Possibly related PRs

oven-sh/bun#33952: Updates the same expectations file by removing HTTP/2 test quarantine entries.

Suggested reviewers: jarred-sumner

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly states the main change: removing quarantine for the three Bun-owned HTTP/2 suites.
Description check	✅ Passed	It covers the PR purpose and verification results, satisfying the template's required content.
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.

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

claude

This is a clean, self-correcting change (removing three stale quarantine entries so the http2 suites run again), but test/expectations.txt is CODEOWNER-protected, so leaving it for a human sign-off.

What was reviewed:

Confirmed the three removed entries are unmodified (no platform prefix) and the referenced test files exist.
Verified the [ ASAN ] test/js/node/http2/ prefix skip and the platform-scoped test-http2-* entries are untouched, matching the PR's stated scope.
Checked that no other lines were touched — pure deletion of the reconciliation block and its comment.

Extended reasoning...

Overview

The PR deletes 8 lines from test/expectations.txt: a 5-line explanatory comment block plus three unmodified [ FAIL ] quarantine entries for test/js/node/http2/node-http2.test.js, h2-conformance.test.ts, and node-http2-invalid-padding.test.ts. These were added during the http2 rewrite (#31584) while the Bun-owned suites were being reconciled with the new engine. The PR description reports 3/3 clean runs on darwin-arm64, linux-x64, and windows-x64 against canary c4fad46.

Security risks

None. No source code is touched; this only re-enables existing test files in the CI runner. The change is additive to coverage, not subtractive.

Level of scrutiny

Low on the mechanics — the diff is a pure deletion from a plain-text expectations list, and the failure mode (tests turn out to still fail on some lane) is self-correcting via CI. However, .github/CODEOWNERS explicitly assigns /test/expectations.txt to Jarred-Sumner, which per my guidelines means a human owner should sign off regardless of change simplicity.

Other factors

The three test files exist on disk and are substantial (338 tests / ~12k assertions per the PR).
The retained [ ASAN ] test/js/node/http2/ [ SKIP ] prefix entry means the ASAN lane is unaffected, consistent with the PR's out-of-scope note.
No prior reviews or outstanding comments on the timeline; robobun confirmed the 3/3 verification independently.
Worst-case downside is a red CI lane, which would surface immediately on build #73699.

robobun · 2026-09-13T18:10:47Z

Closing as part of a cleanup of stale pull requests. This PR has had no new commits since 2026-07-16, 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 requested a review from Jarred-Sumner as a code owner July 16, 2026 07:47

github-actions Bot added the claude label Jul 16, 2026

claude Bot reviewed Jul 16, 2026

View reviewed changes

robobun mentioned this pull request Aug 23, 2026

test(http2): run the six Client Basics groups of node-http2.test.js as one concurrent pool #40197

Open

robobun closed this Sep 13, 2026

Navigation Menu

test: unquarantine the three Bun-owned http2 suites - #34307

test: unquarantine the three Bun-owned http2 suites#34307
robobun wants to merge 1 commit into
mainfrom
farm/5afb6321/unquarantine-http2-suites

robobun commented Jul 16, 2026 •

edited

Loading

Uh oh!

robobun commented Jul 16, 2026 •

edited

Loading

Uh oh!

robobun commented Jul 16, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Jul 16, 2026

Uh oh!

claude Bot left a comment

Uh oh!

robobun commented Sep 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

robobun commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What was quarantined

Verification

Out of scope

Uh oh!

robobun commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

robobun commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Jul 16, 2026

Walkthrough

Changes

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Overview

Security risks

Level of scrutiny

Other factors

Uh oh!

robobun commented Sep 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

robobun commented Jul 16, 2026 •

edited

Loading

robobun commented Jul 16, 2026 •

edited

Loading

robobun commented Jul 16, 2026 •

edited

Loading