bun:test: fixtures and around hooks wait on what await waits on, through the engine

src/jsc/JSPromise.rs

@@ -40,6 +40,10 @@ unsafe extern "C" {
         arg0: &JSGlobalObject,
         js_value1: JSValue,
     ) -> JSValue;
+    safe fn JSC__JSInternalPromise__resolvedPromise(
+        arg0: &JSGlobalObject,
+        js_value1: JSValue,
+    ) -> *mut JSPromise;
     safe fn JSC__JSPromise__wrap(
         arg0: &JSGlobalObject,
         ctx: *mut c_void,
@@ -263,6 +267,15 @@ impl JSPromise {
         JSC__JSPromise__resolvedPromiseValue(global, value)
     }
 
+    /// Spec `PromiseResolve(%Promise%, value)`: what `await value` waits on. A native promise comes back as is.
+    pub fn promise_resolve(global: &JSGlobalObject, value: JSValue) -> JsResult<&mut JSPromise> {
+        // Reads `value.constructor` and `value.then`, which can throw.
+        let promise = crate::call_check_slow(global, || {
+            JSC__JSInternalPromise__resolvedPromise(global, value)
+        })?;
+        Ok(JSPromise::opaque_mut(promise))
+    }
+
     /// Create a new rejected promise rejecting to a given value.
     ///
     /// Note: If you want the result as a `JSValue`, use `rejected_promise().to_js()` instead.

src/jsc/VirtualMachine.rs

@@ -3382,10 +3382,6 @@ unsafe extern "C" {
         global: &JSGlobalObject,
         argv1: JSValue,
     ) -> JSValue;
-    safe fn JSC__JSInternalPromise__resolvedPromise(
-        global: &JSGlobalObject,
-        value: JSValue,
-    ) -> *mut JSInternalPromise;
 }
 
 fn get_origin_timestamp() -> u64 {
@@ -3855,11 +3851,9 @@ impl VirtualMachine {
                                 if let Some(stored) = self.pending_internal_promise() {
                                     return Ok(stored);
                                 }
-                                // `Promise.resolve(ret)` reads `ret.constructor` / `ret.then`,
-                                // which may throw.
-                                jsc::call_check_slow(global_ref, || {
-                                    JSC__JSInternalPromise__resolvedPromise(global_ref, ret)
-                                })?
+                                core::ptr::from_mut(crate::JSPromise::promise_resolve(
+                                    global_ref, ret,
+                                )?)
                             }
                             Err(err) => {
                                 let rejected =

src/runtime/test_runner/bun_test.rs

@@ -1535,6 +1535,17 @@ impl BunTest {
         Ok(if promise.status() == PromiseStatus::Fulfilled { returned } else { promise.to_js() })
     }
 
+    /// What `await returned` waits on: a native promise as is, any other thenable through a new promise that follows its `then()`. `None`: `await` gives `returned` itself.
+    pub(crate) fn awaited_promise(global_this: &JSGlobalObject, returned: JSValue) -> JsResult<Option<JSValue>> {
+        if !returned.is_object() {
+            return Ok(None);
+        }
+        let promise = bun_jsc::JSPromise::promise_resolve(global_this, returned)?;
+        let awaited = promise.to_js();
+        // A new promise that is fulfilled at once: `returned` has no callable `then`.
+        Ok((awaited == returned || promise.status() != PromiseStatus::Fulfilled).then_some(awaited))
+    }
+
     /// Vitest runs a function returned by `beforeEach` / `beforeAll` after the matching after-hooks.
     fn adopt_teardown(&mut self, data: &RefDataValue, returned: JSValue) {
         if !returned.is_callable() {

src/runtime/test_runner/test_context.bind.ts

@@ -88,8 +88,15 @@ export const aroundSetUp = b.fn({
   args: { aroundHook: b.any, hook: b.any, context: b.any, suite: b.any },
   returns: b.any,
 });
-export const aroundOnFulfilled = b.fn(ofAround("onFulfilled", "on_fulfilled"));
-export const aroundOnRejected = b.fn({ ...ofAround("onRejected", "on_rejected"), args: { reason: b.any } });
+/** Reactions of the promise that the hook returned. The engine passes the `AroundHook` as their last argument. */
+export const aroundOnFulfilled = b.fn({
+  ...around("onFulfilled", "on_fulfilled"),
+  args: { value: b.any, aroundHook: b.any },
+});
+export const aroundOnRejected = b.fn({
+  ...around("onRejected", "on_rejected"),
+  args: { reason: b.any, aroundHook: b.any },
+});
 /** `runTest()` or `runSuite()` */
 export const aroundRun = b.fn({ ...ofAround("run", "run"), returns: b.any });
 /** The teardown entry's callback. */
@@ -103,17 +110,24 @@ export const adoptAsyncContext = b.fn({
   capability: "js",
 });
 
-/** What it is a bound function of: `this` is the `ActiveFixture`. */
-const ofFixture = (name: string, rustName: string) =>
+const fixture = (name: string, rustName: string) =>
   ({
     export: `Bun__ActiveFixture__${name}`,
     rustPath: `crate::test_runner::test_context_fixtures::${rustName}`,
-    passThis: true,
     ...js,
   }) as const;
+/** What it is a bound function of: `this` is the `ActiveFixture`. */
+const ofFixture = (name: string, rustName: string) => ({ ...fixture(name, rustName), passThis: true }) as const;
 export const fixtureSetUp = b.fn({ ...ofFixture("setUp", "set_up"), returns: b.any });
-export const fixtureOnFulfilled = b.fn({ ...ofFixture("onFulfilled", "on_fulfilled"), args: { returned: b.any } });
-export const fixtureOnRejected = b.fn({ ...ofFixture("onRejected", "on_rejected"), args: { reason: b.any } });
+/** Reactions of the promise that the fixture function returned. The engine passes the `ActiveFixture` as their last argument. */
+export const fixtureOnFulfilled = b.fn({
+  ...fixture("onFulfilled", "on_fulfilled"),
+  args: { returned: b.any, activeFixture: b.any },
+});
+export const fixtureOnRejected = b.fn({
+  ...fixture("onRejected", "on_rejected"),
+  args: { reason: b.any, activeFixture: b.any },
+});
 export const useFixture = b.fn({ ...ofFixture("useFixture", "use_fixture"), args: { value: b.any }, returns: b.any });
 export const fixtureOnCleanup = b.fn({ ...ofFixture("onCleanup", "on_cleanup"), args: { cleanup: b.any } });
 export const fixtureTearDown = b.fn({ ...ofFixture("tearDown", "tear_down"), returns: b.any });

src/runtime/test_runner/test_context_around.rs

@@ -15,7 +15,7 @@ use bun_jsc::{
 };
 
 use super::bun_test::{
-    Around, AroundPhase, BunTestPtr, BunTestPtrWeak, DescribeScope, RefDataValue,
+    Around, AroundPhase, BunTest, BunTestPtr, BunTestPtrWeak, DescribeScope, RefDataValue,
 };
 use super::execution::ExecutionSequence;
 use super::test_context::{self, Deferred, TestContext, bound};
@@ -169,35 +169,16 @@ impl AroundHook {
             generated::Bun__AroundHook__run,
         )?;
         let returned = hook.call(global, JSValue::UNDEFINED, &[run, context, suite])?;
-        match if returned.is_object() {
-            returned
-                .get(global, "then")?
-                .filter(|then| then.is_callable())
-        } else {
-            None
-        } {
-            Some(then) => {
-                let reactions = [
-                    bound(
-                        global,
-                        this_value,
-                        "",
-                        1,
-                        generated::Bun__AroundHook__onFulfilled,
-                    )?,
-                    bound(
-                        global,
-                        this_value,
-                        "",
-                        1,
-                        generated::Bun__AroundHook__onRejected,
-                    )?,
-                ];
-                then.call(global, returned, &reactions)?;
-                Ok(())
-            }
-            None => self.on_settled(this_value, global, Ok(())),
-        }
+        let Some(awaited) = BunTest::awaited_promise(global, returned)? else {
+            return self.on_settled(this_value, global, Ok(()));
+        };
+        awaited.then_with_value(
+            global,
+            this_value,
+            generated::Bun__AroundHook__onFulfilled,
+            generated::Bun__AroundHook__onRejected,
+        );
+        Ok(())
     }
 
     /// The hook returned or threw, or the promise it returned settled.
@@ -275,15 +256,19 @@ pub(crate) fn set_up(
     Ok(ready)
 }
 
-pub(crate) fn on_fulfilled(cx: &mut JsScope<'_>, this_value: JsLocal<'_>) -> JsResult<()> {
+pub(crate) fn on_fulfilled(
+    cx: &mut JsScope<'_>,
+    _value: JsLocal<'_>,
+    this_value: JsLocal<'_>,
+) -> JsResult<()> {
     let global = cx.global();
     AroundHook::of_call(global, this_value)?.on_settled(this_value.to_js(), global, Ok(()))
 }
 
 pub(crate) fn on_rejected(
     cx: &mut JsScope<'_>,
-    this_value: JsLocal<'_>,
     reason: JsLocal<'_>,
+    this_value: JsLocal<'_>,
 ) -> JsResult<()> {
     let global = cx.global();
     AroundHook::of_call(global, this_value)?.on_settled(

src/runtime/test_runner/test_context_fixtures.rs

@@ -697,20 +697,14 @@ impl ActiveFixture {
             Kind::Use | Kind::Value => bound(global, this_value, "use", generated::Bun__ActiveFixture__useFixture)?,
         };
         let returned = function.call(global, JSValue::UNDEFINED, &[context, second])?;
-        // The runner gets this promise again at teardown, and it calls no second `then()` on a native promise.
-        let awaited = bun_test::BunTest::promise_of_thenable(global, returned)?;
-        active::returned_set_cached(this_value, global, awaited);
-        match if awaited.as_promise().is_some() { awaited.get(global, "then")?.filter(|then| then.is_callable()) } else { None } {
-            Some(then) => {
-                let reactions = [
-                    bound(global, this_value, "", generated::Bun__ActiveFixture__onFulfilled)?,
-                    bound(global, this_value, "", generated::Bun__ActiveFixture__onRejected)?,
-                ];
-                then.call(global, awaited, &reactions)?;
-                Ok(())
-            }
-            None => self.on_returned(this_value, global, returned),
-        }
+        let awaited = bun_test::BunTest::awaited_promise(global, returned)?;
+        // The runner gets this value again at teardown, and it calls no second `then()` on a native promise.
+        active::returned_set_cached(this_value, global, awaited.unwrap_or(returned));
+        let Some(awaited) = awaited else {
+            return self.on_returned(this_value, global, returned);
+        };
+        awaited.then_with_value(global, this_value, generated::Bun__ActiveFixture__onFulfilled, generated::Bun__ActiveFixture__onRejected);
+        Ok(())
     }
 
     #[optimize(size)]
@@ -744,15 +738,15 @@ pub(crate) fn set_up(cx: &mut JsScope<'_>, this_value: JsLocal<'_>) -> JsResult<
 }
 
 #[optimize(size)]
-pub(crate) fn on_fulfilled(cx: &mut JsScope<'_>, this_value: JsLocal<'_>, returned: JsLocal<'_>) -> JsResult<()> {
+pub(crate) fn on_fulfilled(cx: &mut JsScope<'_>, returned: JsLocal<'_>, this_value: JsLocal<'_>) -> JsResult<()> {
     let global = cx.global();
     let (this, _) = ActiveFixture::of_call(global, this_value)?;
     this.on_returned(this_value.to_js(), global, returned.to_js())
 }
 
 /// The teardown entry awaits the same promise, so it reports a rejection that comes after `use()`.
 #[optimize(size)]
-pub(crate) fn on_rejected(cx: &mut JsScope<'_>, this_value: JsLocal<'_>, reason: JsLocal<'_>) -> JsResult<()> {
+pub(crate) fn on_rejected(cx: &mut JsScope<'_>, reason: JsLocal<'_>, this_value: JsLocal<'_>) -> JsResult<()> {
     let global = cx.global();
     let (this, _) = ActiveFixture::of_call(global, this_value)?;
     if this.state.get() == State::SettingUp {

test/js/bun/test/vitest-flavor.test.ts

@@ -2219,6 +2219,14 @@ describe.concurrent("aroundEach and aroundAll", () => {
           aroundEach(runTest => { runTest(); console.log("returned"); });
           test("test", () => console.log("test"));
         });
+        describe("then() is called one microtask turn after the hook returns, and a second thenable is followed", () => {
+          const order = [];
+          aroundEach(runTest => {
+            Promise.resolve().then(() => order.push("turn 1")).then(() => order.push("turn 2"));
+            return { then(resolve) { order.push("then()"); resolve({ then(resolve, reject) { runTest().then(resolve, reject); } }); } };
+          });
+          test("test", () => console.log(order.join(", ")));
+        });
       `,
     });
     expect({ log, results, exitCode }).toEqual({
@@ -2231,16 +2239,51 @@ describe.concurrent("aroundEach and aroundAll", () => {
         "the thenable resolves",
         "returned",
         "test",
+        "turn 1, then(), turn 2",
       ],
       results: [
         "(pass) thenable > test",
         "(pass) thenable > next",
         "(pass) returns right after it calls runTest() > test",
+        "(pass) then() is called one microtask turn after the hook returns, and a second thenable is followed > test",
       ],
       exitCode: 0,
     });
   });
 
+  test("the promise of a hook is awaited without a then() call", async () => {
+    const { log, results, exitCode } = await runTests({
+      "a.test.js": `
+        import { test, describe, aroundEach, aroundAll, afterAll } from "vitest";
+        const then = Promise.prototype.then;
+        let thenCalls = 0;
+        const replacements = {
+          "then is not callable": undefined,
+          "then is replaced": function (...reactions) { thenCalls++; return then.apply(this, reactions); },
+        };
+        // Promise.prototype.then is replaced while the runner attaches to what the hook returned.
+        function whileReplaced(name, returned) {
+          Promise.prototype.then = replacements[name];
+          queueMicrotask(() => { Promise.prototype.then = then; });
+          return returned;
+        }
+        for (const name in replacements) {
+          describe(name, () => {
+            aroundAll(runSuite => whileReplaced(name, (async () => { await null; await runSuite(); })()));
+            aroundEach(runTest => whileReplaced(name, (async () => { await null; await runTest(); })()));
+            test("test", () => {});
+          });
+        }
+        afterAll(() => console.log("then() calls:", thenCalls));
+      `,
+    });
+    expect({ log, results, exitCode }).toEqual({
+      log: ["then() calls: 0"],
+      results: ["(pass) then is not callable > test", "(pass) then is replaced > test"],
+      exitCode: 0,
+    });
+  });
+
   test("in a preload script, aroundAll goes around every file", async () => {
     const { log, exitCode } = await runTests(
       {
@@ -4626,6 +4669,51 @@ describe.concurrent("test.extend()", () => {
     });
   });
 
+  test("the promise of a fixture function is awaited without a then() call", async () => {
+    const { log, results, exitCode } = await runTests({
+      "a.test.js": `
+        import { test as base, expect, afterAll } from "vitest";
+        const then = Promise.prototype.then;
+        let thenCalls = 0;
+        const replacements = {
+          "then is not callable": undefined,
+          "then is replaced": function (...reactions) { thenCalls++; return then.apply(this, reactions); },
+        };
+        // Promise.prototype.then is replaced while the runner attaches to what the fixture function returned.
+        function whileReplaced(task, returned) {
+          Promise.prototype.then = replacements[task.name];
+          queueMicrotask(() => { Promise.prototype.then = then; });
+          return returned;
+        }
+        const test = base
+          .extend({ used: ({ task }, use) => whileReplaced(task, (async () => { await null; await use("used"); })()) })
+          .extend("built", ({ task }, {}) => whileReplaced(task, { then(resolve) { resolve("built"); } }));
+        for (const name in replacements) test(name, ({ used, built }) => { expect([used, built]).toEqual(["used", "built"]); });
+        afterAll(() => console.log("then() calls:", thenCalls));
+      `,
+    });
+    expect({ log, results, exitCode }).toEqual({
+      log: ["then() calls: 0"],
+      results: ["(pass) then is not callable", "(pass) then is replaced"],
+      exitCode: 0,
+    });
+  });
+
+  test("a promise whose own then is not callable is awaited or passed on, as await does", async () => {
+    const { log, results, exitCode } = await runTests({
+      "a.test.js": `
+        import { test as base } from "vitest";
+        class Plain extends Promise {}
+        const withoutThen = promise => Object.assign(promise, { then: 1 });
+        const test = base
+          .extend("native", ({}, {}) => withoutThen(Promise.resolve("awaited")))
+          .extend("subclass", ({}, {}) => withoutThen(Plain.resolve("not awaited")));
+        test("fixtures", ({ native, subclass }) => { console.log(native, subclass instanceof Plain); });
+      `,
+    });
+    expect({ log, results, exitCode }).toEqual({ log: ["awaited true"], results: ["(pass) fixtures"], exitCode: 0 });
+  });
+
   test("a fixture that fails", async () => {
     const { log, results, errors, stderr, exitCode } = await runTests({
       "a.test.js": `
@@ -5027,6 +5115,32 @@ describe.concurrent("test.extend()", () => {
     });
   });
 
+  test("a fixture function that settles after its test has ended runs its cleanup right away", async () => {
+    const { log, results, exitCode } = await runTests({
+      "a.test.js": `
+        import { test as base } from "vitest";
+        for (const event of ["unhandledRejection", "uncaughtException"]) process.on(event, error => console.log(event + ":", error.message));
+        const gate = Promise.withResolvers();
+        const late = cleanup => async ({}, { onCleanup }) => { onCleanup(cleanup); await gate.promise; };
+        const test = base
+          .extend("throws", late(() => { throw new Error("cleanup throws"); }))
+          .extend("rejects", late(async () => { throw new Error("cleanup rejects"); }));
+        test("throws", { timeout: 1 }, ({ throws }) => console.log("unreachable"));
+        test("rejects", { timeout: 1 }, ({ rejects }) => console.log("unreachable"));
+        test("a later test", async () => {
+          gate.resolve();
+          await new Promise(resolve => setImmediate(resolve));
+        });
+      `,
+    });
+    // The cleanup runs inside the reaction of the function's promise, so what it throws is thrown out of a job.
+    expect({ log, results, exitCode }).toEqual({
+      log: ["uncaughtException: cleanup throws", "unhandledRejection: cleanup rejects"],
+      results: ["(fail) throws", "(fail) rejects", "(pass) a later test"],
+      exitCode: 1,
+    });
+  });
+
   test("concurrent tests have their own fixtures, and share those of the file", async () => {
     const { log, results, exitCode } = await runTests({
       "a.test.js": `