Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/runtime/test_runner/Execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,12 @@ pub struct ExecutionSequence {
pub(crate) remaining_retry_count: u32,
pub(crate) result: Result,
pub(crate) executing: bool,
/// The active entry's callback returned a promise that is still pending,
/// so the test body itself is still running. An unhandled rejection while
/// this is set records the failure but does not enqueue a completion token;
/// the body's own promise settling (or a timeout) is what advances.
/// https://github.com/oven-sh/bun/issues/14644
pub(crate) has_pending_promise: bool,
pub(crate) started_at: Timespec,
/// Number of expect() calls observed in this sequence.
pub(crate) expect_call_count: u32,
Expand All @@ -185,6 +191,7 @@ impl ExecutionSequence {
// defaults:
result: Result::Pending,
executing: false,
has_pending_promise: false,
started_at: Timespec::EPOCH,
expect_call_count: 0,
expect_assertions: ExpectAssertions::NotSet,
Expand Down Expand Up @@ -514,6 +521,7 @@ impl Execution {
let entry = unsafe { entry_ptr.as_ref() };

sequence.executing = false;
sequence.has_pending_promise = false;
if sequence.maybe_skip {
sequence.maybe_skip = false;
sequence.active_entry = match entry.failure_skip_past {
Expand Down
18 changes: 18 additions & 0 deletions src/runtime/test_runner/bun_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1173,6 +1173,24 @@ impl BunTest {

done_callback.ensure_still_alive();

// If the callback returned a pending promise, note it on the sequence
// before draining so a rejection drained below is attributed to a
// still-running body and does not prematurely enqueue a completion
// token (on_unhandled_rejection checks this). Cleared by
// `advance_sequence`.
if !result.is_empty() {
if let Some(promise) = result.as_promise() {
// SAFETY: `as_promise` returned a non-null GC-managed JSPromise.
if unsafe { (*promise).status() } == PromiseStatus::Pending {
// SAFETY: `UnsafeCell`-derived `*mut`; sole `&mut` for this
// field write (no re-entrant `.get()` between here and drop).
if let Some(sequence) = cfg_data.sequence(unsafe { &mut *this }) {
sequence.has_pending_promise = true;
}
}
}
}

// Drain unhandled promise rejections.
loop {
// Prevent the user's Promise rejection from going into the uncaught promise rejection queue.
Expand Down
10 changes: 10 additions & 0 deletions src/runtime/test_runner/jest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -602,11 +602,14 @@ pub(crate) mod on_unhandled_rejection {
let entry_ptr: Option<*mut bun_test::ExecutionEntry> = current_state_data
.entry(buntest)
.map(std::ptr::from_mut::<bun_test::ExecutionEntry>);
let mut has_pending_promise = false;
if let Some(entry) = entry_ptr {
if let Some(sequence) = current_state_data.sequence(buntest) {
if sequence.test_entry.map(|p| p.as_ptr()) != Some(entry) {
// mark errors in hooks as 'unhandled error between tests'
current_state_data = RefDataValue::Start;
} else {
has_pending_promise = sequence.has_pending_promise;
}
}
}
Expand All @@ -616,6 +619,13 @@ pub(crate) mod on_unhandled_rejection {
true,
&current_state_data,
);
if has_pending_promise {
// The test body returned a promise that hasn't settled. Record
// the failure (above) but let the body finish; its own promise
// settling (or a timeout) is what enqueues completion.
// https://github.com/oven-sh/bun/issues/14644
return;
}
buntest.add_result(current_state_data);
// `report_unhandled` reports the uncaught exception, with a guard
// for `Terminated` (which carries no pending exception to take).
Expand Down
1 change: 0 additions & 1 deletion test/js/bun/test/test-error-code-done-callback.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ test("verify we print error messages passed to done callbacks", () => {
^
error: you should see this(async)
at <anonymous> (<dir>/test-error-done-callback-fixture.ts:42:14)
at <anonymous> (<dir>/test-error-done-callback-fixture.ts:37:3)
(fail) error done callback (async)
43 | });
44 | });
Expand Down
2 changes: 1 addition & 1 deletion test/regression/issue/14624.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ test("uncaught promise rejection in async test should not hang", async () => {

expect(timeout).toBeFalse();
expect(output).toContain("test start");
// expect(output).toContain("test end"); // the process exits before this executes
expect(output).toContain("test end");
expect(output).toContain("uncaught error");
expect(exitCode).not.toBe(0);
expect(output).toMatch(/✗|\(fail\)/);
Expand Down
109 changes: 109 additions & 0 deletions test/regression/issue/14644.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
import { expect, test } from "bun:test";
import { bunEnv, bunExe, tempDir } from "harness";

// https://github.com/oven-sh/bun/issues/14644
// An unhandled promise rejection inside an async test must mark the test as
// failed but still await the returned promise before running afterEach and the
// next test.

test("unhandled rejection in async test does not advance past the test body", async () => {
using dir = tempDir("issue-14644", {
"order.test.js": `
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);
});
`,
});

await using proc = Bun.spawn({
cmd: [bunExe(), "test", "order.test.js"],
env: bunEnv,
cwd: String(dir),
stdout: "pipe",
stderr: "pipe",
});

const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
const combined = stdout + stderr;

const pick = (s: string) =>
s
.split("\n")
.map(l => l.trim())
.filter(l => l.startsWith("beforeEach") || l.startsWith("afterEach") || l.startsWith("test "));

expect(pick(stdout)).toEqual([
"beforeEach",
"test a start",
"test a end",
"afterEach",
"beforeEach",
"test b",
"afterEach",
]);

expect(combined).toContain("123");
expect(combined).toMatch(/\n 1 pass/);
expect(combined).toMatch(/\n 1 fail/);
expect(exitCode).toBe(1);
});

test("unhandled rejection from a macrotask while awaiting does not advance past the test body", async () => {
using dir = tempDir("issue-14644-timer", {
"order.test.js": `
import { afterEach, test } from "bun:test";

afterEach(() => { console.log("afterEach"); });

test("a", async () => {
console.log("test a start");
setTimeout(() => {
;(async () => { throw 123; })();
}, 1);
await Bun.sleep(10);
console.log("test a end");
});

test("b", () => {
console.log("test b");
});
`,
});

await using proc = Bun.spawn({
cmd: [bunExe(), "test", "order.test.js"],
env: bunEnv,
cwd: String(dir),
stdout: "pipe",
stderr: "pipe",
});

const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
const combined = stdout + stderr;

const pick = (s: string) =>
s
.split("\n")
.map(l => l.trim())
.filter(l => l.startsWith("afterEach") || l.startsWith("test "));

expect(pick(stdout)).toEqual(["test a start", "test a end", "afterEach", "test b", "afterEach"]);

expect(combined).toContain("123");
expect(combined).toMatch(/\n 1 pass/);
expect(combined).toMatch(/\n 1 fail/);
expect(exitCode).toBe(1);
});
Loading