Skip to content
Open
Show file tree
Hide file tree
Changes from 4 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
58 changes: 53 additions & 5 deletions src/runtime/test_runner/expect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -471,6 +471,36 @@
}
}

/// `wait_for_promise` bounded by the test's deadline; returns `true` if still pending.
fn wait_for_promise_bounded_by_test(
global_this: &JSGlobalObject,
promise: bun_jsc::AnyPromise,
) -> bool {
use bun_core::{Timespec, TimespecMockMode};
if promise.status() != js_promise::Status::Pending {
return false;
}
let deadline = Jest::runner().map_or(Timespec::EPOCH, |r| r.get_active_timeout());
let jsc_vm = global_this.vm();
let bun_vm = global_this.bun_vm().as_mut();
while promise.status() == js_promise::Status::Pending {
if jsc_vm.execution_forbidden() {
break;
}
if !deadline.eql(&Timespec::EPOCH)
&& deadline.order(&Timespec::now(TimespecMockMode::ForceRealTime))
== core::cmp::Ordering::Less
{
break;
}
bun_vm.event_loop_mut().tick();
if promise.status() == js_promise::Status::Pending {
bun_vm.event_loop_mut().auto_tick();
}
}
promise.status() == js_promise::Status::Pending
}

/// Processes the async flags (resolves/rejects), waiting for the async value if needed.
/// If no flags, returns the original value
/// If either flag is set, waits for the result, and returns either it as a JSValue, or null if the expectation failed (in which case if silent is false, also throws a js exception)
Expand All @@ -489,8 +519,17 @@
let vm = global_this.vm();
promise.set_handled(vm);

// SAFETY: bun_vm() returns the live thread-local VirtualMachine.
global_this.bun_vm().as_mut().wait_for_promise(promise);
if Self::wait_for_promise_bounded_by_test(global_this, promise) {
if !silent {
return Err(Self::throw_promise_matcher_error(
global_this, custom_label, matcher_name, matcher_params, flags,
"Expected promise to settle within the test timeout",
"Received promise that is still pending: ",
"[Promise]",
));
}
return Err(JsError::Thrown);
}

let new_value = promise.result(vm);
match promise.status() {
Expand Down Expand Up @@ -892,9 +931,14 @@
return_value = return_value_from_function;
}

if let Some(promise) = return_value.as_any_promise() {
vm.wait_for_promise(promise);
let still_pending = Self::wait_for_promise_bounded_by_test(global_this, promise);
scope.apply(vm);
if still_pending {
return Err(global_this.throw(format_args!(
"Received function returned a promise that did not settle within the test timeout",
)));
}

Check failure on line 941 in src/runtime/test_runner/expect.rs

View check run for this annotation

Claude / Claude Code Review

Timed-out toThrow() promise not marked handled — later rejection leaks as unhandled

The `still_pending` early return here bypasses `promise.unwrap(..., MarkHandled)` at line 942 without a prior `set_handled`, so a promise that rejects *after* the deadline surfaces as an unhandled rejection during the next test. The other two sites this PR modifies (`process_promise` ~L520, `execute_custom_matcher` ~L1532) both call `promise.set_handled(vm)` before waiting; add `promise.set_handled(global_this.vm())` here too, before or inside the `still_pending` branch.
Comment thread
robobun marked this conversation as resolved.
match promise.unwrap(global_this.vm(), js_promise::UnwrapMode::MarkHandled) {
js_promise::Unwrapped::Fulfilled(_) => {
return Ok((None, return_value_from_function));
Expand Down Expand Up @@ -1487,8 +1531,12 @@
let vm = global_this.vm();
promise.set_handled(vm);

// SAFETY: bun_vm() returns the live thread-local VirtualMachine.
global_this.bun_vm().as_mut().wait_for_promise(promise);
if Self::wait_for_promise_bounded_by_test(global_this, promise) {
return Err(global_this.throw(format_args!(
"Matcher `{}` returned a promise that did not settle within the test timeout",
matcher_name,
)));
}

result = promise.result(vm);
result.ensure_still_alive();
Expand Down
117 changes: 117 additions & 0 deletions test/regression/issue/14950.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
// https://github.com/oven-sh/bun/issues/14950
// `expect(pendingPromise).resolves.<matcher>()` in a sync test body must not
// hang `bun test` forever at 100% CPU; the per-test timeout has to fire.
import { describe, expect, test } from "bun:test";
import { bunEnv, bunExe, tempDir } from "harness";

async function runTestFile(name: string, body: string) {
using dir = tempDir(name, { "t.test.js": body });
await using proc = Bun.spawn({
cmd: [bunExe(), "test", "t.test.js", "--timeout", "500"],
env: bunEnv,
cwd: String(dir),
stderr: "pipe",
stdout: "pipe",
});
// On an unfixed build the inner runner never exits (the per-test --timeout
// cannot interrupt the wait_for_promise spin), so kill it ourselves instead
// of letting the outer runner's own timeout abort the assertion.
let hung = false;
const watchdog = setTimeout(() => {
hung = true;
proc.kill();
}, 20_000);
try {
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
return { stdout, stderr, exitCode, hung };
} finally {
clearTimeout(watchdog);
}
}

describe.concurrent("expect().resolves/.rejects on a not-yet-settled promise", () => {
test(".resolves on a promise resolved after the matcher call times out instead of hanging", async () => {
const { stderr, exitCode, hung } = await runTestFile(
"issue-14950-resolves",
`test("promise resolves after expect call", () => {
let resolve;
expect(new Promise(r => (resolve = r))).resolves.toBe(25);
resolve(25);
});`,
);
expect(hung).toBe(false);
expect(stderr).toContain("still pending");
expect(stderr).toMatch(/timed out after \d+ms/);
expect(exitCode).toBe(1);
}, 60_000);

test(".rejects on a never-settling promise times out instead of hanging", async () => {
const { stderr, exitCode, hung } = await runTestFile(
"issue-14950-rejects",
`test("never settles", () => {
expect(new Promise(() => {})).rejects.toThrow();
});`,
);
expect(hung).toBe(false);
expect(stderr).toContain("still pending");
expect(stderr).toMatch(/timed out after \d+ms/);
expect(exitCode).toBe(1);
}, 60_000);

test("toThrow on an async fn returning a never-settling promise times out instead of hanging", async () => {
const { stderr, exitCode, hung } = await runTestFile(
"issue-14950-tothrow",
`test("never settles", () => {
expect(() => new Promise(() => {})).toThrow();
});`,
);
expect(hung).toBe(false);
expect(stderr).toContain("did not settle within the test timeout");
expect(exitCode).toBe(1);
}, 60_000);

test("async custom matcher returning a never-settling promise times out instead of hanging", async () => {
const { stderr, exitCode, hung } = await runTestFile(
"issue-14950-custom",
`const { expect, test } = require("bun:test");
expect.extend({
toNeverSettle() { return new Promise(() => {}); },
});
test("never settles", () => {
expect(1).toNeverSettle();
});`,
);
expect(hung).toBe(false);
expect(stderr).toContain("did not settle within the test timeout");
expect(exitCode).toBe(1);
}, 60_000);

test("a test after one that spins on .resolves still runs", async () => {
const { stderr, exitCode, hung } = await runTestFile(
"issue-14950-next-test",
`test("hangs", () => {
expect(new Promise(() => {})).resolves.toBe(1);
});
test("runs after", () => {
expect(1).toBe(1);
});`,
);
expect(hung).toBe(false);
expect(stderr).toMatch(/1 pass/);
expect(stderr).toMatch(/1 fail/);
expect(exitCode).toBe(1);
}, 60_000);

test(".resolves on an already-resolved promise still passes synchronously", async () => {
const { stderr, exitCode, hung } = await runTestFile(
"issue-14950-settled",
`test("already settled", () => {
expect(Promise.resolve(25)).resolves.toBe(25);
});`,
);
expect(hung).toBe(false);
expect(stderr).toMatch(/1 pass/);
expect(stderr).not.toContain("still pending");
expect(exitCode).toBe(0);
}, 60_000);
});
9 changes: 9 additions & 0 deletions test/regression/issue/23865.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,15 @@ test("23865", async () => {
expect(normalizeBunSnapshot(stdout)).toMatchInlineSnapshot(`"bun test <version> (<revision>)"`);
expect(normalizeBunSnapshot(stderr)).toMatchInlineSnapshot(`
"23865.fixture.ts:
1 | // Should not crash
2 | test("abc", () => {
3 | expect(async () => {
4 | await Bun.sleep(100);
5 | throw new Error("uh oh!");
6 | }).toThrow("uh oh!");
^
error: Received function returned a promise that did not settle within the test timeout
at <anonymous> (file:NN:NN)
(fail) abc
^ this test timed out after 50ms.

Expand Down
Loading