fix(test): retain file ownership through isolation teardown

CHANGELOG.md

@@ -2,6 +2,8 @@
 
 ## Unreleased
 
+- Count uncaught errors during isolated test-file cleanup against the outgoing file and include them in JUnit; handled cleanup callbacks continue running. Adapts the file-error reporting pattern from [oven-sh/bun#44435](https://github.com/oven-sh/bun/pull/44435); thanks @robobun!
+
 - Drain queued Worker stdout and stderr before emitting `exit`, preserving output when a synchronous worker exit races the parent's stdio setup.
 
 - Avoid an extra tracing collection when destroying Workers while preserving native-resource finalizers and the mandatory final heap sweep.

src/runtime/cli/test/parallel/runner.rs

@@ -578,6 +578,11 @@ impl<'a> WorkerLoop<'a> {
             let before_unhandled = self.reporter.jest.unhandled_errors_between_tests;
             let started_ns =
                 bun_core::Timespec::now(bun_core::TimespecMockMode::ForceRealTime).ns();
+            let isolation = if vm.test_isolation_enabled {
+                test_command::FileIsolation::SwapGlobal
+            } else {
+                test_command::FileIsolation::KeepGlobal
+            };
 
             // A worker never knows which file is its last, so preload-level hooks wrap every file (with or without --isolate).
             if let Err(err) = TestCommand::run(
@@ -588,17 +593,11 @@ impl<'a> WorkerLoop<'a> {
                     first: true,
                     last: true,
                 },
+                isolation,
             ) {
                 test_command::handle_top_level_test_error_before_javascript_start(&err);
             }
-            if vm.test_isolation_enabled {
-                crate::jsc_hooks::stop_active_handles_for_test_isolation(vm);
-                vm.swap_global_for_test_isolation();
-                self.reporter
-                    .jest
-                    .bun_test_root
-                    .reset_hook_scope_for_test_isolation();
-            } else {
+            if !vm.test_isolation_enabled {
                 Global::mimalloc_cleanup(false);
             }
             self.reporter.jest.default_timeout_override = u32::MAX;

src/runtime/cli/test_command.rs

@@ -374,9 +374,9 @@ impl TestFailure {
 
     /// VirtualMachine::on_print_error_zig_exception thunk.
     pub(crate) fn record_cb(ctx: *mut core::ffi::c_void, exception: &jsc::ZigException) {
-        // SAFETY: `ctx` was set to `&mut CommandLineReporter.test_failure` by
-        // `on_uncaught_exception` for the duration of a single
-        // `run_error_handler` call; single-threaded, no other borrow live.
+        // SAFETY: `on_uncaught_exception` points `ctx` at an Option<TestFailure>
+        // for one synchronous `run_error_handler` call, then clears the callback.
+        // The slot is either the reporter's test failure or a local file failure.
         let slot = unsafe { &mut *ctx.cast::<Option<TestFailure>>() };
         TestFailure::record(slot, exception);
     }
@@ -1261,6 +1261,30 @@ impl CommandLineReporter {
         &mut self.jest.summary
     }
 
+    /// The uncaught hook already counted this error. Attach it to its file
+    /// without turning the last completed test into a failure.
+    pub(crate) fn handle_unhandled_file_error(
+        &mut self,
+        file_id: jest::FileId,
+        failure: Option<TestFailure>,
+    ) {
+        let report = TestCaseReport {
+            file: junit_file_name(self.jest.files.items_source()[file_id as usize].path.text),
+            scopes: Vec::new(),
+            name: b"(unhandled error)",
+            status: bun_test::Execution::Result::Fail,
+            assertions: 0,
+            elapsed_ns: 0,
+            line_number: 0,
+            failure,
+        };
+        if let Some(idx) = self.worker_ipc_file_idx {
+            ParallelRunner::worker_emit_test_done(idx, b"", Some(&report));
+        } else if let Some(junit) = self.reporters.junit.as_mut() {
+            junit.record_test_case(&report).expect("oom");
+        }
+    }
+
     pub(crate) fn handle_test_completed(
         buntest: &mut bun_test::BunTest,
         sequence: &mut bun_test::Execution::ExecutionSequence,
@@ -1730,6 +1754,12 @@ extern "C" fn BunTest__shouldGenerateCodeCoverage(test_name_str: &bun_core::Stri
     true
 }
 
+#[derive(Clone, Copy, PartialEq, Eq)]
+pub(crate) enum FileIsolation {
+    KeepGlobal,
+    SwapGlobal,
+}
+
 pub(crate) struct TestCommand;
 
 impl TestCommand {
@@ -2759,6 +2789,11 @@ impl TestCommand {
                                 first: isolate || i == 0,
                                 last: isolate,
                             },
+                            if isolate {
+                                FileIsolation::SwapGlobal
+                            } else {
+                                FileIsolation::KeepGlobal
+                            },
                         ) {
                             handle_top_level_test_error_before_javascript_start(&err);
                         }
@@ -2767,14 +2802,6 @@ impl TestCommand {
                         }
                         reporter.jest.default_timeout_override = u32::MAX;
                         Global::mimalloc_cleanup(false);
-                        if isolate {
-                            crate::jsc_hooks::stop_active_handles_for_test_isolation(vm);
-                            vm.swap_global_for_test_isolation();
-                            reporter
-                                .jest
-                                .bun_test_root
-                                .reset_hook_scope_for_test_isolation();
-                        }
                     }
                 }
 
@@ -2788,6 +2815,7 @@ impl TestCommand {
                         first: isolate || files.len() == 1,
                         last: true,
                     },
+                    FileIsolation::KeepGlobal,
                 ) {
                     handle_top_level_test_error_before_javascript_start(&err);
                 }
@@ -2819,6 +2847,7 @@ impl TestCommand {
         vm: &mut VirtualMachine,
         file_name: &[u8],
         first_last: bun_test::FirstLast,
+        isolation: FileIsolation,
     ) -> crate::Result<()> {
         // Capture the raw log pointer (Copy) so the guard does not borrow `vm`.
         let vm_log = vm.log;
@@ -2873,6 +2902,13 @@ impl TestCommand {
 
         let repeat_count = reporter.repeat_count;
         let mut repeat_index: u32 = 0;
+        scopeguard::defer! {
+            // SAFETY: the caller-owned reporter outlives the run and every
+            // per-repeat cleanup guard below. Teardown errors are recorded first.
+            if let Some(junit) = unsafe { &mut *reporter_ptr }.reporters.junit.as_mut() {
+                let _ = junit.end_file(None);
+            }
+        }
         vm.on_unhandled_rejection_ctx = None;
         vm.on_unhandled_rejection = jest::on_unhandled_rejection::on_unhandled_rejection;
 
@@ -2893,13 +2929,45 @@ impl TestCommand {
             bun_test_root.enter_file(file_id, reporter, should_run_concurrent, first_last);
             let bun_test_root_ptr: *mut bun_test::BunTestRoot = bun_test_root;
             let global = vm.global();
+            let vm_ptr: *mut VirtualMachine = vm;
+            // An early return also finalizes the file; successful intermediate
+            // reruns only release their BunTest, retaining the current global.
+            let finalize_file = core::cell::Cell::new(true);
             scopeguard::defer! {
+                let swap = finalize_file.get() && isolation == FileIsolation::SwapGlobal;
+                if swap {
+                    // SAFETY: the root and active file remain live through this guard.
+                    // End collection/execution before callbacks can re-enter the scheduler.
+                    if let Some(active) = unsafe { (*bun_test_root_ptr).clone_active_file() } {
+                        active.get().begin_file_teardown();
+                    }
+                }
+                // A pending patch must be forgotten before its global is retired.
+                bun_jsc::cpp::JSMock__forgetPendingModulePatches(global);
+                if swap {
+                    // SAFETY: the caller's VM remains live on this JS thread.
+                    // Keep the active file and reporter through the existing close
+                    // callbacks so uncaught teardown errors belong to this file.
+                    let vm = unsafe { &mut *vm_ptr };
+                    crate::jsc_hooks::stop_active_handles_for_test_isolation(vm);
+                    vm.swap_global_for_test_isolation();
+                }
                 // SAFETY: `bun_test_root` is `&'static mut` from `Jest::runner()`;
                 // raw-ptr escape so the closure does not hold a borrowck lock on
                 // it for the loop body.
                 unsafe { (*bun_test_root_ptr).exit_file(); }
-                // A mock.module() patch still pending must not hold up the next file.
-                bun_jsc::cpp::JSMock__forgetPendingModulePatches(global);
+                if swap {
+                    // SAFETY: exit_file and the loop's local Rc releases precede
+                    // dropping the old preload scopes those BunTests referenced.
+                    unsafe { (*bun_test_root_ptr).reset_hook_scope_for_test_isolation(); }
+                }
+                if Output::is_github_action()
+                    // SAFETY: reporter is caller-owned and remains live through this guard.
+                    && unsafe { (*reporter_ptr).worker_ipc_file_idx.is_none() }
+                {
+                    pretty_errorln!("<r>\n::endgroup::\n");
+                    Output::flush();
+                }
             }
 
             // SAFETY: `set()` reads only `reporter.{worker_ipc_file_idx, reporters}`
@@ -3043,11 +3111,6 @@ impl TestCommand {
 
             let _ = vm.global().handle_rejected_promises();
 
-            if Output::is_github_action() && reporter.worker_ipc_file_idx.is_none() {
-                pretty_errorln!("<r>\n::endgroup::\n");
-                Output::flush();
-            }
-
             if !vm.test_isolation_enabled {
                 // Ensure these never linger across files. Under --isolate this
                 // is done by swapGlobalForTestIsolation() (kill+clear) and we
@@ -3057,9 +3120,7 @@ impl TestCommand {
             }
 
             repeat_index += 1;
-        }
-        if let Some(junit) = reporter.reporters.junit.as_mut() {
-            let _ = junit.end_file(None);
+            finalize_file.set(repeat_index == repeat_count);
         }
         Ok(())
     }

src/runtime/test_runner/bun_test.rs

@@ -903,6 +903,17 @@ impl BunTest {
         let _ = self.result_queue.write_item(result); // OOM/capacity: fire-and-forget
     }
 
+    /// Retain error attribution while closing a file, including one whose load
+    /// failed before execution. Close callbacks must not resume its tests or hooks.
+    pub(crate) fn begin_file_teardown(&mut self) {
+        debug_assert!(!self.in_run_loop);
+        self.phase = Phase::Done;
+        self.wants_wakeup = false;
+        if self.timer.state == EventLoopTimerState::ACTIVE {
+            vm_timer().remove(&raw mut self.timer);
+        }
+    }
+
     pub(crate) fn run(this_strong: &BunTestPtr, global_this: &JSGlobalObject) -> JsResult<()> {
         let _g = group_begin!();
         // `Collection::step` / `Execution::step` re-enter and call `.get()` on
@@ -1287,17 +1298,20 @@ impl BunTest {
             return; // the exception should not be visible (eg m_terminationException)
         };
 
+        let mut file_failure = None;
+        let is_file_error = self.phase == Phase::Done;
         let failure_ctx: *mut core::ffi::c_void = 'ctx: {
-            if handle_status != HandleUncaughtExceptionResult::ShowHandledError {
-                break 'ctx core::ptr::null_mut();
-            }
             let Some(reporter) = self.reporter else {
                 break 'ctx core::ptr::null_mut();
             };
             // SAFETY: `BunTest.reporter` carries write provenance from `enter_file`'s
             // `&mut`; single-threaded test runner, no other borrow live here.
             let reporter = unsafe { &mut *reporter.as_ptr() };
-            if reporter.jest.test_options.reporters.junit {
+            if !reporter.jest.test_options.reporters.junit {
+                core::ptr::null_mut()
+            } else if is_file_error {
+                core::ptr::from_mut(&mut file_failure).cast()
+            } else if handle_status == HandleUncaughtExceptionResult::ShowHandledError {
                 core::ptr::from_mut(&mut reporter.test_failure).cast()
             } else {
                 core::ptr::null_mut()
@@ -1343,6 +1357,14 @@ impl BunTest {
         }
 
         Output::flush();
+        if is_file_error && !failure_ctx.is_null() {
+            // SAFETY: the active file retains the caller-owned reporter through
+            // teardown. The formatter callback was cleared before moving its slot.
+            unsafe {
+                (*self.reporter.unwrap().as_ptr())
+                    .handle_unhandled_file_error(self.file_id, file_failure);
+            }
+        }
     }
 }
 
@@ -1974,4 +1996,3 @@ pub(crate) use super::execution as Execution;
 pub(crate) use super::debug;
 pub(crate) use super::scope_functions as ScopeFunctions;
 pub(crate) use super::order as Order;
-