From b033bc6ed763550ad0b823fab7e0d0af0a9bfbbe Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 29 Jun 2026 16:33:11 +0000 Subject: [PATCH 1/6] test runner: fail dependent tests when a hook's done() receives an error --- src/runtime/test_runner/bun_test.rs | 50 +++++++++++----- .../test-error-code-done-callback.test.ts | 60 +++++++++++++++++-- .../fixtures/06-failing-before-hook.js | 10 ++++ test/js/node/test_runner/node-test.test.ts | 11 ++++ 4 files changed, 113 insertions(+), 18 deletions(-) create mode 100644 test/js/node/test_runner/fixtures/06-failing-before-hook.js diff --git a/src/runtime/test_runner/bun_test.rs b/src/runtime/test_runner/bun_test.rs index a59d41f484d2..577dc34916bd 100644 --- a/src/runtime/test_runner/bun_test.rs +++ b/src/runtime/test_runner/bun_test.rs @@ -815,31 +815,53 @@ impl BunTest { let [value] = callframe.arguments_as_array::<1>(); let was_error = !value.is_empty_or_undefined_or_null(); + // A second done() is a no-op, matching Bun 1.2.20. + // In Jest it is "Expected done to be called once, but it was called multiple times." + // Vitest does not support done callbacks. // SAFETY: `this` is the live `*mut DoneCallback` returned by `from_js`; // single-threaded JS VM, GC keeps the wrapper alive for the call frame. - if unsafe { (*this).called } { - // in Bun 1.2.20, this is a no-op - // in Jest, this is "Expected done to be called once, but it was called multiple times." - // Vitest does not support done callbacks - } else { - // error is only reported for the first done() call - if was_error { - let _ = global_this.bun_vm().as_mut().uncaught_exception(global_this, value, false); - } - } + let first_call = !unsafe { (*this).called }; // SAFETY: see above — `this` is a live `*mut DoneCallback`. let ref_in = unsafe { (*this).called = true; (*this).r#ref.take() }; - let Some(ref_in) = ref_in else { - return Ok(JSValue::UNDEFINED); - }; // `this.ref` was already taken above. // RefPtr currently has NO Drop impl, so decrement the // intrusive count explicitly at scope exit. Without this the // paired promise then/catch path never sees has_one_ref()==true and the RefData leaks. - let ref_in = scopeguard::guard(ref_in, |r: RefDataPtr| r.deref()); + let ref_in = ref_in.map(|r| scopeguard::guard(r, |r: RefDataPtr| r.deref())); + + // error is only reported for the first done() call + if first_call && was_error { + // `done(error)` is a failure of the entry that owns this callback, + // not a stray unhandled exception. Attribute it there (like the + // promise-catch path) so a failing hook fails its dependent tests. + let strong = match ref_in.as_ref() { + Some(r) => r.buntest_weak.upgrade(), + // done() ran synchronously inside the callback (`run_test_callback` + // attaches the ref after it returns), so the owner is the active entry. + None => clone_active_strong(), + }; + match strong { + Some(strong) => { + let phase = match ref_in.as_ref() { + Some(r) => r.phase.clone(), + None => strong.get().get_current_state_data(), + }; + // `strong.get()` is re-derived; the `get_current_state_data` borrow ended above. + strong.get().on_uncaught_exception(global_this, Some(value), false, &phase); + } + None => { + // No live BunTest owns this callback; report it generically. + let _ = global_this.bun_vm().as_mut().uncaught_exception(global_this, value, false); + } + } + } + + let Some(ref_in) = ref_in else { + return Ok(JSValue::UNDEFINED); + }; // dupe the ref and enqueue a task to call the done callback. // this makes it so if you do something else after calling done(), the next test doesn't start running until the next tick. diff --git a/test/js/bun/test/test-error-code-done-callback.test.ts b/test/js/bun/test/test-error-code-done-callback.test.ts index 610c4d85e7ec..af3364559ae6 100644 --- a/test/js/bun/test/test-error-code-done-callback.test.ts +++ b/test/js/bun/test/test-error-code-done-callback.test.ts @@ -1,5 +1,5 @@ -import { expect, test } from "bun:test"; -import { bunEnv, bunExe } from "harness"; +import { describe, expect, test } from "bun:test"; +import { bunEnv, bunExe, tempDir } from "harness"; import path from "path"; test("verify we print error messages passed to done callbacks", () => { @@ -80,7 +80,6 @@ test("verify we print error messages passed to done callbacks", () => { ^ error: you should see this(async) at (/test-error-done-callback-fixture.ts:42:14) - at (/test-error-done-callback-fixture.ts:37:3) (fail) error done callback (async) 43 | }); 44 | }); @@ -111,7 +110,6 @@ test("verify we print error messages passed to done callbacks", () => { ^ error: you should see this(async, nextTick) at (/test-error-done-callback-fixture.ts:60:14) - at (/test-error-done-callback-fixture.ts:54:5) (fail) error done callback (async, nextTick) 62 | }); 63 | @@ -140,3 +138,57 @@ test("verify we print error messages passed to done callbacks", () => { " `); }); + +// A `done(error)` in a lifecycle hook must fail the hook's dependent tests, +// exactly like a synchronous throw in the same hook does. It used to be +// surfaced as an "Unhandled error between tests" while every dependent test +// was still counted as a pass. `node:test` routes every hook through the +// done-callback form, so that module's `before()` was affected too. +describe.concurrent("done(error) in a lifecycle hook", () => { + // One describe block containing 2 tests; expected counts match the + // synchronous-throw variant of each hook. + const expected = { + beforeAll: { pass: 0, fail: 1 }, + beforeEach: { pass: 0, fail: 2 }, + afterEach: { pass: 0, fail: 2 }, + afterAll: { pass: 2, fail: 1 }, + } as const; + + test.each(Object.keys(expected) as (keyof typeof expected)[])( + "%s(done => done(err)) matches the synchronous-throw counts", + async hook => { + using dir = tempDir(`done-error-${hook}`, { + "hook.test.ts": ` + import { describe, ${hook}, test } from "bun:test"; + describe("suite", () => { + ${hook}(done => { done(new Error("hook failed")); }); + test("t1", () => {}); + test("t2", () => {}); + }); + `, + }); + await using proc = Bun.spawn({ + cmd: [bunExe(), "test", "./hook.test.ts"], + 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 out = stdout + stderr; + // The hook's error is still reported, attributed to the failing entry. + expect(out).toContain("error: hook failed"); + expect(summaryCounts(out)).toEqual(expected[hook]); + expect(exitCode).toBe(1); + }, + ); +}); + +/** `" 2 pass\n 0 fail\n 1 error\n"` -> `{ pass: 2, fail: 0, error: 1 }` */ +function summaryCounts(out: string): Record { + const counts: Record = {}; + for (const [, n, label] of out.matchAll(/^ (\d+) (pass|fail|skip|todo|error)s?$/gm)) { + counts[label] = Number(n); + } + return counts; +} diff --git a/test/js/node/test_runner/fixtures/06-failing-before-hook.js b/test/js/node/test_runner/fixtures/06-failing-before-hook.js new file mode 100644 index 000000000000..95c6dcf6a7f3 --- /dev/null +++ b/test/js/node/test_runner/fixtures/06-failing-before-hook.js @@ -0,0 +1,10 @@ +const { describe, before, test } = require("node:test"); + +describe("db suite", () => { + before(() => { + throw new Error("DB connection failed"); + }); + test("reads row", () => { + // would touch the DB; must not be reported as a pass + }); +}); diff --git a/test/js/node/test_runner/node-test.test.ts b/test/js/node/test_runner/node-test.test.ts index 3e422ae5ae0d..18bab1b19572 100644 --- a/test/js/node/test_runner/node-test.test.ts +++ b/test/js/node/test_runner/node-test.test.ts @@ -52,6 +52,17 @@ describe("node:test", () => { stderr: expect.stringContaining("0 fail"), }); }); + + test("should not report a test as passing when its before() hook threw", async () => { + const { exitCode, stderr } = await runTests(["06-failing-before-hook.js"]); + // node fails every test under a suite whose before() hook failed. Bun + // used to report the test as a pass and surface the hook error only as + // an "Unhandled error between tests". + expect(stderr).toContain("error: DB connection failed"); + expect(stderr).toContain(" 0 pass\n 1 fail\n"); + expect(stderr).not.toContain("Unhandled error between tests"); + expect(exitCode).toBe(1); + }); }); async function runTests(filenames: string[]) { From 10fe9b173a26e9b7bfbd78da56682c3ad435beb3 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Mon, 29 Jun 2026 16:36:16 +0000 Subject: [PATCH 2/6] [autofix.ci] apply automated fixes --- docs/guides/util/base64.mdx | 9 +++++---- docs/runtime/web-apis.mdx | 36 ++++++++++++++++++------------------ 2 files changed, 23 insertions(+), 22 deletions(-) diff --git a/docs/guides/util/base64.mdx b/docs/guides/util/base64.mdx index 8a976ad02474..5a088240ed1b 100644 --- a/docs/guides/util/base64.mdx +++ b/docs/guides/util/base64.mdx @@ -40,10 +40,11 @@ const text = bytes.toString("utf8"); The older [`btoa()`](https://developer.mozilla.org/en-US/docs/Web/API/Window/btoa) and [`atob()`](https://developer.mozilla.org/en-US/docs/Web/API/Window/atob) APIs are still available for compatibility, but they operate on binary strings instead of byte arrays. Avoid them in new code, especially when handling arbitrary binary data or non-ASCII text. - ```ts - const encoded = btoa("bun"); // => "YnVu" - const decoded = atob(encoded); // => "bun" - ``` +```ts +const encoded = btoa("bun"); // => "YnVu" +const decoded = atob(encoded); // => "bun" +``` + --- diff --git a/docs/runtime/web-apis.mdx b/docs/runtime/web-apis.mdx index 97e781ae04d9..7c07a493e421 100644 --- a/docs/runtime/web-apis.mdx +++ b/docs/runtime/web-apis.mdx @@ -8,22 +8,22 @@ Some Web APIs aren't relevant in the context of a server-first runtime like Bun, The following Web APIs are partially or completely supported. -| Category | APIs | -| --------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| HTTP | [`fetch`](https://developer.mozilla.org/en-US/docs/Web/API/fetch), [`Response`](https://developer.mozilla.org/en-US/docs/Web/API/Response), [`Request`](https://developer.mozilla.org/en-US/docs/Web/API/Request), [`Headers`](https://developer.mozilla.org/en-US/docs/Web/API/Headers), [`AbortController`](https://developer.mozilla.org/en-US/docs/Web/API/AbortController), [`AbortSignal`](https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal) | -| URLs | [`URL`](https://developer.mozilla.org/en-US/docs/Web/API/URL), [`URLSearchParams`](https://developer.mozilla.org/en-US/docs/Web/API/URLSearchParams) | -| Web Workers | [`Worker`](https://developer.mozilla.org/en-US/docs/Web/API/Worker), [`self.postMessage`](https://developer.mozilla.org/en-US/docs/Web/API/DedicatedWorkerGlobalScope/postMessage), [`structuredClone`](https://developer.mozilla.org/en-US/docs/Web/API/structuredClone), [`MessagePort`](https://developer.mozilla.org/en-US/docs/Web/API/MessagePort), [`MessageChannel`](https://developer.mozilla.org/en-US/docs/Web/API/MessageChannel), [`BroadcastChannel`](https://developer.mozilla.org/en-US/docs/Web/API/BroadcastChannel) | -| Streams | [`ReadableStream`](https://developer.mozilla.org/en-US/docs/Web/API/ReadableStream), [`WritableStream`](https://developer.mozilla.org/en-US/docs/Web/API/WritableStream), [`TransformStream`](https://developer.mozilla.org/en-US/docs/Web/API/TransformStream), [`ByteLengthQueuingStrategy`](https://developer.mozilla.org/en-US/docs/Web/API/ByteLengthQueuingStrategy), [`CountQueuingStrategy`](https://developer.mozilla.org/en-US/docs/Web/API/CountQueuingStrategy) and associated classes | -| Blob | [`Blob`](https://developer.mozilla.org/en-US/docs/Web/API/Blob) | -| WebSockets | [`WebSocket`](https://developer.mozilla.org/en-US/docs/Web/API/WebSocket) | +| Category | APIs | +| --------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| HTTP | [`fetch`](https://developer.mozilla.org/en-US/docs/Web/API/fetch), [`Response`](https://developer.mozilla.org/en-US/docs/Web/API/Response), [`Request`](https://developer.mozilla.org/en-US/docs/Web/API/Request), [`Headers`](https://developer.mozilla.org/en-US/docs/Web/API/Headers), [`AbortController`](https://developer.mozilla.org/en-US/docs/Web/API/AbortController), [`AbortSignal`](https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal) | +| URLs | [`URL`](https://developer.mozilla.org/en-US/docs/Web/API/URL), [`URLSearchParams`](https://developer.mozilla.org/en-US/docs/Web/API/URLSearchParams) | +| Web Workers | [`Worker`](https://developer.mozilla.org/en-US/docs/Web/API/Worker), [`self.postMessage`](https://developer.mozilla.org/en-US/docs/Web/API/DedicatedWorkerGlobalScope/postMessage), [`structuredClone`](https://developer.mozilla.org/en-US/docs/Web/API/structuredClone), [`MessagePort`](https://developer.mozilla.org/en-US/docs/Web/API/MessagePort), [`MessageChannel`](https://developer.mozilla.org/en-US/docs/Web/API/MessageChannel), [`BroadcastChannel`](https://developer.mozilla.org/en-US/docs/Web/API/BroadcastChannel) | +| Streams | [`ReadableStream`](https://developer.mozilla.org/en-US/docs/Web/API/ReadableStream), [`WritableStream`](https://developer.mozilla.org/en-US/docs/Web/API/WritableStream), [`TransformStream`](https://developer.mozilla.org/en-US/docs/Web/API/TransformStream), [`ByteLengthQueuingStrategy`](https://developer.mozilla.org/en-US/docs/Web/API/ByteLengthQueuingStrategy), [`CountQueuingStrategy`](https://developer.mozilla.org/en-US/docs/Web/API/CountQueuingStrategy) and associated classes | +| Blob | [`Blob`](https://developer.mozilla.org/en-US/docs/Web/API/Blob) | +| WebSockets | [`WebSocket`](https://developer.mozilla.org/en-US/docs/Web/API/WebSocket) | | Encoding and decoding | [`Uint8Array`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Uint8Array), [`Uint8Array.prototype.toBase64()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Uint8Array/toBase64), [`Uint8Array.fromBase64()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Uint8Array/fromBase64), [`TextEncoder`](https://developer.mozilla.org/en-US/docs/Web/API/TextEncoder), [`TextDecoder`](https://developer.mozilla.org/en-US/docs/Web/API/TextDecoder), [`atob`](https://developer.mozilla.org/en-US/docs/Web/API/atob), [`btoa`](https://developer.mozilla.org/en-US/docs/Web/API/btoa) | -| JSON | [`JSON`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON) | -| Timeouts | [`setTimeout`](https://developer.mozilla.org/en-US/docs/Web/API/setTimeout), [`clearTimeout`](https://developer.mozilla.org/en-US/docs/Web/API/clearTimeout) | -| Intervals | [`setInterval`](https://developer.mozilla.org/en-US/docs/Web/API/setInterval), [`clearInterval`](https://developer.mozilla.org/en-US/docs/Web/API/clearInterval) | -| Crypto | [`crypto`](https://developer.mozilla.org/en-US/docs/Web/API/Crypto), [`SubtleCrypto`](https://developer.mozilla.org/en-US/docs/Web/API/SubtleCrypto), [`CryptoKey`](https://developer.mozilla.org/en-US/docs/Web/API/CryptoKey) | -| Debugging | [`console`](https://developer.mozilla.org/en-US/docs/Web/API/console), [`performance`](https://developer.mozilla.org/en-US/docs/Web/API/Performance) | -| Microtasks | [`queueMicrotask`](https://developer.mozilla.org/en-US/docs/Web/API/queueMicrotask) | -| Errors | [`reportError`](https://developer.mozilla.org/en-US/docs/Web/API/reportError) | -| User interaction | [`alert`](https://developer.mozilla.org/en-US/docs/Web/API/Window/alert), [`confirm`](https://developer.mozilla.org/en-US/docs/Web/API/Window/confirm), [`prompt`](https://developer.mozilla.org/en-US/docs/Web/API/Window/prompt) (intended for interactive CLIs) | -| Realms | [`ShadowRealm`](https://github.com/tc39/proposal-shadowrealm) | -| Events | [`EventTarget`](https://developer.mozilla.org/en-US/docs/Web/API/EventTarget), [`Event`](https://developer.mozilla.org/en-US/docs/Web/API/Event), [`ErrorEvent`](https://developer.mozilla.org/en-US/docs/Web/API/ErrorEvent), [`CloseEvent`](https://developer.mozilla.org/en-US/docs/Web/API/CloseEvent), [`MessageEvent`](https://developer.mozilla.org/en-US/docs/Web/API/MessageEvent) | +| JSON | [`JSON`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON) | +| Timeouts | [`setTimeout`](https://developer.mozilla.org/en-US/docs/Web/API/setTimeout), [`clearTimeout`](https://developer.mozilla.org/en-US/docs/Web/API/clearTimeout) | +| Intervals | [`setInterval`](https://developer.mozilla.org/en-US/docs/Web/API/setInterval), [`clearInterval`](https://developer.mozilla.org/en-US/docs/Web/API/clearInterval) | +| Crypto | [`crypto`](https://developer.mozilla.org/en-US/docs/Web/API/Crypto), [`SubtleCrypto`](https://developer.mozilla.org/en-US/docs/Web/API/SubtleCrypto), [`CryptoKey`](https://developer.mozilla.org/en-US/docs/Web/API/CryptoKey) | +| Debugging | [`console`](https://developer.mozilla.org/en-US/docs/Web/API/console), [`performance`](https://developer.mozilla.org/en-US/docs/Web/API/Performance) | +| Microtasks | [`queueMicrotask`](https://developer.mozilla.org/en-US/docs/Web/API/queueMicrotask) | +| Errors | [`reportError`](https://developer.mozilla.org/en-US/docs/Web/API/reportError) | +| User interaction | [`alert`](https://developer.mozilla.org/en-US/docs/Web/API/Window/alert), [`confirm`](https://developer.mozilla.org/en-US/docs/Web/API/Window/confirm), [`prompt`](https://developer.mozilla.org/en-US/docs/Web/API/Window/prompt) (intended for interactive CLIs) | +| Realms | [`ShadowRealm`](https://github.com/tc39/proposal-shadowrealm) | +| Events | [`EventTarget`](https://developer.mozilla.org/en-US/docs/Web/API/EventTarget), [`Event`](https://developer.mozilla.org/en-US/docs/Web/API/Event), [`ErrorEvent`](https://developer.mozilla.org/en-US/docs/Web/API/ErrorEvent), [`CloseEvent`](https://developer.mozilla.org/en-US/docs/Web/API/CloseEvent), [`MessageEvent`](https://developer.mozilla.org/en-US/docs/Web/API/MessageEvent) | From d71344b8bd3d2e21974b964a98987bc9521a2bb6 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 29 Jun 2026 16:57:20 +0000 Subject: [PATCH 3/6] ci: retrigger From c3ae127a40db53cae9c3e384aeb65a4ca3bf0a3b Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 29 Jun 2026 17:15:58 +0000 Subject: [PATCH 4/6] test runner: do not blame an orphaned done(err) on the active entry --- src/runtime/test_runner/bun_test.rs | 12 ++++--- .../test-error-code-done-callback.test.ts | 35 +++++++++++++++++++ 2 files changed, 43 insertions(+), 4 deletions(-) diff --git a/src/runtime/test_runner/bun_test.rs b/src/runtime/test_runner/bun_test.rs index 577dc34916bd..b0916c6424fb 100644 --- a/src/runtime/test_runner/bun_test.rs +++ b/src/runtime/test_runner/bun_test.rs @@ -839,9 +839,12 @@ impl BunTest { // promise-catch path) so a failing hook fails its dependent tests. let strong = match ref_in.as_ref() { Some(r) => r.buntest_weak.upgrade(), - // done() ran synchronously inside the callback (`run_test_callback` - // attaches the ref after it returns), so the owner is the active entry. - None => clone_active_strong(), + // No ref means `run_test_callback` has not attached one yet: done() + // ran synchronously inside the callback, or the body threw and + // orphaned it. Only while synchronously inside the runner's step + // (`in_run_loop`) is the active entry the owner; an orphan firing + // later must not be blamed on whatever entry is active by then. + None => clone_active_strong().filter(|s| s.get().in_run_loop), }; match strong { Some(strong) => { @@ -853,7 +856,8 @@ impl BunTest { strong.get().on_uncaught_exception(global_this, Some(value), false, &phase); } None => { - // No live BunTest owns this callback; report it generically. + // Orphaned, or no live BunTest: report it generically, keeping + // the hook-demotion guard in `jest::on_unhandled_rejection`. let _ = global_this.bun_vm().as_mut().uncaught_exception(global_this, value, false); } } diff --git a/test/js/bun/test/test-error-code-done-callback.test.ts b/test/js/bun/test/test-error-code-done-callback.test.ts index af3364559ae6..43a96f45f7ff 100644 --- a/test/js/bun/test/test-error-code-done-callback.test.ts +++ b/test/js/bun/test/test-error-code-done-callback.test.ts @@ -184,6 +184,41 @@ describe.concurrent("done(error) in a lifecycle hook", () => { ); }); +// A test body that throws after handing `done` to a timer leaves an orphaned +// done callback: the throw returns from the runner before its ref is attached. +// When that done(err) fires later, it must stay an "Unhandled error between +// tests" and never be attributed to whatever entry happens to be active then. +test.concurrent("a late done(err) from a test whose body threw does not fail an unrelated test", async () => { + using dir = tempDir("orphaned-done", { + "orphan.test.ts": ` + import { test, describe, beforeEach } from "bun:test"; + const { promise: orphanFired, resolve: markOrphanFired } = Promise.withResolvers(); + test("a", done => { + setTimeout(() => { done(new Error("late orphan")); markOrphanFired(); }, 5); + throw new Error("immediate"); + }); + describe("suite", () => { + // The orphan's done(err) lands while this hook is the active entry. + beforeEach(done => { orphanFired.then(() => done()); }); + test("b still passes", () => {}); + }); + `, + }); + await using proc = Bun.spawn({ + cmd: [bunExe(), "test", "./orphan.test.ts"], + 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 out = stdout + stderr; + expect(out).toContain("(pass) suite > b still passes"); + expect(out).toContain("Unhandled error between tests"); + expect(summaryCounts(out)).toEqual({ pass: 1, fail: 1, error: 1 }); + expect(exitCode).toBe(1); +}); + /** `" 2 pass\n 0 fail\n 1 error\n"` -> `{ pass: 2, fail: 0, error: 1 }` */ function summaryCounts(out: string): Record { const counts: Record = {}; From 32770fdf62ea5b3ba75c17d2b962910e1f160e20 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 29 Jun 2026 18:16:33 +0000 Subject: [PATCH 5/6] test runner: mark a done callback orphaned when its body throws --- src/runtime/test_runner/DoneCallback.rs | 4 ++ src/runtime/test_runner/bun_test.rs | 23 ++++--- .../test-error-code-done-callback.test.ts | 69 +++++++++++-------- 3 files changed, 58 insertions(+), 38 deletions(-) diff --git a/src/runtime/test_runner/DoneCallback.rs b/src/runtime/test_runner/DoneCallback.rs index 10cb136bd201..ea5cd8517cf7 100644 --- a/src/runtime/test_runner/DoneCallback.rs +++ b/src/runtime/test_runner/DoneCallback.rs @@ -8,6 +8,9 @@ pub struct DoneCallback { /// Some = not called yet. None = done already called, no-op. pub r#ref: Option, pub called: bool, // = false + /// The owning body threw before `run_test_callback` could attach `ref`, + /// so a later done(error) from this callback has no attributable owner. + pub orphaned: bool, // = false } impl DoneCallback { @@ -32,6 +35,7 @@ impl DoneCallback { let done_callback = DoneCallback { r#ref: None, called: false, + orphaned: false, }; // `JsClass::to_js` boxes `self` and hands the raw pointer to the JS diff --git a/src/runtime/test_runner/bun_test.rs b/src/runtime/test_runner/bun_test.rs index b0916c6424fb..dedc2a21991b 100644 --- a/src/runtime/test_runner/bun_test.rs +++ b/src/runtime/test_runner/bun_test.rs @@ -839,12 +839,14 @@ impl BunTest { // promise-catch path) so a failing hook fails its dependent tests. let strong = match ref_in.as_ref() { Some(r) => r.buntest_weak.upgrade(), - // No ref means `run_test_callback` has not attached one yet: done() - // ran synchronously inside the callback, or the body threw and - // orphaned it. Only while synchronously inside the runner's step - // (`in_run_loop`) is the active entry the owner; an orphan firing - // later must not be blamed on whatever entry is active by then. - None => clone_active_strong().filter(|s| s.get().in_run_loop), + // The body threw before a ref could be attached; whenever and + // however this done() fires, it has no attributable owner. + // SAFETY: `this` is the live `*mut DoneCallback`; see above. + None if unsafe { (*this).orphaned } => None, + // Not orphaned and no ref: `run_test_callback` attaches the ref + // only after the callback returns, so done() must be running + // synchronously inside it and the active entry is the owner. + None => clone_active_strong(), }; match strong { Some(strong) => { @@ -1224,11 +1226,16 @@ impl BunTest { // second `RefDataPtr` here would over-count and the done-callback path // would never observe `has_one_ref()`. let mut dcb_ref: Option> = None; - if !done_callback.is_empty() && !result.is_empty() { + if !done_callback.is_empty() { if let Some(dcb_data) = DoneCallback::from_js(done_callback) { // SAFETY: `dcb_data` is the live `*mut DoneCallback` from `from_js`; // single-threaded JS VM, GC roots `done_callback` for this frame. - if unsafe { (*dcb_data).called } { + if result.is_empty() { + // The body threw, so no ref is attached. A later done(error) + // from this callback (the body may have handed it to a timer + // or microtask before throwing) has no attributable owner. + unsafe { (*dcb_data).orphaned = true }; + } else if unsafe { (*dcb_data).called } { // done callback already called or the callback errored; add result immediately } else { let r = Self::ref_(this_strong, cfg_data.clone()); diff --git a/test/js/bun/test/test-error-code-done-callback.test.ts b/test/js/bun/test/test-error-code-done-callback.test.ts index 43a96f45f7ff..c968b1618a4d 100644 --- a/test/js/bun/test/test-error-code-done-callback.test.ts +++ b/test/js/bun/test/test-error-code-done-callback.test.ts @@ -184,39 +184,48 @@ describe.concurrent("done(error) in a lifecycle hook", () => { ); }); -// A test body that throws after handing `done` to a timer leaves an orphaned -// done callback: the throw returns from the runner before its ref is attached. +// A test body that throws after handing `done` away leaves an orphaned done +// callback: the throw returns from the runner before its ref is attached. // When that done(err) fires later, it must stay an "Unhandled error between // tests" and never be attributed to whatever entry happens to be active then. -test.concurrent("a late done(err) from a test whose body threw does not fail an unrelated test", async () => { - using dir = tempDir("orphaned-done", { - "orphan.test.ts": ` - import { test, describe, beforeEach } from "bun:test"; - const { promise: orphanFired, resolve: markOrphanFired } = Promise.withResolvers(); - test("a", done => { - setTimeout(() => { done(new Error("late orphan")); markOrphanFired(); }, 5); - throw new Error("immediate"); - }); - describe("suite", () => { - // The orphan's done(err) lands while this hook is the active entry. - beforeEach(done => { orphanFired.then(() => done()); }); - test("b still passes", () => {}); - }); - `, - }); - await using proc = Bun.spawn({ - cmd: [bunExe(), "test", "./orphan.test.ts"], - env: bunEnv, - cwd: String(dir), - stdout: "pipe", - stderr: "pipe", +// A macrotask orphan fires from a later event-loop turn; a microtask orphan +// is drained inside the NEXT entry's callback (the throw skips the thrower's +// own microtask drain), so both schedulings must be covered. +describe.concurrent("a late done(err) from a test whose body threw", () => { + test.each([ + ["a setTimeout", "setTimeout(fire, 5)"], + ["a microtask", "Promise.resolve().then(fire)"], + ])("scheduled via %s does not fail an unrelated test", async (_name, schedule) => { + using dir = tempDir("orphaned-done", { + "orphan.test.ts": ` + import { test, describe, beforeEach } from "bun:test"; + const { promise: orphanFired, resolve: markOrphanFired } = Promise.withResolvers(); + test("a", done => { + const fire = () => { done(new Error("late orphan")); markOrphanFired(); }; + ${schedule}; + throw new Error("immediate"); + }); + describe("suite", () => { + // The orphan's done(err) lands while this hook is the active entry. + beforeEach(done => { orphanFired.then(() => done()); }); + test("b still passes", () => {}); + }); + `, + }); + await using proc = Bun.spawn({ + cmd: [bunExe(), "test", "./orphan.test.ts"], + 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 out = stdout + stderr; + expect(out).toContain("(pass) suite > b still passes"); + expect(out).toContain("Unhandled error between tests"); + expect(summaryCounts(out)).toEqual({ pass: 1, fail: 1, error: 1 }); + expect(exitCode).toBe(1); }); - const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); - const out = stdout + stderr; - expect(out).toContain("(pass) suite > b still passes"); - expect(out).toContain("Unhandled error between tests"); - expect(summaryCounts(out)).toEqual({ pass: 1, fail: 1, error: 1 }); - expect(exitCode).toBe(1); }); /** `" 2 pass\n 0 fail\n 1 error\n"` -> `{ pass: 2, fail: 0, error: 1 }` */ From 21e6a3f1911f36f59027e15e0af45b6ad7ed4d08 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 29 Jun 2026 18:25:44 +0000 Subject: [PATCH 6/6] test: drop the wall clock from the orphaned-done case and trim comments --- .../test/test-error-code-done-callback.test.ts | 18 ++++++------------ test/js/node/test_runner/node-test.test.ts | 4 +--- 2 files changed, 7 insertions(+), 15 deletions(-) diff --git a/test/js/bun/test/test-error-code-done-callback.test.ts b/test/js/bun/test/test-error-code-done-callback.test.ts index c968b1618a4d..e326f10f7411 100644 --- a/test/js/bun/test/test-error-code-done-callback.test.ts +++ b/test/js/bun/test/test-error-code-done-callback.test.ts @@ -140,10 +140,8 @@ test("verify we print error messages passed to done callbacks", () => { }); // A `done(error)` in a lifecycle hook must fail the hook's dependent tests, -// exactly like a synchronous throw in the same hook does. It used to be -// surfaced as an "Unhandled error between tests" while every dependent test -// was still counted as a pass. `node:test` routes every hook through the -// done-callback form, so that module's `before()` was affected too. +// exactly like a synchronous throw in the same hook does. `node:test` routes +// every hook through the done-callback form. describe.concurrent("done(error) in a lifecycle hook", () => { // One describe block containing 2 tests; expected counts match the // synchronous-throw variant of each hook. @@ -184,16 +182,12 @@ describe.concurrent("done(error) in a lifecycle hook", () => { ); }); -// A test body that throws after handing `done` away leaves an orphaned done -// callback: the throw returns from the runner before its ref is attached. -// When that done(err) fires later, it must stay an "Unhandled error between -// tests" and never be attributed to whatever entry happens to be active then. -// A macrotask orphan fires from a later event-loop turn; a microtask orphan -// is drained inside the NEXT entry's callback (the throw skips the thrower's -// own microtask drain), so both schedulings must be covered. +// A done callback orphaned by its body throwing must stay an "Unhandled error +// between tests" and never be blamed on whatever entry is active when it fires. +// A macrotask fires on a later turn; a microtask drains inside the next entry. describe.concurrent("a late done(err) from a test whose body threw", () => { test.each([ - ["a setTimeout", "setTimeout(fire, 5)"], + ["setImmediate", "setImmediate(fire)"], ["a microtask", "Promise.resolve().then(fire)"], ])("scheduled via %s does not fail an unrelated test", async (_name, schedule) => { using dir = tempDir("orphaned-done", { diff --git a/test/js/node/test_runner/node-test.test.ts b/test/js/node/test_runner/node-test.test.ts index 18bab1b19572..8d916c65246b 100644 --- a/test/js/node/test_runner/node-test.test.ts +++ b/test/js/node/test_runner/node-test.test.ts @@ -55,9 +55,7 @@ describe("node:test", () => { test("should not report a test as passing when its before() hook threw", async () => { const { exitCode, stderr } = await runTests(["06-failing-before-hook.js"]); - // node fails every test under a suite whose before() hook failed. Bun - // used to report the test as a pass and surface the hook error only as - // an "Unhandled error between tests". + // node fails every test under a suite whose before() hook failed. expect(stderr).toContain("error: DB connection failed"); expect(stderr).toContain(" 0 pass\n 1 fail\n"); expect(stderr).not.toContain("Unhandled error between tests");