Skip to content

bun test: a matcher blocked on a promise that never settles fails with the test timeout - #39089

Open
robobun wants to merge 9 commits into
mainfrom
farm/d6cfbbee/matcher-wait-test-timeout
Open

robobun wants to merge 9 commits into
mainfrom
farm/d6cfbbee/matcher-wait-test-timeout

Conversation

@robobun

@robobun robobun commented Aug 15, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

Fix

  • EventLoop::wait_for_promise_or_give_up asks a predicate before each poll. The three matcher waits poll a bun_test::WaitingEntry through it. On give-up the matcher throws Received promise that was still pending when the test timed out (or ended) and the test is reported as timed out.
  • WaitingEntry checks the clock against its entry's deadline, marks the entry's sequence timed out the way evaluate_timeout does, and throws. The timer callback leaves that deadline to it (defers_timeout_to_matcher_wait), so the completion lands on the entry, not on the run. A wait the test never awaited ends with the file.
  • Correct because nothing depends on the file timer firing inside the nested wait (on Windows it cannot, see Background), a promise that settles is still waited for, and the failure is the entry's own: a test that passes on retry exits 0.
  • Verified: test/cli/test/test-timeout-behavior.test.ts, 11 new tests, 13/13 on Linux and Windows x64 debug builds. The released binary hangs on them. Other suites in Notes.

Background

Notes

History. Until #24355 (the fix for #23865) the timeout timer requested VM termination, which crashed in the #23865 fixture. This PR supersedes #36716, which bounded the same three waits by get_active_timeout() captured at loop entry. Both failed on main at 731aa92 (5 and 6 hanging fixtures). #36716 conflicts with main since #37075, hangs when the continuation reaches the matcher after the runner has finished the file (phase Done, get_active_timeout() returns EPOCH), and does not cover hooks or concurrent groups. Its toThrow set_handled fix and its test cases (the #14950 shape, late rejection, already-settled) are folded in here.

Attribution. Execution::on_stack_entry is the entry whose callback the runner is synchronously inside. get_current_state_data() names what is executing (the entry, or only the group when tests run concurrently). RefDataValue::entry finds an entry only while it is still the one executing, which is how the runner detects outdated completions. A continuation of a test the runner already gave up on is charged to whatever runs now (like its expect() calls are). Inside a concurrent group a nested wait is charged to the group as a whole. The per-invocation refs proposed in #38880 would sharpen both.

What each case does (EndsWhen in bun_test.rs):

  • EntryTimedOut: a callback is on the stack and is not itself blocked in a matcher. The wait watches ExecutionEntry::has_timed_out and arms the file timer for the deadline on each poll (wake_at). When it throws, the callback completes and step_sequence_one evaluates the overrun as for a sync test that ran too long: the error, then this test timed out after Nms.
  • GroupTimedOut: the on-stack callback is itself blocked in a matcher (Execution::blocking_matcher_waits), so the JS running is some continuation of the group, which one unknowable. Once every executing entry of the group has timed out, the wait marks them so (evaluate_timeout) and gives up.
  • Continuation: a continuation of the one running entry, the runner live. At the entry's deadline the wait marks the sequence timed out and gives up. The body's rejection then completes the entry, attributed, and the runner advances it (retry, repeat, or report). The timer callback, seeing a continuation wait whose claim still names the one running entry (Execution::matcher_wait_claim), only kills dangling processes and re-arms for the entry's deadline, so it never moves the runner past the entry first. If the runner is past the entry anyway (a wait the test never awaited, so the test completed without it), the wait goes on while the file runs and gives up once the file is done (ended).
  • GroupOver: a continuation inside a concurrent group, entry unknown. Gives up once every executing entry has timed out (marking them), or once the group is over (ended).
  • No test file running, or a describe callback: no timeout, unbounded as before.

Windows. The first version of this branch hung in CI on both Windows lanes in the continuation fixture: the runner's timer never fired inside the nested wait. Reproduced on a Windows x64 debug build (100% CPU, no output). A JS timer inserted from inside the wait re-arms the libuv timer and unblocks it. Two fixtures avoid the limitation on purpose and say so: the concurrent-sibling one blocks both siblings on a promise that never settles, and the leaked waits are immediates, not timers.

Disarmed timer. With a server (or interval) keeping the loop alive, an earlier test's timer firing under the blocked wait disarmed the file timer, and the poll parked past the deadline. The GC idle timer woke it about once a second, so the test failed about 1 s late. With BUN_GC_TIMER_DISABLE=1 it hung. The new fixture runs with that variable set.

toThrow. get_value_as_to_throw did not mark the function's promise handled before the wait, unlike the other two sites, so a rejection arriving after the wait gave up was reported as an unhandled rejection of the next test.

Self-review findings addressed in 9aa670e. The first version stepped the runner (handle_timeout + BunTest::run) from inside the wait for continuations, so their error always arrived as an outdated rejection (Unhandled error between tests): a test that passed on retry exited 1. It also said "timed out" when a leaked wait's test had merely finished, and gave up on such a wait at once instead of letting its promise settle while the file ran.

Darwin race fixed in 454f6dc. The timer callback used to defer only when the entry had already timed out on the callback's own clock. macOS fires timers rounded to the millisecond, so the callback could run just before the deadline, decline to defer, and then step past the entry on step's later clock sample: the matcher error arrived as Unhandled error between tests (build 106465, darwin x64, the concurrent-continuation and retry fixtures). The deferral now keys on the wait's claim alone, with no clock check, and a stale claim (the runner already past the wait's entry) stops deferring instead of holding the timer off the current entry.

Also run: expect.test.js, expect-extend.test.js, expect-assertions.test.ts, test-test.test.ts, jest-hooks.test.ts, bun-test.test.ts, done-async, test-failing, test-retry-repeats-basic, concurrent.test.ts, node-test.test.ts, 23865.test.ts. expect-extend "intensive usage" and concurrent_immediate time out locally on a loaded debug build with or without this change (5 s default, about 5.2 s and 2 x 3 s children).

Found on the way, not changed here. Immediates queued in the same batch as the callback a matcher blocks in are not run until it returns. The runner busy-loops rather than sleeping while only the test timeout is pending. A completion of a timed-out attempt that arrives while its retry runs is accepted as the retry's (get_current_and_valid_execution_sequence compares the entry and the repeat count, not the attempt).


no test proof · iteration 1 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/cli/test/test-timeout-behavior.test.ts

…h the test timeout

expect(p).resolves / .rejects, toThrow() on an async function and async
expect.extend matchers wait for their promise by ticking the event loop
from inside the matcher. The per-test timeout timer does fire during
those ticks, but the runner cannot give up an entry whose callback is
still on the stack, so a promise that never settles kept the whole
bun test run ticking forever.

The three waits now go through Expect::wait_for_promise, which polls a
bun_test::WaitingEntry between ticks and gives up, throwing a matcher
error, once the entry the matcher runs in has timed out. With a callback
on the stack the matcher watches the deadline itself (its own entry's,
or every executing entry of the group when that callback is itself
blocked in a matcher, in which case it also queues the re-step the timer
would have so the entry is reported as timed out). Reached from a
continuation, it only notices the runner giving the entry or its group
up, which also ends right away a continuation that outlived its file.
@coderabbitai

coderabbitai Bot commented Aug 15, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

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: d587ad5a-e8c5-4736-8d2f-826912ea039c

📥 Commits

Reviewing files that changed from the base of the PR and between 731aa92 and daabd8b.

📒 Files selected for processing (6)
  • src/jsc/event_loop.rs
  • src/runtime/test_runner/Execution.rs
  • src/runtime/test_runner/bun_test.rs
  • src/runtime/test_runner/expect.rs
  • test/cli/test/test-timeout-behavior.test.ts
  • test/regression/issue/23865.test.ts

Included review availability: Your plan provides up to 5 included reviews per hour; 1 remains after this review.


Walkthrough

Changes

Matcher timeout flow

Layer / File(s) Summary
Promise wait termination API
src/jsc/event_loop.rs
Adds wait_for_promise_or_give_up, which reports promise settlement and supports termination callbacks.
Execution timeout claims
src/runtime/test_runner/Execution.rs
Tracks blocking matcher waits and resolves matcher-owned execution deadlines.
Matcher wait lifecycle
src/runtime/test_runner/bun_test.rs
Classifies matcher waits, polls deadlines, handles timeout and test-end states, schedules wakeups, and restores state on cleanup.
Promise matcher integration and validation
src/runtime/test_runner/expect.rs, test/cli/test/test-timeout-behavior.test.ts, test/regression/issue/23865.test.ts
Uses matcher-aware waits across promise-based matchers and adds coverage for timeout, retry, concurrency, timers, late errors, and successful settlement.

Suggested reviewers: dylan-conway, jarred-sumner, cirospaciari

Merge Risk: ⚪ Minimal · up to daabd

This change addresses matcher timeout behavior, and no actionable merge-blocking risk remains beyond normal checks and review.

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: matcher waits on unresolved promises now fail with the test timeout instead of hanging.
Description check ✅ Passed The description clearly explains the problem, implementation, affected cases, timer behavior, and verification results. It does not use the exact template headings, but it provides the required inform…
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.
Full details: Description check

Explanation

The description clearly explains the problem, implementation, affected cases, timer behavior, and verification results. It does not use the exact template headings, but it provides the required information in equivalent sections.


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

@robobun

robobun commented Aug 15, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 6:26 PM PT - Aug 26th, 2026

✅ @robobun, your commit daabd8b90dfb7b87f10b06a82153c55e700bcfb2 passed in Build #106502! 🎉


🧪   To try this PR locally:

bunx bun-pr 39089

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

bun-39089 --bun

@robobun

robobun commented Aug 15, 2026 •

Copy link
Copy Markdown
Collaborator Author

Reproduced on the released binary: each of the six hanging fixtures in test/cli/test/test-timeout-behavior.test.ts makes the spawned bun test hang until the file timeout kills it. Fix in this PR. CI is green, ready for review.

Comment thread src/jsc/event_loop.rs Resolved
Comment thread src/runtime/test_runner/Execution.rs Resolved
Comment thread src/runtime/test_runner/bun_test.rs Resolved
Comment thread src/runtime/test_runner/bun_test.rs Resolved
Comment thread src/runtime/test_runner/bun_test.rs Resolved
Comment thread src/runtime/test_runner/bun_test.rs Resolved
Comment thread src/runtime/test_runner/bun_test.rs Resolved
Comment thread src/runtime/test_runner/bun_test.rs Resolved
Comment thread src/runtime/test_runner/bun_test.rs Resolved
Comment thread src/runtime/test_runner/expect.rs Resolved
…3865 snapshot

Debug builds report one more frame than release builds for errors thrown
inside async test bodies, so the fixtures' stderr is compared without the
frame lines. The 23865 fixture is a toThrow() blocked past its test's
timeout and now prints the matcher's error before the timeout line.
Comment thread src/jsc/event_loop.rs Resolved
Comment thread src/runtime/test_runner/Execution.rs Resolved
Comment thread src/runtime/test_runner/bun_test.rs Resolved
Comment thread src/runtime/test_runner/bun_test.rs Resolved
Comment thread src/runtime/test_runner/bun_test.rs Resolved
Comment thread src/runtime/test_runner/bun_test.rs Resolved
Comment thread src/runtime/test_runner/expect.rs Resolved
Comment thread src/runtime/test_runner/bun_test.rs Resolved
@robobun

robobun commented Aug 15, 2026

Copy link
Copy Markdown
Collaborator Author

Cross-references for triage, since this overlaps with two other open PRs:

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review found no issues

No high-confidence issues detected in this change.

…t gave up

A wait reached from a continuation no longer steps the runner past the entry from inside the
matcher. It marks the entry's sequence timed out at the deadline (what evaluate_timeout does
when the runner re-steps) and throws; the completion that produces then reports the entry as
timed out, attributed to it, so a test that passes on retry no longer exits 1 for the first
attempt's late error. The timeout timer leaves the one running entry's deadline to such a wait
(Execution::defers_timeout_to_matcher_wait) and comes back 1ms later for the rest.

A wait whose entry the runner is already past (it was never awaited, and its test completed
without it) waits on while the file runs, since its promise may still settle, and gives up when
the file is done. The matcher's error now says whether the test timed out or ended.

wait_for_promise_or_give_up asks the predicate after each tick, before the poll that may park:
what the tick ran (the runner finishing the file) may be what ends the wait.

Tests: a test that passes on retry after a blocked attempt exits 0; leaked waits settle while
the file runs or end with it; the continuation fixtures are attributed to their test.
Comment thread src/jsc/event_loop.rs Resolved
Comment thread src/runtime/test_runner/Execution.rs Resolved
Comment thread src/runtime/test_runner/Execution.rs Resolved
Comment thread src/runtime/test_runner/bun_test.rs Resolved
Comment thread src/runtime/test_runner/bun_test.rs Resolved
Comment thread src/runtime/test_runner/bun_test.rs Resolved
Comment thread src/runtime/test_runner/bun_test.rs Resolved
Comment thread src/runtime/test_runner/bun_test.rs Resolved
Comment thread src/runtime/test_runner/bun_test.rs Resolved
Comment thread src/runtime/test_runner/bun_test.rs Resolved
Comment thread src/runtime/test_runner/bun_test.rs Resolved
Comment thread src/runtime/test_runner/bun_test.rs Resolved
Comment thread src/runtime/test_runner/expect.rs Resolved
… by the clock

The timer callback and the runner step sample the clock separately. On
macOS the timer can fire inside the gap: the defer check says the entry
is not due yet, then the step's fresh clock says it is and steps past
the entry, so the wait reports the test as ended and the matcher error
lands between tests. The wait now registers which entry (or group) it
claims, and the timer defers exactly when the claim still names the
entry that is due. A stale claim stops deferring, so a wait the runner
moved past can no longer hold the timer off the current entry.
Comment thread src/runtime/test_runner/Execution.rs Resolved
Comment thread src/runtime/test_runner/Execution.rs Resolved
Comment thread src/runtime/test_runner/bun_test.rs Resolved
Comment thread src/runtime/test_runner/bun_test.rs Resolved
Comment thread src/runtime/test_runner/bun_test.rs Resolved
Comment thread src/runtime/test_runner/Execution.rs Resolved

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review found no issues

No high-confidence issues detected in this change.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review found no issues

No high-confidence issues detected in this change.

@robobun

robobun commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator Author

Findings from a second self-review pass, checked against daabd8b on a Linux debug build. None is a regression against main (main hangs in all three), so they are follow-ups for this branch or a later PR.

  1. A leaked wait in a concurrent group fails a later test. GroupOver gives up with TestEnded as soon as its group is over, while the sequential Continuation arm waits for the file to end. For a wait the test never awaited whose promise settles after the group but while the file still runs, the error is charged to whatever test runs then:

    const later = () => new Promise(resolve => setTimeout(resolve, 300, "settled"));
    test.concurrent("leaks a wait that settles after its group ends", () => {
      setImmediate(async () => { await expect(later()).resolves.toBe("settled"); });
    });
    test.concurrent("sibling", async () => { await Bun.sleep(50); });
    test("a later test keeps the file running", async () => { await Bun.sleep(400); });

    Released bun: 3 pass, exit 0. This branch: (fail) a later test keeps the file running, 2 pass 1 fail. Dropping the "group moved on" arm (keep waiting until Phase::Done, like Continuation) makes both shapes follow the same rule. No fixture covers the arm today.

  2. Retry inside a concurrent group still exits 1. The deferral only applies when the claim names the one executing sequence. With a sibling still running at the deadline the timer steps past the attempt, and the orphaned wait's error lands as Unhandled error between tests after the retry passed:

    let attempt = 0;
    test.concurrent("passes on retry", async () => {
      attempt++;
      await soon();
      await expect(attempt === 1 ? never() : soon()).resolves.toBe("settled");
    }, { retry: 1, timeout: 200 });
    test.concurrent("sibling still running at the deadline", async () => { await Bun.sleep(400); });

    This branch: 3 pass, 0 fail, 1 error, exit 1. Deferring for a group claim whenever an executing entry of that group is due (re-arming for the next deadline, or 1 ms) would let the wait give up once the sibling finishes and fail attempt 1 itself. The cost is that the timeout is reported when the sibling finishes, not at the deadline.

  3. Two more nested waits a test body can reach are still unbounded: src/runtime/api/JSBundler.rs:558 (Bun.build with a plugin whose setup() returns a promise) and src/runtime/bake/bake_body.rs:379 (Bun.serve({ app: { framework: "react", plugins } })). Both hang a test until killed today and on this branch:

    test("Bun.build plugin setup that never settles", async () => {
      await Bun.build({ entrypoints: [import.meta.path], plugins: [{ name: "p", setup() { return new Promise(() => {}); } }] });
    }, { timeout: 300 });

    They have the same shape as the matcher sites (set_handled, wait_for_promise, unwrap with Pending unreachable). Moving Expect::wait_for_promise into bun_test as a pub(crate) helper (and dropping "matcher" from blocking_matcher_waits, matcher_wait_claim, defers_timeout_to_matcher_wait) would let both call it and throw Plugin setup() returned a promise that was still pending when the test timed out. Outside a test file the helper waits without bound as today.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant