bun test: await async test body after an unhandled rejection before running afterEach

[robobun]

Fixes #14644.

Repro

import { beforeEach, afterEach, test } from "bun:test";
beforeEach(() => console.log("beforeEach"));
afterEach(() => console.log("afterEach"));
test("a", async () => {
  console.log("test a start");
  ;(async () => { throw 123; })();
  await Bun.sleep(1);
  console.log("test a end");
});
test("b", async () => { console.log("test b"); await Bun.sleep(1); });

Before: test a start / afterEach / beforeEach / test b / test a end (test a's body finishes while test b is already running).
After: test a start / test a end / afterEach / beforeEach / test b.

Cause

jest::on_unhandled_rejection called add_result(current_state_data) with the currently-executing test entry's identity. BunTest::run then dequeued that token and Execution::step unconditionally called advance_sequence for it, moving on to afterEach and the next test while the test body's returned promise was still pending. When the promise later settled, its completion was rejected by get_current_and_valid_execution_sequence as stale.

Fix

Track has_pending_promise on ExecutionSequence. run_test_callback sets it when the callback returns a promise that is still Pending (before draining rejections, so it is already set when the drain fires); advance_sequence clears it. on_unhandled_rejection still records the failure via on_uncaught_exception but, when the flag is set, does not enqueue a completion token: the body's own promise settling (or a timeout) advances execution.

Tests that only wait on a done() callback (no returned promise) keep the existing behavior of advancing on an uncaught exception (done-infinity.fixture.ts).

Tests

• test/regression/issue/14644.test.ts covers the microtask-drain path and the macrotask path (rejection from a setTimeout while awaiting).
• test/regression/issue/14624.test.ts now also asserts test end is printed.
• test-error-code-done-callback.test.ts snapshot drops one frame: an async done => { await Bun.sleep(0); done(err) } test's done() no longer synchronously starts the next test inline, so that next test's async stack no longer carries a frame pointing into the previous test's body.

---

[review] gate passed · iteration 0 · 6 files touched

fails on main (without fix)

ASAN without fix: 4 FAILED
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/js/bun/test/test-error-code-done-callback.test.ts "test/regression/issue/14624.test.ts" "test/regression/issue/14644.test.ts"
bun test v1.4.0 (e90ca3317)

test/regression/issue/14624.test.ts:
37 | 
38 |   const output = stdout + stderr;
39 | 
40 |   expect(timeout).toBeFalse();
41 |   expect(output).toContain("test start");
42 |   expect(output).toContain("test end");
                      ^
error: expect(received).toContain(expected)

Expected to contain: "test end"
Received: "bun test v1.4.0 (e90ca3317)\ntest start\n\nhang.test.js:\n2 |       import { test } from 'bun:test'\n3 | \n4 |       test('async test with uncaught rejection', async () => {\n5 |         console.log('test start');\n6 |         // This creates an unhandled promise rejection\n7 |         (async () => { throw new Error('uncaught error'); })();\n                                     ^\nerror: uncaught error\n      at <anonymous> (/tmp/issue-14624_Sqzx7B/hang.test.js:7:34)\n      at <anonymous> (/tmp/issue-14624_Sqzx7B/hang.test.js:7:56)\n      at <anonymous> (/t
... (truncated)

release without fix: 4 FAILED
bun test v1.4.0-canary.1 (1498d7b77)

test/regression/issue/14624.test.ts:
37 | 
38 |   const output = stdout + stderr;
39 | 
40 |   expect(timeout).toBeFalse();
41 |   expect(output).toContain("test start");
42 |   expect(output).toContain("test end");
                      ^
error: expect(received).toContain(expected)

Expected to contain: "test end"
Received: "bun test v1.4.0-canary.1 (1498d7b77)\ntest start\n\nhang.test.js:\n2 |       import { test } from 'bun:test'\n3 | \n4 |       test('async test with uncaught rejection', async () = {\n5 |         console.log('test start');\n6 |         // This creates an unhandled promise rejection\n7 |         (async () => { throw new Error('uncaught error'); })();\n                                     ^\nerror: uncaught error\n      at  (/tmp/issue-14624_mrvsBk/hang.test.js:7:34)\n      at  (/tmp/issue-14624_mrvsBk/hang.test.js:7:56)\n(fail) async test with uncaught rejection [0.67ms]\n\n 0 pass\n 1 fail\nRan 1 test across 1 file. [15.00ms]\n"

      at <anonymous> (/workspace/bun/test/regression/issue/14624.test.ts:42:18)
(fail) uncaught promise rejection in async test should not hang [20.82ms]

test/regression/issue/1464
... (truncated)

passes on PR (with fix)

ASAN with fix: all passed
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/js/bun/test/test-error-code-done-callback.test.ts "test/regression/issue/14624.test.ts" "test/regression/issue/14644.test.ts"
bun test v1.4.0 (e90ca3317)

test/regression/issue/14624.test.ts:
(pass) uncaught promise rejection in async test should not hang [579.10ms]

test/regression/issue/14644.test.ts:
(pass) unhandled rejection in async test does not advance past the test body [652.26ms]
(pass) unhandled rejection from a macrotask while awaiting does not advance past the test body [737.15ms]

test/js/bun/test/test-error-code-done-callback.test.ts:
(pass) verify we print error messages passed to done callbacks [3265.72ms]

 4 pass
 0 fail
 2 snapshots, 19 expect() calls
Ran 4 tests across 3 files. [10.28s]
__F:0:S:0

release with fix: all passed
$ bun scripts/build.ts --profile=release
[configured] bun-profile → bun (stripped)
  target       linux-x64-gnu
  build type   Release
  build dir    ./build/release
  revision     e90ca3317a
  features     baseline

22 deps, 108 codegen, 1171 objects in 1689ms

ninja: Entering directory `/workspace/bun/build/release'
[1/1234] gen bindgenv2
[2/1234] gen ProcessBindingConstants.lut.h
Generating /workspace/bun/build/release/codegen/ProcessBindingConstants.lut.h from /workspace/bun/src/jsc/bindings/ProcessBindingConstants.cpp
[3/1234] gen .bind.ts → GeneratedBindings.cpp
[4/1234] gen ErrorCode+*.h
[5/1234] install /workspace/bun
bun install v1.4.0-canary.1 (1498d7b77)

Checked 124 installs across 170 packages (no changes) [438.00ms]
[6/1234] fetch picohttpparser
[picohttpparser] up to date
[7/1234] fetch tinycc
[tinycc] up to date
[8/1234] fetch zlib
[zlib] up to date
[9/1234] fetch libjpeg-turbo
[libjpeg-turbo] up to date
[10/1234] gen JSBuffer.lut.h
Generating /workspace/bun/build/release/codegen/JSBuffer.lut.h from /workspace/bun/src/jsc/bindings/JSBuffer.cpp
[11/1234] install /workspace/bun/packages/bun-error
bun install v1.4.0-canary.1 (1498d7b77)

Checked 1 i
... (truncated)

diff hotspot

src/runtime/test_runner/Execution.rs               |   8 ++
 src/runtime/test_runner/bun_test.rs                |  18 ++++
 src/runtime/test_runner/jest.rs                    |  10 ++
 .../bun/test/test-error-code-done-callback.test.ts |   1 -
 test/regression/issue/14624.test.ts                |   2 +-
 test/regression/issue/14644.test.ts                | 109 +++++++++++++++++++++
 6 files changed, 146 insertions(+), 2 deletions(-)

gate history · 1 passed · 0 rejected · iteration 0

evidence per changed file

file                                                    reads  edits  tests
src/runtime/test_runner/Execution.rs                        4      1      0
src/runtime/test_runner/bun_test.rs                         4      3      0
src/runtime/test_runner/jest.rs                             3      4      0
test/js/bun/test/test-error-code-done-callback.test.ts      1      0      0
test/regression/issue/14624.test.ts                         1      1      0
test/regression/issue/14644.test.ts                         0      2      0

[robobun]

Status: Gate confirmed fail-before/pass-after. Automated review found no issues.

CI build 87165: the lanes covering this change (linux x64/aarch64, alpine, debian incl. ASAN, ubuntu, macOS, Windows x64/aarch64) all pass my test and the related test-runner tests. The remaining red is unrelated: bun-upgrade.test.ts on Windows aarch64 fails with "Canary builds are not available for this platform yet" (release-artifact availability, not touched by this diff), and the rest are pre-existing flakes that passed on retry or passed alone in the parallel batch. Ready for a maintainer.

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 7 minutes

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

How can I continue?

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

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: c9e3418f-f0ec-4bc0-baf1-953a741d4ff7

📥 Commits

Reviewing files that changed from the base of the PR and between f91d5c9 and e90ca33.

📒 Files selected for processing (6)

• src/runtime/test_runner/Execution.rs
• src/runtime/test_runner/bun_test.rs
• src/runtime/test_runner/jest.rs
• test/js/bun/test/test-error-code-done-callback.test.ts
• test/regression/issue/14624.test.ts
• test/regression/issue/14644.test.ts

---

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

[github-actions]

Found 2 issues this PR may fix:

1. afterEach not playing nicely with testing library's cleanup #16926 - afterEach (cleanup) runs before async test body completes, causing document.body to be missing mid-test — exactly the premature afterEach execution this PR fixes
2. async tests are not awaited, leading to parallel execution #19660 - async test functions not fully awaited before the next test runs, causing race conditions — directly related to ensuring the test body promise settles before advancing

If this is helpful, copy the block below into the PR description to auto-close these issues on merge.

Fixes #16926
Fixes #19660

🤖 Generated with Claude Code

[robobun]

On the two linked issues:

• async tests are not awaited, leading to parallel execution #19660: the repro from this comment (Promise.reject(...) then await setTimeout) is the same bug and is fixed here: test 1 after sleep now prints before test 2 start. The original report has no repro so I'm leaving it off the Fixes list; someone on that issue can confirm.
• afterEach not playing nicely with testing library's cleanup #16926: plausibly the same root cause (afterEach running before the async body finishes), but the repro depends on happy-dom + testing-library and I haven't reproduced it locally to confirm an unhandled rejection is what triggers it there.

[claude]

I didn't find any bugs. The fix is small and looks correct, but it changes ordering semantics in the test runner's execution state machine (unhandled-rejection path now defers to the body's own promise/timeout instead of advancing immediately), so worth a human look.

What was reviewed:

• Traced the flag lifecycle: set in run_test_callback before the rejection drain, read only when the active entry is the test entry, cleared in advance_sequence and reset_sequence — no leak across entries or retries.
• Checked done-infinity.fixture.ts cases still terminate: done-only tests never set the flag (no returned promise), and async done => { … throw } advances via bun_test_catch on the body's own rejection.
• Verified bun_test_done_callback's vm.uncaught_exception routes through on_unhandled_rejection under isBunTest, which explains the dropped stack frame in the snapshot (next test no longer starts synchronously from inside done(err)).

Extended reasoning...

Overview

Adds a has_pending_promise flag to ExecutionSequence. run_test_callback sets it when the callback returns a still-pending promise (before the handle_rejected_promises drain loop), advance_sequence clears it, and on_unhandled_rejection — when the flag is set and the active entry is the test entry — records the failure via on_uncaught_exception but skips add_result/BunTest::run. The body's own .then() handler (or a timeout) is what enqueues completion. Two regression tests cover the microtask-drain and macrotask paths; 14624 is tightened; one snapshot frame is dropped.

Security risks

None. No untrusted input, no boundary crossings; this is internal test-runner sequencing.

Level of scrutiny

Medium-high. The diff is ~30 lines of runtime code, but it sits in the test runner's execution state machine where done-callbacks, returned promises, unhandled rejections, timeouts, and maybe_skip all interact. I traced the paths I could think of (done-only, async+done, async-throw, macrotask rejection, concurrent groups, hooks, retry/repeat reset) and they hold up, but this is exactly the kind of subsystem where a maintainer's mental model catches an edge I don't have.

Other factors

• done-infinity.fixture.ts still terminates: tests 4–7 return no promise so the flag is never set; tests 1–3 are async and advance via bun_test_catch when the returned promise itself rejects (that path doesn't consult the flag).
• Hooks: the flag can be set for an async hook, but on_unhandled_rejection only reads it when entry == test_entry; for hooks it falls through to RefDataValue::Start as before. advance_sequence clears the flag before the next entry.
• The snapshot change is a real behavioral improvement (next test no longer starts synchronously inside the previous test's done(err) frame), not a mask.
• One semantic shift worth a maintainer's eye: an async test whose body's promise never settles used to be advanced by an unhandled rejection; now it waits for the timeout. With the default timeout that's fine, and it's arguably more consistent with the no-rejection case, but it is a change.