diff --git a/src/jsc/JSPromise.rs b/src/jsc/JSPromise.rs index dd755e9019a8..c068f97802bc 100644 --- a/src/jsc/JSPromise.rs +++ b/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. diff --git a/src/jsc/VirtualMachine.rs b/src/jsc/VirtualMachine.rs index a00fcf3785a2..74e2c7eb9512 100644 --- a/src/jsc/VirtualMachine.rs +++ b/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 = diff --git a/src/runtime/test_runner/bun_test.rs b/src/runtime/test_runner/bun_test.rs index 58b7b7a6ccc9..4a13e90be660 100644 --- a/src/runtime/test_runner/bun_test.rs +++ b/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> { + 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() { diff --git a/src/runtime/test_runner/test_context.bind.ts b/src/runtime/test_runner/test_context.bind.ts index b9bb36ce5fdf..f51d6955a611 100644 --- a/src/runtime/test_runner/test_context.bind.ts +++ b/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 }); diff --git a/src/runtime/test_runner/test_context_around.rs b/src/runtime/test_runner/test_context_around.rs index bbe1c02cae6a..a6b36f6f9f6d 100644 --- a/src/runtime/test_runner/test_context_around.rs +++ b/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( diff --git a/src/runtime/test_runner/test_context_fixtures.rs b/src/runtime/test_runner/test_context_fixtures.rs index 3725fb392fbd..bfb72593c94d 100644 --- a/src/runtime/test_runner/test_context_fixtures.rs +++ b/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,7 +738,7 @@ 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()) @@ -752,7 +746,7 @@ pub(crate) fn on_fulfilled(cx: &mut JsScope<'_>, this_value: JsLocal<'_>, return /// 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 { diff --git a/test/js/bun/test/vitest-flavor.test.ts b/test/js/bun/test/vitest-flavor.test.ts index 8854b5b4fac7..96c632ce727e 100644 --- a/test/js/bun/test/vitest-flavor.test.ts +++ b/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": `