diff --git a/src/js/node/async_hooks.ts b/src/js/node/async_hooks.ts index ade40926f8da..8fe3b472d634 100644 --- a/src/js/node/async_hooks.ts +++ b/src/js/node/async_hooks.ts @@ -22,6 +22,9 @@ // each key is an AsyncLocalStorage object and the value is the associated value. There are a ton of // calls to $assert which will verify this invariant (only during bun-debug) // +// The one other key that can appear is bun:test's AsyncContextRef (stored as both key and value, only +// read from native code; see src/runtime/test_runner/AsyncContextRef.rs). Nothing in here touches it. +// const setAsyncHooksEnabled = $newCppFunction("NodeAsyncHooks.cpp", "jsSetAsyncHooksEnabled", 1); const cleanupLater = $newCppFunction("NodeAsyncHooks.cpp", "jsCleanupLater", 0); const { validateFunction, validateString, validateObject } = require("internal/validators"); @@ -49,7 +52,7 @@ function assertValidAsyncContextArray(array: unknown): array is ReadonlyArray 0, "AsyncContextData should be undefined if empty, got", Bun.inspect(array, { depth: 1 })); for (var i = 0; i < array.length; i += 2) { $assert( - array[i] instanceof AsyncLocalStorage, + array[i] instanceof AsyncLocalStorage || isBunTestAsyncContextRef(array[i]), `Odd indexes in AsyncContextData should be an array of AsyncLocalStorage\nIndex %s was %s`, i, array[i], @@ -58,12 +61,17 @@ function assertValidAsyncContextArray(array: unknown): array is ReadonlyArray | undefined) { if (value === undefined) return "undefined"; let str = "{\n"; for (var i = 0; i < value.length; i += 2) { - str += ` ${value[i].__id__}: typeof = ${typeof value[i + 1]}\n`; + str += ` ${isBunTestAsyncContextRef(value[i]) ? "bun:test" : value[i].__id__}: typeof = ${typeof value[i + 1]}\n`; } str += "}"; return str; diff --git a/src/jsc/bindings/AsyncContextFrame.cpp b/src/jsc/bindings/AsyncContextFrame.cpp index fd11c1d8291a..e70f4bda7253 100644 --- a/src/jsc/bindings/AsyncContextFrame.cpp +++ b/src/jsc/bindings/AsyncContextFrame.cpp @@ -1,7 +1,10 @@ #include "root.h" #include "ZigGlobalObject.h" +#include "ZigGeneratedClasses.h" #include "AsyncContextFrame.h" +#include #include +#include #if ASSERT_ENABLED #include @@ -131,3 +134,155 @@ JSValue AsyncContextFrame::profiledCall(JSGlobalObject* global, JSValue function return AsyncContextFrame::call(global, functionObject, thisValue, args); } #undef ASYNCCONTEXTFRAME_CALL_IMPL + +// ── bun:test (src/runtime/test_runner/AsyncContextRef.rs) ────────────────── +// +// The context array ([key, value, ...], see node/async_hooks.ts) gets one extra +// pair per test or hook invocation: (ref, ref), the invocation's AsyncContextRef. + +static bool isAsyncContextRef(JSValue value) +{ + return dynamicDowncast(value) != nullptr; +} + +static JSValue findAsyncContextRef(JSValue context) +{ + auto* array = dynamicDowncast(context); + if (!array) + return jsUndefined(); + unsigned length = array->length(); + for (unsigned i = 0; i < length; i += 2) { + if (!array->canGetIndexQuickly(i)) + continue; + JSValue key = array->getIndexQuickly(i); + if (isAsyncContextRef(key)) + return key; + } + return jsUndefined(); +} + +// A context the runner alone populated: without the runner there would be none. +static bool holdsOnlyAsyncContextRefs(JSValue context) +{ + auto* array = dynamicDowncast(context); + if (!array) + return false; + unsigned length = array->length(); + if (length == 0) + return false; + for (unsigned i = 0; i < length; i += 2) { + if (!array->canGetIndexQuickly(i) || !isAsyncContextRef(array->getIndexQuickly(i))) + return false; + } + return true; +} + +// The caller checks for an exception afterwards. +static void appendPairsWithoutRefs(JSGlobalObject* globalObject, JSValue context, MarkedArgumentBuffer& entries) +{ + auto& vm = JSC::getVM(globalObject); + auto scope = DECLARE_THROW_SCOPE(vm); + auto* array = dynamicDowncast(context); + if (!array) + return; + unsigned length = array->length(); + entries.ensureCapacity(length + 2); + for (unsigned i = 0; i < length; i += 2) { + JSValue key = array->getIndex(globalObject, i); + RETURN_IF_EXCEPTION(scope, ); + JSValue value = i + 1 < length ? array->getIndex(globalObject, i + 1) : jsUndefined(); + RETURN_IF_EXCEPTION(scope, ); + if (isAsyncContextRef(key)) + continue; + entries.append(key); + entries.append(value); + } +} + +// node:vm contexts copy this flag when created, so it is set before a test file loads. +extern "C" [[ZIG_EXPORT(nothrow)]] void Bun__AsyncContextRef__enableTracking(JSC::JSGlobalObject* globalObject) +{ + globalObject->setAsyncContextTrackingEnabled(true); +} + +// AsyncContextFrame::withAsyncContextIfNeeded for a callback being registered: the +// registering invocation's (ref, ref) is not a context to capture. __enter drops it +// from a context that is. +extern "C" [[ZIG_EXPORT(nothrow)]] JSC::EncodedJSValue Bun__AsyncContextRef__withAsyncContextIfNeeded(JSC::JSGlobalObject* globalObject, JSC::EncodedJSValue callbackValue) +{ + if (holdsOnlyAsyncContextRefs(globalObject->m_asyncContextData.get()->getInternalField(0))) + return callbackValue; + return JSValue::encode(AsyncContextFrame::withAsyncContextIfNeeded(globalObject, JSValue::decode(callbackValue))); +} + +// Returns what to invoke in place of `callback` so that it runs with its usual +// context plus (ref, ref). A callback registered under a context (an +// AsyncContextFrame) gets a new frame, which Bun__JSValue__call installs and +// restores as usual. Any other callback gets the array installed in place, so +// that, as before, what it does to the context (als.enterWith()) outlives it; +// __leave then only removes the ref. +extern "C" [[ZIG_EXPORT(zero_is_throw)]] JSC::EncodedJSValue Bun__AsyncContextRef__enter(JSC::JSGlobalObject* globalObject, JSC::EncodedJSValue callbackValue, JSC::EncodedJSValue refValue) +{ + auto& vm = JSC::getVM(globalObject); + auto scope = DECLARE_THROW_SCOPE(vm); + + JSValue callback = JSValue::decode(callbackValue); + JSValue ref = JSValue::decode(refValue); + ASSERT(isAsyncContextRef(ref)); + auto* slot = globalObject->m_asyncContextData.get(); + + auto* registrationFrame = dynamicDowncast(callback); + JSValue previousContext = registrationFrame ? registrationFrame->context.get() : slot->getInternalField(0); + + MarkedArgumentBuffer entries; + appendPairsWithoutRefs(globalObject, previousContext, entries); + RETURN_IF_EXCEPTION(scope, {}); + entries.append(ref); + entries.append(ref); + if (entries.hasOverflowed()) [[unlikely]] { + throwOutOfMemoryError(globalObject, scope); + return {}; + } + JSArray* context = constructArray(globalObject, static_cast(nullptr), entries); + RETURN_IF_EXCEPTION(scope, {}); + + globalObject->setAsyncContextTrackingEnabled(true); + + if (registrationFrame) + return JSValue::encode(AsyncContextFrame::create(globalObject, registrationFrame->callback.get(), context)); + + slot->putInternalField(vm, 0, context); + return JSValue::encode(callback); +} + +// Takes the refs out of whatever the callback left in the slot, keeping the rest +// in effect as before. Always rebuilt: als.disable() splices the installed array +// in place. After a frame the call restored the slot itself, so there is no ref. +extern "C" [[ZIG_EXPORT(check_slow)]] void Bun__AsyncContextRef__leave(JSC::JSGlobalObject* globalObject) +{ + auto& vm = JSC::getVM(globalObject); + auto scope = DECLARE_THROW_SCOPE(vm); + + auto* slot = globalObject->m_asyncContextData.get(); + JSValue current = slot->getInternalField(0); + if (findAsyncContextRef(current).isUndefined()) + return; + MarkedArgumentBuffer entries; + appendPairsWithoutRefs(globalObject, current, entries); + RETURN_IF_EXCEPTION(scope, ); + if (entries.hasOverflowed()) [[unlikely]] { + throwOutOfMemoryError(globalObject, scope); + return; + } + JSValue remaining = jsUndefined(); + if (!entries.isEmpty()) { + remaining = constructArray(globalObject, static_cast(nullptr), entries); + RETURN_IF_EXCEPTION(scope, ); + } + slot->putInternalField(vm, 0, remaining); +} + +extern "C" [[ZIG_EXPORT(nothrow)]] JSC::EncodedJSValue Bun__AsyncContextRef__current(JSC::JSGlobalObject* globalObject) +{ + return JSValue::encode(findAsyncContextRef(globalObject->m_asyncContextData.get()->getInternalField(0))); +} diff --git a/src/jsc/event_loop.rs b/src/jsc/event_loop.rs index 4058f8e0b9b4..8a138a6be800 100644 --- a/src/jsc/event_loop.rs +++ b/src/jsc/event_loop.rs @@ -1138,26 +1138,6 @@ impl EventLoop { } } - /// Prefer `runCallbackWithResult` unless you really need to make sure that microtasks are drained. - pub fn run_callback_with_result_and_forcefully_drain_microtasks( - &mut self, - callback: JSValue, - global_object: &JSGlobalObject, - this_value: JSValue, - arguments: &[JSValue], - ) -> JsResult { - // Same gate as `run_callback`. - if global_object.has_exception() { - return Ok(JSValue::UNDEFINED); - } - let result = callback.call(global_object, this_value, arguments)?; - result.ensure_still_alive(); - let jsc_vm = global_object.bun_vm().jsc_vm(); - self.drain_microtasks_with_global(global_object, jsc_vm) - .map_err(|stopped| stopped.throw(global_object))?; - Ok(result) - } - /// Keep one poll registered with the loop so `us_loop_run_bun_tick` parks /// instead of returning immediately on `num_polls == 0`. #[cfg(not(windows))] diff --git a/src/jsc/generated_classes_list.rs b/src/jsc/generated_classes_list.rs index 0a3be533a841..8c14429a23bb 100644 --- a/src/jsc/generated_classes_list.rs +++ b/src/jsc/generated_classes_list.rs @@ -33,6 +33,7 @@ pub mod Classes { pub use crate::crypto::CryptoHasher; pub use crate::image as Image; pub use crate::shell::Interpreter as ShellInterpreter; + pub use crate::test_runner::async_context_ref::AsyncContextRef; pub use crate::test_runner::done_callback::DoneCallback; pub use crate::test_runner::expect::Expect; pub use crate::test_runner::expect::ExpectAny; diff --git a/src/runtime/cli/test_command.rs b/src/runtime/cli/test_command.rs index 577d075d380d..5eaab8c8e181 100644 --- a/src/runtime/cli/test_command.rs +++ b/src/runtime/cli/test_command.rs @@ -3276,6 +3276,8 @@ impl TestCommand { } // need to wake up so autoTick() doesn't wait for 16-100ms after loading the entrypoint vm.wakeup(); + // Before the file loads: node:vm contexts copy this flag when they are created. + jsc::cpp::Bun__AsyncContextRef__enableTracking(vm.global()); let promise = vm.load_entry_point_for_test_runner(file_path)?; // Only count the file once, not once per repeat if repeat_index == 0 { diff --git a/src/runtime/test_runner/AsyncContextRef.rs b/src/runtime/test_runner/AsyncContextRef.rs new file mode 100644 index 000000000000..ef3547f9a68b --- /dev/null +++ b/src/runtime/test_runner/AsyncContextRef.rs @@ -0,0 +1,64 @@ +//! Put into the async context for the duration of one test or hook invocation, +//! so that the invocation's continuations can still be traced back to it. +//! +//! `expect()`, the snapshot matchers, `expect.assertions()` and hook registration +//! belong to whatever entry the runner is executing when they are called. Once +//! the runner has abandoned an invocation that is still running (timeout, or an +//! unhandled error while it was awaiting), that would be the next entry; +//! `RefData::abandoned` makes `bun_test::caller_ref` attribute such late calls +//! to the abandoned invocation instead, which rejects them. Invocations that +//! completed are never consulted, so callbacks they registered keep belonging to +//! whichever entry runs them. Uncaught errors are not covered: by the time one is +//! reported (`jest::on_unhandled_rejection`) the context it was thrown in is gone. +//! +//! The slot manipulation lives next to `AsyncContextFrame` (AsyncContextFrame.cpp). + +use bun_jsc::{JSGlobalObject, JSValue, JsClass as _, JsResult}; + +use crate::test_runner::bun_test::RefDataPtr; + +#[bun_jsc::JsClass(no_construct, no_constructor)] // codegen wires to_js / from_js +pub struct AsyncContextRef { + /// Owned `+1`, released in `finalize`. + r#ref: RefDataPtr, +} + +impl AsyncContextRef { + // Codegen calls `finalize(Box)`; clippy::boxed_local is a false positive. + #[allow(clippy::boxed_local)] + pub fn finalize(self: Box) { + self.r#ref.deref(); // `RefPtr` has no `Drop` + } + + /// Puts `refdata` (a `+1`, consumed) into the context `callback` is about to + /// run with, and returns what to invoke in its place. Pair with [`Self::leave`]. + pub(crate) fn enter(global: &JSGlobalObject, callback: JSValue, refdata: RefDataPtr) -> JsResult { + let ref_js = AsyncContextRef { r#ref: refdata }.to_js(global); + let callable = bun_jsc::cpp::Bun__AsyncContextRef__enter(global, callback, ref_js); + ref_js.ensure_still_alive(); + callable + } + + /// Call once the callback returned, before its microtasks run. + pub(crate) fn leave(global: &JSGlobalObject) -> JsResult<()> { + bun_jsc::cpp::Bun__AsyncContextRef__leave(global) + } + + /// A `+1` to the abandoned invocation the running JS descends from, if any. + pub(crate) fn abandoned_caller(global: &JSGlobalObject) -> Option { + Self::abandoned_in_context(global).map(RefDataPtr::dupe_ref) + } + + pub(crate) fn caller_is_abandoned(global: &JSGlobalObject) -> bool { + Self::abandoned_in_context(global).is_some() + } + + /// To use right away: the borrow is of a wrapper that the context array + /// installed at this moment keeps alive. + fn abandoned_in_context(global: &JSGlobalObject) -> Option<&RefDataPtr> { + let this = Self::from_js(bun_jsc::cpp::Bun__AsyncContextRef__current(global))?; + // SAFETY: live payload of the wrapper `__current` just found in the context array. + let refdata: &RefDataPtr = unsafe { &(*this).r#ref }; + refdata.abandoned.get().then_some(refdata) + } +} diff --git a/src/runtime/test_runner/Collection.rs b/src/runtime/test_runner/Collection.rs index 44bfd6875d28..e4e35d368d3a 100644 --- a/src/runtime/test_runner/Collection.rs +++ b/src/runtime/test_runner/Collection.rs @@ -141,7 +141,7 @@ impl Collection { // `self.active_scope` in `step()` and mutated through). self.current_scope_callback_queue.push(QueuedDescribe { active_scope: self.active_scope, - callback: DeprecatedStrong::init(cb), + callback: DeprecatedStrong::init(bun_test::keep_registration_async_context(cb)), new_scope: NonNull::from(new_scope), }); } @@ -238,6 +238,7 @@ impl Collection { buntest_strong, global_this, callback.get(), + None, false, RefDataValue::Collection { active_scope: previous_scope }, &Timespec::EPOCH, diff --git a/src/runtime/test_runner/Execution.rs b/src/runtime/test_runner/Execution.rs index a864c8f8b17b..3c333c4a240b 100644 --- a/src/runtime/test_runner/Execution.rs +++ b/src/runtime/test_runner/Execution.rs @@ -46,7 +46,7 @@ use bun_core::scoped_log; use super::debug::group as group_log; // bun_test.debug.group use super::bun_test::{ group_begin, AddedInPhase, BunTest, BunTestPtr, EntryData, ExecutionEntry, - HandleUncaughtExceptionResult, Order, RefDataValue, ScopeMode, StepResult, + HandleUncaughtExceptionResult, Order, RefDataPtr, RefDataValue, ScopeMode, StepResult, }; use crate::cli::test_command; @@ -167,6 +167,9 @@ pub struct ExecutionSequence { /// Expectation set by expect.hasAssertions() or expect.assertions(n). pub(crate) expect_assertions: ExpectAssertions, pub(crate) maybe_skip: bool, + /// The `RefData` the callback being executed runs under (AsyncContextRef.rs). + /// Owned `+1`, released by `advance_sequence` or `Drop`. + pub(crate) executing_ref: Option, } impl ExecutionSequence { @@ -189,6 +192,20 @@ impl ExecutionSequence { expect_call_count: 0, expect_assertions: ExpectAssertions::NotSet, maybe_skip: false, + executing_ref: None, + } + } + + /// Called when the runner moves on before the executing callback completed. + pub(crate) fn abandon_executing_callback(&self) { + if let Some(executing_ref) = &self.executing_ref { + executing_ref.abandoned.set(true); + } + } + + fn release_executing_ref(&mut self) { + if let Some(executing_ref) = self.executing_ref.take() { + executing_ref.deref(); } } @@ -201,6 +218,12 @@ impl ExecutionSequence { } } +impl Drop for ExecutionSequence { + fn drop(&mut self) { + self.release_executing_ref(); + } +} + #[derive(Clone, Copy, PartialEq, Eq, Default, strum::IntoStaticStr)] #[repr(u8)] pub enum Result { @@ -509,6 +532,7 @@ impl Execution { let sequence = unsafe { &mut *sequence_ptr.as_ptr() }; debug_assert!(sequence.executing); + sequence.release_executing_ref(); if let Some(entry_ptr) = sequence.active_entry { // SAFETY: arena-owned entry, alive for lifetime of BunTest let entry = unsafe { entry_ptr.as_ref() }; @@ -974,6 +998,7 @@ fn step_sequence_one( // SAFETY: arena-owned entry let active_entry = unsafe { &mut *active_entry_ptr.as_ptr() }; if active_entry.evaluate_timeout(sequence, now) { + sequence.abandon_executing_callback(); Execution::advance_sequence(buntest_ptr, sequence_ptr, group); return Ok(None); // run again } @@ -1012,6 +1037,11 @@ fn step_sequence_one( }; group_log::log(format_args!("runSequence queued callback: {}", callback_data)); + // Shared by the sequence (to mark it abandoned) and the callback's async context. + let invocation_ref: RefDataPtr = BunTest::ref_(buntest_strong, callback_data.clone()); + debug_assert!(sequence.executing_ref.is_none()); + sequence.executing_ref = Some(invocation_ref.dupe_ref()); + let prev_on_stack = this.on_stack_entry.replace(Some(next_item_ptr)); let prev_on_stack_data = this.on_stack_entry_data.replace(Some(entry_data)); let on_stack_cell = &raw const this.on_stack_entry; @@ -1027,6 +1057,7 @@ fn step_sequence_one( buntest_strong, global_this, cb.get(), + Some(invocation_ref), next_item.has_done_parameter, callback_data, &next_item.timespec, diff --git a/src/runtime/test_runner/ScopeFunctions.rs b/src/runtime/test_runner/ScopeFunctions.rs index a30dd6f486b5..5a11fa0c88d6 100644 --- a/src/runtime/test_runner/ScopeFunctions.rs +++ b/src/runtime/test_runner/ScopeFunctions.rs @@ -664,10 +664,11 @@ pub(crate) fn parse_arguments( }; let (description, callback, options) = (items.description, items.callback, items.options); + // Unwrapped; `bun_test::keep_registration_async_context` applies when it is stored. let result_callback: Option = if cfg.callback != CallbackMode::Require && callback.is_undefined_or_null() { None } else if callback.is_function() { - Some(callback.with_async_context_if_needed(global)) + Some(callback) } else { let ordinal = if cfg.kind == FunctionKind::Hook { "first" } else { "second" }; return Err(global.throw(format_args!("{} expects a function as the {} argument", signature, ordinal))); diff --git a/src/runtime/test_runner/bun_test.rs b/src/runtime/test_runner/bun_test.rs index 8f2ee1f522c2..338bd946e2bc 100644 --- a/src/runtime/test_runner/bun_test.rs +++ b/src/runtime/test_runner/bun_test.rs @@ -52,11 +52,31 @@ fn strong_create(value: JSValue) -> Strong { Strong::create(value, global) } +/// `JSValue::with_async_context_if_needed` minus the registering test's own ref (AsyncContextRef.rs). +/// Applied where the callback is stored: registration reads `.length` of and `.bind()`s the +/// function itself, and the wrapper is not a function. +pub(crate) fn keep_registration_async_context(callback: JSValue) -> JSValue { + bun_jsc::cpp::Bun__AsyncContextRef__withAsyncContextIfNeeded(VirtualMachine::get().global(), callback) +} + pub(crate) fn clone_active_strong() -> Option { let runner = Jest::runner()?; runner.bun_test_root.clone_active_file() } +/// What an `expect()`-family call made right now belongs to: the entry being +/// executed, or the abandoned invocation the caller descends from (AsyncContextRef.rs). +/// `None` outside `bun test`. Returns an owned `+1`. +pub(crate) fn caller_ref(global: &JSGlobalObject) -> Option { + if let Some(abandoned) = AsyncContextRef::abandoned_caller(global) { + return Some(abandoned); + } + let buntest_strong = clone_active_strong()?; + let state = buntest_strong.get().get_current_state_data(); + Some(BunTest::ref_(&buntest_strong, state)) +} + +pub use super::async_context_ref::AsyncContextRef; pub use super::done_callback::DoneCallback; pub mod js_fns { @@ -173,6 +193,14 @@ pub mod js_fns { let tag_name: &'static str = tag.into(); let sig_bytes: &'static [u8] = tag.sig(); + // Otherwise the hook would be added to whatever happens to be running now. + if AsyncContextRef::caller_is_abandoned(global_this) { + return Err(global_this.throw(format_args!( + "Cannot call {}() here. The test or hook it was called from has already finished executing (it timed out, or failed while it was still running).", + tag_name + ))); + } + let args = ScopeFunctions::parse_arguments( global_this, call_frame, @@ -736,6 +764,7 @@ impl BunTest { bun_ptr::IntrusiveRc::new(RefData { buntest_weak: Rc::downgrade(this_strong), phase, + abandoned: core::cell::Cell::new(false), ref_count: bun_ptr::RefCount::init(), }) } @@ -1118,10 +1147,14 @@ impl BunTest { } /// if sync, the result is returned. if async, None is returned. + /// + /// `invocation_ref` (a `+1`, consumed) is what the callback runs under + /// (AsyncContextRef.rs); describe callbacks pass `None`. pub(crate) fn run_test_callback( this_strong: &BunTestPtr, global_this: &JSGlobalObject, cfg_callback: JSValue, + invocation_ref: Option, cfg_done_parameter: bool, cfg_data: RefDataValue, timeout: &Timespec, @@ -1152,24 +1185,61 @@ impl BunTest { }; } + let mut callable: JSValue = cfg_callback; + let mut entered = false; + if let Some(invocation_ref) = invocation_ref { + match AsyncContextRef::enter(global_this, cfg_callback, invocation_ref) { + Ok(v) => { + callable = v; + entered = true; + } + Err(e) => { + // OOM: charge it to this entry and still run the callback, as for `done` above. + // SAFETY: `UnsafeCell`-derived; sole `&mut` at this point. + unsafe { (*this).on_uncaught_exception(global_this, Some(global_this.take_exception(e)), false, &cfg_data) }; + } + } + } + // SAFETY: `UnsafeCell`-derived; sole `&mut` at this point (before JS re-entry). unsafe { (*this).update_min_timeout(global_this, timeout) }; let args_slice: &[JSValue] = if !done_arg.is_empty() { core::slice::from_ref(&done_arg) } else { &[] }; - let result: JSValue = match vm.event_loop_mut().run_callback_with_result_and_forcefully_drain_microtasks( - cfg_callback, - global_this, - JSValue::UNDEFINED, - args_slice, - ) { - Ok(v) => v, - Err(_) => { + + // Call, leave, then drain microtasks. The callback's exception is taken + // before `leave` runs. `Some(None)` is a termination (nothing to print). + let mut failure: Option> = None; + let mut result: JSValue = JSValue::UNDEFINED; + if !global_this.has_exception() { + match callable.call(global_this, JSValue::UNDEFINED, args_slice) { + Ok(v) => result = v, + Err(_) => { + global_this.clear_termination_exception(); + failure = Some(global_this.try_take_exception()); + } + } + } + if entered { + if let Err(e) = AsyncContextRef::leave(global_this) { + let exception = global_this.take_exception(e); + if failure.is_none() { + failure = Some(Some(exception)); + } + } + } + if failure.is_none() { + if let Err(stopped) = vm.event_loop_mut().drain_microtasks_with_global(global_this, vm.jsc_vm()) { + let _ = stopped.throw(global_this); global_this.clear_termination_exception(); - // SAFETY: re-derive after JS callback returned; no outer `&mut` was held across it. - unsafe { (*this).on_uncaught_exception(global_this, global_this.try_take_exception(), false, &cfg_data) }; - bun_core::scoped_log!(bun_test_group, "callTestCallback -> error"); - JSValue::ZERO + failure = Some(global_this.try_take_exception()); } - }; + } + result.ensure_still_alive(); + if let Some(exception) = failure { + // SAFETY: re-derive after JS callback returned; no outer `&mut` was held across it. + unsafe { (*this).on_uncaught_exception(global_this, exception, false, &cfg_data) }; + bun_core::scoped_log!(bun_test_group, "callTestCallback -> error"); + result = JSValue::ZERO; + } done_callback.ensure_still_alive(); @@ -1511,6 +1581,9 @@ impl fmt::Display for RefDataValue { pub struct RefData { pub(crate) buntest_weak: BunTestPtrWeak, pub(crate) phase: RefDataValue, + /// The runner moved on while this invocation's callback was still running + /// (set through `ExecutionSequence::executing_ref`, read by [`caller_ref`]). + pub(crate) abandoned: core::cell::Cell, pub(crate) ref_count: bun_ptr::RefCount, } // `*RefData` crosses FFI (`as_promise_ptr`), so this MUST be `bun_ptr::IntrusiveRc` (= `RefPtr`), never `Rc`. @@ -1943,9 +2016,9 @@ impl ExecutionEntry { ScopeMode::Skip => None, ScopeMode::Todo => { let run_todo = Jest::runner().is_some_and(|runner| runner.run_todo); - if run_todo { Some(strong_create(c)) } else { None } + if run_todo { Some(strong_create(keep_registration_async_context(c))) } else { None } } - _ => Some(strong_create(c)), + _ => Some(strong_create(keep_registration_async_context(c))), }; } entry diff --git a/src/runtime/test_runner/expect.rs b/src/runtime/test_runner/expect.rs index 3f551ac9a85b..7b9753dd4a21 100644 --- a/src/runtime/test_runner/expect.rs +++ b/src/runtime/test_runner/expect.rs @@ -188,7 +188,9 @@ impl Expect { let Some(parent) = self.parent.as_ref() else { return }; // not in bun:test let Some(buntest_strong) = parent.bun_test() else { return }; // the test file this expect() call was for is no longer let buntest = buntest_strong.get(); - if let Some(sequence) = parent.phase.sequence(buntest) { + // An abandoned invocation's late calls only count towards the total. + let sequence = if parent.abandoned.get() { None } else { parent.phase.sequence(buntest) }; + if let Some(sequence) = sequence { // found active sequence sequence.expect_call_count = sequence.expect_call_count.saturating_add(1); } else { @@ -210,6 +212,14 @@ impl Expect { parent.bun_test() } + /// Same error as for an `expect()` left over from a finished test (AsyncContextRef.rs). + fn reject_snapshot_if_abandoned(&self, global_this: &JSGlobalObject) -> JsResult<()> { + if self.parent.as_ref().is_some_and(|parent| parent.abandoned.get()) { + return Err(global_this.throw(format_args!("Snapshot matchers are not supported after the test has finished executing"))); + } + Ok(()) + } + pub(crate) fn get_signature( matcher_name: &'static str, args: &'static str, @@ -707,22 +717,13 @@ impl Expect { } } - let active_execution_entry_ref = if let Some(buntest_strong_) = bun_test::clone_active_strong() { - let buntest_strong = buntest_strong_; - let state = buntest_strong.get().get_current_state_data(); - Some(bun_test::BunTest::ref_(&buntest_strong, state)) - } else { - None - }; - // The ref - // moves into `Expect` below and `to_js()` is infallible, so there is no - // error path between ref creation and the wrapper taking ownership; from - // then on `Expect::finalize` derefs `parent` (RefDataPtr has no Drop). - + // The ref moves into `Expect` below and `to_js()` is infallible, so there + // is no error path between ref creation and the wrapper taking ownership; + // from then on `Expect::finalize` derefs `parent` (RefDataPtr has no Drop). let expect = Expect { flags: Cell::new(Flags::default()), custom_label, - parent: active_execution_entry_ref, + parent: bun_test::caller_ref(global_this), }; // `JsClass::to_js` boxes `self` and hands the pointer to `${T}__create`. let expect_js_value = expect.to_js(global_this); @@ -1057,6 +1058,7 @@ impl Expect { let signature = Self::get_signature(fn_name, "", false); return throw!(this, global_this, signature, "\n\nMatcher error: Snapshot matchers cannot be used outside of a test\n"); }; + this.reject_snapshot_if_abandoned(global_this)?; match runner.snapshots.add_count(this, b"") { Ok(_) => {} Err(crate::Error::Alloc(bun_alloc::AllocError)) => return Err(JsError::OutOfMemory), @@ -1205,6 +1207,7 @@ impl Expect { fn_name: &'static str, ) -> JsResult { let this = self; + this.reject_snapshot_if_abandoned(global_this)?; let mut pretty_value: Vec = Vec::new(); this.match_and_fmt_snapshot(global_this, value, property_matchers, &mut pretty_value, fn_name)?; @@ -1660,18 +1663,32 @@ impl Expect { // SAFETY: bun_vm() returns the live VM pointer for this global. let _gc = global_this.bun_vm().as_mut().auto_gc_on_drop(); - let Some(buntest_strong) = bun_test::clone_active_strong() else { - return Err(global_this.throw(format_args!("expect.assertions() must be called within a test"))); + Self::with_caller_sequence(global_this, "expect.hasAssertions()", |sequence| { + if !matches!(sequence.expect_assertions, ExpectAssertions::Exact(_)) { + sequence.expect_assertions = ExpectAssertions::AtLeastOne; + } + }) + } + + /// Runs `f` on the sequence a call of `matcher_name` made right now applies to. + fn with_caller_sequence( + global_this: &JSGlobalObject, + matcher_name: &str, + f: impl FnOnce(&mut super::execution::ExecutionSequence), + ) -> JsResult { + let Some(caller) = bun_test::caller_ref(global_this) else { + return Err(global_this.throw(format_args!("{matcher_name} must be called within a test"))); }; - let buntest = buntest_strong.get(); - let state_data = buntest.get_current_state_data(); - let Some(execution) = state_data.sequence(buntest) else { - return Err(global_this.throw(format_args!("expect.assertions() is not supported in the describe phase, in concurrent tests, between tests, or after test execution has completed"))); + // `RefPtr` has no Drop; release the `+1` from `caller_ref` on every exit path. + let caller = scopeguard::guard(caller, |caller| caller.deref()); + let buntest_strong = if caller.abandoned.get() { None } else { caller.bun_test() }; + let sequence = buntest_strong + .as_ref() + .and_then(|buntest_strong| caller.phase.sequence(buntest_strong.get())); + let Some(sequence) = sequence else { + return Err(global_this.throw(format_args!("{matcher_name} is not supported in the describe phase, in concurrent tests, between tests, or after test execution has completed"))); }; - if !matches!(execution.expect_assertions, ExpectAssertions::Exact(_)) { - execution.expect_assertions = ExpectAssertions::AtLeastOne; - } - + f(sequence); Ok(JSValue::UNDEFINED) } @@ -1712,17 +1729,9 @@ impl Expect { let unsigned_expected_assertions: u32 = expected_assertions as u32; - let Some(buntest_strong) = bun_test::clone_active_strong() else { - return Err(global_this.throw(format_args!("expect.assertions() must be called within a test"))); - }; - let buntest = buntest_strong.get(); - let state_data = buntest.get_current_state_data(); - let Some(execution) = state_data.sequence(buntest) else { - return Err(global_this.throw(format_args!("expect.assertions() is not supported in the describe phase, in concurrent tests, between tests, or after test execution has completed"))); - }; - execution.expect_assertions = ExpectAssertions::Exact(unsigned_expected_assertions); - - Ok(JSValue::UNDEFINED) + Self::with_caller_sequence(global_this, "expect.assertions()", |sequence| { + sequence.expect_assertions = ExpectAssertions::Exact(unsigned_expected_assertions); + }) } diff --git a/src/runtime/test_runner/jest.classes.ts b/src/runtime/test_runner/jest.classes.ts index cb7c7b8d5735..d2cca26ffbcf 100644 --- a/src/runtime/test_runner/jest.classes.ts +++ b/src/runtime/test_runner/jest.classes.ts @@ -803,6 +803,18 @@ export default [ klass: {}, proto: {}, }), + // Internal to the runner (AsyncContextRef.rs). + define({ + name: "AsyncContextRef", + construct: false, + noConstructor: true, + finalize: true, + JSType: "0b11101110", + values: [], + configurable: false, + klass: {}, + proto: {}, + }), define({ name: "ScopeFunctions", construct: false, diff --git a/src/runtime/test_runner/jest.rs b/src/runtime/test_runner/jest.rs index 2889638ff424..8dd82e37d461 100644 --- a/src/runtime/test_runner/jest.rs +++ b/src/runtime/test_runner/jest.rs @@ -603,7 +603,9 @@ pub(crate) mod on_unhandled_rejection { // re-borrows. Const→mut projection is centralized in `buntest_as_mut` // pending the BunTestPtr interior-mut reshape (see bun_test.rs). let buntest = unsafe { bun_test::buntest_as_mut(&buntest_strong) }; - // mark unhandled errors as belonging to the currently active test. note that this can be misleading. + // mark unhandled errors as belonging to the currently active test. note that this can be misleading: + // it includes errors thrown late by an abandoned invocation (AsyncContextRef.rs), whose context is + // no longer installed when the error gets here. let mut current_state_data = buntest.get_current_state_data(); // split entry()/sequence() borrows via raw-ptr capture (per-use reborrow). let entry_ptr: Option<*mut bun_test::ExecutionEntry> = current_state_data @@ -614,6 +616,9 @@ pub(crate) mod on_unhandled_rejection { if sequence.test_entry.map(|p| p.as_ptr()) != Some(entry) { // mark errors in hooks as 'unhandled error between tests' current_state_data = RefDataValue::Start; + } else { + // `add_result` below moves past this test while its callback may still be awaiting. + sequence.abandon_executing_callback(); } } } diff --git a/src/runtime/test_runner/mod.rs b/src/runtime/test_runner/mod.rs index 4281593576e6..b99c33f4d2a8 100644 --- a/src/runtime/test_runner/mod.rs +++ b/src/runtime/test_runner/mod.rs @@ -134,6 +134,7 @@ macro_rules! throw_pretty_static { } cfg_jsc! { + #[path = "AsyncContextRef.rs"] pub mod async_context_ref; #[path = "bun_test.rs"] pub mod bun_test; #[path = "Collection.rs"] pub mod collection; #[path = "debug.rs"] pub mod debug; @@ -493,6 +494,7 @@ pub mod expect { // public surface for `crate::test_runner::*` consumers cfg_jsc! { + pub use async_context_ref::AsyncContextRef; pub use done_callback::DoneCallback; pub use expect::{ Expect, ExpectAny, ExpectAnything, ExpectArrayContaining, ExpectCloseTo, diff --git a/test/js/bun/test/expect-assertions.test.ts b/test/js/bun/test/expect-assertions.test.ts index d8df6410fdb6..a87c735337af 100644 --- a/test/js/bun/test/expect-assertions.test.ts +++ b/test/js/bun/test/expect-assertions.test.ts @@ -28,3 +28,43 @@ test("expect.assertions causes the test to fail when it should", async () => { expect(result.stderr.toString()).toContain("5 fail\n"); expect(result.stderr.toString()).toContain("0 pass\n"); }); + +test("expect() calls made by a test after it timed out do not count towards the next test", async () => { + // "timed out" only continues once "counts its own" has started, so it has always timed out by + // then; the two expect() calls it makes from there on belong to it, not to the running test. + using dir = tempDir("late-expect-calls", { + "late.test.ts": /* ts */ ` + import { expect, test } from "bun:test"; + const nextStarted = Promise.withResolvers(); + const lateCallsDone = Promise.withResolvers(); + test("timed out", async () => { + await nextStarted.promise; + expect(1).toBe(1); + expect(2).toBe(2); + lateCallsDone.resolve(); + }, 1); + test("counts its own", async () => { + expect.assertions(1); + nextStarted.resolve(); + await lateCallsDone.promise; + expect(3).toBe(3); + }); + `, + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "test", "late.test.ts"], + cwd: String(dir), + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect(stderr).toContain("(fail) timed out"); + expect(stderr).toContain("(pass) counts its own"); + expect(stderr).not.toContain("expected 1 assertion"); + // The late calls are still counted in the total, just not against the running test. + expect(stderr).toContain(" 3 expect() calls\n"); + expect(exitCode).toBe(1); +}); diff --git a/test/js/bun/test/jest-each.test.ts b/test/js/bun/test/jest-each.test.ts index 0bd0239ac72a..9a9265ba6dac 100644 --- a/test/js/bun/test/jest-each.test.ts +++ b/test/js/bun/test/jest-each.test.ts @@ -1,4 +1,5 @@ -import { describe, expect, it } from "bun:test"; +import { beforeEach, describe, expect, it } from "bun:test"; +import { AsyncLocalStorage } from "node:async_hooks"; const NUMBERS = [ [1, 1, 2], @@ -57,3 +58,42 @@ describe.each(["some", "cool", "strings"])("works with describe: %s", s => { describe("does not return zero", () => { expect(it.each([1, 2])("wat", () => {})).toBeUndefined(); }); + +// Callbacks registered while an AsyncLocalStorage store is active run with that store. Registration +// must still look at the function itself: its length decides whether it takes `done`, and `.each` +// binds the row to it. +describe("registered inside an AsyncLocalStorage context", () => { + const storage = new AsyncLocalStorage(); + storage.run("registration", () => { + beforeEach(() => { + expect(storage.getStore()).toBe("registration"); + }); + it("a test without a done parameter is not waited on", () => { + expect(storage.getStore()).toBe("registration"); + }); + it("a test with a done parameter still gets one", done => { + expect(storage.getStore()).toBe("registration"); + done(); + }); + it.each(NUMBERS)("it.each: %i + %i = %i", (a, b, e) => { + expect(a + b).toBe(e); + expect(storage.getStore()).toBe("registration"); + }); + it.each([[1, 1, 2]])("it.each with a done parameter: %i + %i = %i", (a, b, e, done) => { + expect(a + b).toBe(e); + expect(storage.getStore()).toBe("registration"); + (done as unknown as () => void)(); + }); + describe.each(["nested"])("describe.each: %s", s => { + it(`keeps the store in ${s} describes`, () => { + expect(storage.getStore()).toBe("registration"); + }); + }); + }); + + // The beforeEach above still runs (with its own store) before this test; its store must not + // be left behind for a test that was registered outside of it. + it("a test registered outside of the store does not see it", () => { + expect(storage.getStore()).toBeUndefined(); + }); +}); diff --git a/test/js/bun/test/jest-hooks.test.ts b/test/js/bun/test/jest-hooks.test.ts index 425951591a14..3de94e88e9e2 100644 --- a/test/js/bun/test/jest-hooks.test.ts +++ b/test/js/bun/test/jest-hooks.test.ts @@ -1,4 +1,5 @@ -import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it } from "bun:test"; +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, onTestFinished } from "bun:test"; +import { AsyncLocalStorage } from "node:async_hooks"; let hooks_run: string[] = []; @@ -249,3 +250,97 @@ describe("test jest hooks in bun-test", () => { }); }); }); + +// The runner adds its own entry to the async context while a hook or test runs; what the +// callback itself does to the context has to stay in effect afterwards, as it always has. +describe("what a hook does to the AsyncLocalStorage context stays in effect for the tests that follow", () => { + const storage = new AsyncLocalStorage(); + afterAll(() => storage.disable()); + + // First, while no store has been entered yet: a hook registered while a store is active runs + // under that store instead (last describe in this file). The runner's own entry is in the + // context when these hooks are registered, and must not count as such a store. + describe("entered in hooks registered inside a test", () => { + let storeWhenOnTestFinishedRan: string | undefined; + + it("registers them", () => { + afterEach(() => { + storage.enterWith("from the afterEach"); + }); + // Runs after the afterEach hooks, including the file's. + onTestFinished(() => { + storeWhenOnTestFinishedRan = storage.getStore(); + storage.enterWith("from the onTestFinished"); + }); + }); + + it("each one's store is in effect for what follows it", () => { + expect(storeWhenOnTestFinishedRan).toBe("from the afterEach"); + expect(storage.getStore()).toBe("from the onTestFinished"); + }); + }); + + describe("entered in beforeAll", () => { + beforeAll(() => { + storage.enterWith("from beforeAll"); + }); + + it("is the store of the first test", () => { + expect(storage.getStore()).toBe("from beforeAll"); + }); + + it("and of the next one, before and after an await", async () => { + expect(storage.getStore()).toBe("from beforeAll"); + await Promise.resolve(); + expect(storage.getStore()).toBe("from beforeAll"); + }); + }); + + describe("entered in beforeEach", () => { + let runs = 0; + beforeEach(() => { + storage.enterWith(`beforeEach run ${++runs}`); + }); + + it("is the store of the test it ran for", () => { + expect(storage.getStore()).toBe("beforeEach run 1"); + }); + + it("and is replaced for the next test", () => { + expect(storage.getStore()).toBe("beforeEach run 2"); + }); + }); + + // disable() splices the storage out of the context array in place rather than replacing it. + describe("disabled in a later beforeAll", () => { + beforeAll(() => { + storage.enterWith("entered before disable()"); + }); + beforeAll(() => { + storage.disable(); + }); + + it("stays gone: run() does not bring the old store back", () => { + storage.run("inside run()", () => {}); + expect(storage.getStore()).toBeUndefined(); + }); + }); +}); + +describe("a hook registered inside a test under AsyncLocalStorage.run() runs with that store", () => { + const storage = new AsyncLocalStorage(); + let storeSeenByAfterEach: string | undefined; + + it("registers the hook", () => { + storage.run("store of the run() that registered it", () => { + afterEach(() => { + storeSeenByAfterEach = storage.getStore(); + }); + }); + }); + + it("the hook saw the store; the next test does not", () => { + expect(storeSeenByAfterEach).toBe("store of the run() that registered it"); + expect(storage.getStore()).toBeUndefined(); + }); +}); diff --git a/test/js/bun/test/snapshot-tests/snapshots/snapshot.test.ts b/test/js/bun/test/snapshot-tests/snapshots/snapshot.test.ts index bef562452a65..46e7efc5e73e 100644 --- a/test/js/bun/test/snapshot-tests/snapshots/snapshot.test.ts +++ b/test/js/bun/test/snapshot-tests/snapshots/snapshot.test.ts @@ -1,7 +1,8 @@ import { $ } from "bun"; import { describe, expect, it, test } from "bun:test"; -import { readFileSync, writeFileSync } from "fs"; +import { existsSync, readFileSync, writeFileSync } from "fs"; import { bunEnv, bunExe, DirectoryTree, isDebug, tempDir, tempDirWithFiles } from "harness"; +import { join } from "path"; function test1000000(arg1: any, arg218718132: any) {} @@ -958,3 +959,243 @@ test("write snapshot from filter", async () => { expect(await Bun.file(dir + "/mytests/snap2.test.ts").text()).toBe(sver("b", true)); expect(await Bun.file(dir + "/mytests/more/testing.test.ts").text()).toBe(sver("TEST", true)); }); + +// When the runner gives up on a test or hook that is still running (it timed out, or an unhandled +// error failed it while it was still waiting), its body keeps running while the next test executes. +// The snapshot matchers it calls from then on must be rejected, not written under the next test's name. +describe("snapshot matchers called after the runner gave up on the test", () => { + const rejected = "Snapshot matchers are not supported after the test has finished executing"; + const header = "// Bun Snapshot v1, https://bun.sh/docs/test/snapshots\n"; + + // `report` logs whether each late matcher threw, and what, so stdout tells the outcomes apart. + const prelude = /* ts */ ` + import { beforeAll, beforeEach, describe, expect, test } from "bun:test"; + function report(label: string, matcher: () => void) { + try { + matcher(); + console.log(label + ": did not throw"); + } catch (error) { + console.log(label + ": " + (error as Error).message); + } + } + `; + + async function runTestFile(source: string) { + const testFile = prelude + source; + using dir = tempDir("snapshot-after-runner-moved-on", { "late.test.ts": testFile }); + await using proc = Bun.spawn({ + cmd: [bunExe(), "test", "late.test.ts"], + cwd: String(dir), + env: { ...bunEnv, CI: "false" }, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + const snapPath = join(String(dir), "__snapshots__", "late.test.ts.snap"); + const snap = existsSync(snapPath) ? readFileSync(snapPath, "utf8") : null; + // Inline snapshots are written into the test file itself once the run is over. + const testFileAfterwards = readFileSync(join(String(dir), "late.test.ts"), "utf8"); + // stdout starts with the "bun test vX.Y.Z (sha)" banner; the rest is what the test file logged. + return { stdout: stdout.replace(/^bun test v.*\n/, ""), stderr, exitCode, snap, testFile, testFileAfterwards }; + } + + test.concurrent("a test that timed out", async () => { + // Same body for both cases, as with test.each. The first case only continues once the second + // one has started, so it has always timed out by then; the second case then waits for the + // first one's late matchers before taking its own snapshots. + const { stdout, stderr, snap, testFile, testFileAfterwards } = await runTestFile(/* ts */ ` + const secondStarted = Promise.withResolvers(); + const firstDone = Promise.withResolvers(); + async function migrate(name: string) { + if (name === "first") { + await secondStarted.promise; + report("late with hint", () => expect("lockfile of " + name).toMatchSnapshot(name)); + report("late without hint", () => expect("lockfile of " + name).toMatchSnapshot()); + report("late toThrowErrorMatchingSnapshot", () => + expect(() => { + throw new Error(name); + }).toThrowErrorMatchingSnapshot(), + ); + // Block bodies: a matcher called in tail position gets located at report()'s own call to it. + report("late toMatchInlineSnapshot", () => { + expect("lockfile of " + name).toMatchInlineSnapshot(); + }); + report("late toThrowErrorMatchingInlineSnapshot", () => { + expect(() => { + throw new Error(name); + }).toThrowErrorMatchingInlineSnapshot(); + }); + firstDone.resolve(); + return; + } + secondStarted.resolve(); + await firstDone.promise; + expect("lockfile of " + name).toMatchSnapshot(name); + expect("lockfile of " + name).toMatchSnapshot(); + } + describe("migrate", () => { + test("first", () => migrate("first"), 1); + test("second", () => migrate("second")); + }); + `); + + expect(stdout).toBe( + [ + `late with hint: ${rejected}`, + `late without hint: ${rejected}`, + `late toThrowErrorMatchingSnapshot: ${rejected}`, + `late toMatchInlineSnapshot: ${rejected}`, + `late toThrowErrorMatchingInlineSnapshot: ${rejected}`, + "", + ].join("\n"), + ); + expect(stderr).toContain("this test timed out after 1ms"); + expect(stderr).toContain("(pass) migrate > second"); + // Nothing of the first case's under the second case's name, and the second case's own + // unhinted snapshot is still number 1 (on the previous behaviour the late calls pushed it to + // number 2). + expect(snap).toBe( + header + + '\nexports[`migrate second: second 1`] = `"lockfile of second"`;\n' + + '\nexports[`migrate second 1`] = `"lockfile of second"`;\n', + ); + // The late inline matchers did not get their snapshots written into the test file either. + expect(testFileAfterwards).toBe(testFile); + }); + + test.concurrent("a hook that timed out", async () => { + // The hook only applies to "first": a 1ms timeout also fails a hook that returns right away + // whenever invoking it took longer than that, which happens on a loaded machine. + const { stdout, stderr, snap } = await runTestFile(/* ts */ ` + const secondStarted = Promise.withResolvers(); + const hookDone = Promise.withResolvers(); + describe("with the hook", () => { + beforeEach(async () => { + await secondStarted.promise; + report("late from the hook", () => expect("from the hook").toMatchSnapshot("hook")); + hookDone.resolve(); + }, 1); + test("first", () => {}); + }); + test("second", async () => { + secondStarted.resolve(); + await hookDone.promise; + expect("from second").toMatchSnapshot(); + }); + `); + + expect(stdout).toBe(`late from the hook: ${rejected}\n`); + expect(stderr).toContain("(fail) with the hook > first"); + expect(stderr).toContain("(pass) second"); + expect(snap).toBe(header + '\nexports[`second 1`] = `"from second"`;\n'); + }); + + // The two cases below fail the running test with an unhandled error. They wait through a done + // callback rather than a returned promise: the runner gives up on a test that is only waiting for + // done() as soon as the error is reported (an awaited body is going to be waited for instead, + // see #36719), and that is the situation these cases are about. + test.concurrent("a test failed by an unhandled error while it was waiting for done()", async () => { + // The only snapshot matcher in the file is the late one, so nothing at all should be written: + // not even an empty snapshot file. + const { stdout, stderr, snap } = await runTestFile(/* ts */ ` + const secondStarted = Promise.withResolvers(); + const firstDone = Promise.withResolvers(); + test("first", done => { + Promise.reject(new Error("failure in the background")); + secondStarted.promise.then(() => { + report("late after the rejection", () => expect("from first").toMatchSnapshot()); + firstDone.resolve(); + done(); + }); + }); + test("second", async () => { + secondStarted.resolve(); + await firstDone.promise; + }); + `); + + expect(stdout).toBe(`late after the rejection: ${rejected}\n`); + expect(stderr).toContain("failure in the background"); + expect(stderr).toContain("(pass) second"); + expect(snap).toBeNull(); + }); + + test.concurrent("an attempt that the runner gave up on, while its retry runs", async () => { + // Both attempts run the same inline matcher. On the previous behaviour the first attempt's late + // call and the retry's call asked for different values on the same line, which fails the + // writing of the file's inline snapshots as a whole ("Multiple inline snapshots on the same + // line must all have the same value"). The first attempt never calls done(): the runner has + // given up on it, and only the retry's done() is meant to end the test. + const { stdout, stderr, snap, testFile, testFileAfterwards } = await runTestFile(/* ts */ ` + const retryStarted = Promise.withResolvers(); + const firstAttemptDone = Promise.withResolvers(); + let attempts = 0; + test( + "retried", + done => { + const attempt = ++attempts; + // Block body: a matcher called in tail position gets located at the call to inline() instead. + const inline = () => { + expect("from attempt " + attempt).toMatchInlineSnapshot(); + }; + if (attempt === 1) { + Promise.reject(new Error("first attempt fails in the background")); + retryStarted.promise.then(() => { + report("late from the first attempt", () => expect("from attempt " + attempt).toMatchSnapshot()); + report("late inline from the first attempt", inline); + firstAttemptDone.resolve(); + }); + return; + } + retryStarted.resolve(); + firstAttemptDone.promise.then(() => { + expect("from attempt " + attempt).toMatchSnapshot(); + inline(); + done(); + }); + }, + { retry: 1 }, + ); + `); + + expect(stdout).toBe(`late from the first attempt: ${rejected}\nlate inline from the first attempt: ${rejected}\n`); + expect(stderr).toContain("(pass) retried"); + expect(stderr).not.toContain("Failed to update inline snapshot"); + // The passing attempt owns "retried 1" and the inline snapshot; a later run must compare + // against its values. + expect(snap).toBe(header + '\nexports[`retried 1`] = `"from attempt 2"`;\n'); + expect(testFileAfterwards).toBe( + testFile.replace("toMatchInlineSnapshot()", 'toMatchInlineSnapshot(`"from attempt 2"`)'), + ); + }); + + test.concurrent("callbacks registered by a finished hook still snapshot under the running test", async () => { + // The server's fetch handler was registered by beforeAll, which finished normally, so its + // snapshots belong to whichever test is making the request, as before. + const { stderr, snap, exitCode } = await runTestFile(/* ts */ ` + let server: ReturnType; + beforeAll(() => { + server = Bun.serve({ + port: 0, + fetch(request) { + expect(new URL(request.url).pathname).toMatchSnapshot("requested path"); + return new Response("ok"); + }, + }); + }); + test("one", async () => { + expect(await (await fetch(server.url + "one")).text()).toBe("ok"); + }); + test("two", async () => { + expect(await (await fetch(server.url + "two")).text()).toBe("ok"); + server.stop(true); + }); + `); + + expect(stderr).toContain(" 2 pass\n"); + expect(snap).toBe( + header + '\nexports[`one: requested path 1`] = `"/one"`;\n' + '\nexports[`two: requested path 1`] = `"/two"`;\n', + ); + expect(exitCode).toBe(0); + }); +}); diff --git a/test/js/bun/test/test-on-test-finished.test.ts b/test/js/bun/test/test-on-test-finished.test.ts index de75e4734733..3bcc56d4a561 100644 --- a/test/js/bun/test/test-on-test-finished.test.ts +++ b/test/js/bun/test/test-on-test-finished.test.ts @@ -1,4 +1,5 @@ import { afterAll, afterEach, describe, expect, onTestFinished, test } from "bun:test"; +import { bunEnv, bunExe, tempDir } from "harness"; // Test the basic ordering of onTestFinished describe("onTestFinished ordering", () => { @@ -130,3 +131,56 @@ describe("onTestFinished with failing test", () => { expect(output).toEqual(["test", "onTestFinished"]); }); }); + +// A test the runner has given up on (here: it timed out) keeps running while the next test +// executes; hooks it registers from then on have no test to attach to and must be rejected rather +// than added to the test that happens to be running. +test("hooks registered by a test after the runner gave up on it are rejected", async () => { + using dir = tempDir("hooks-after-runner-moved-on", { + "late.test.ts": /* ts */ ` + import { afterAll, afterEach, onTestFinished, test } from "bun:test"; + const secondStarted = Promise.withResolvers(); + const firstDone = Promise.withResolvers(); + test("first", async () => { + await secondStarted.promise; + for (const [name, register] of [ + ["onTestFinished", onTestFinished], + ["afterEach", afterEach], + ["afterAll", afterAll], + ] as const) { + try { + register(() => console.log(name + " registered by first ran")); + console.log(name + ": registered"); + } catch (error) { + console.log(name + ": " + (error as Error).message); + } + } + firstDone.resolve(); + }, 1); + test("second", async () => { + secondStarted.resolve(); + await firstDone.promise; + }); + `, + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "test", "late.test.ts"], + cwd: String(dir), + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + const rejected = (name: string) => + `${name}: Cannot call ${name}() here. The test or hook it was called from has already finished executing (it timed out, or failed while it was still running).`; + expect(stdout.replace(/^bun test v.*\n/, "")).toBe( + [rejected("onTestFinished"), rejected("afterEach"), rejected("afterAll"), ""].join("\n"), + ); + expect(stderr).toContain("this test timed out after 1ms"); + expect(stderr).toContain("(pass) second"); + expect(stderr).toContain(" 1 pass\n"); + expect(stderr).toContain(" 1 fail\n"); + expect(exitCode).toBe(1); +});