diff --git a/scripts/runner.node.mjs b/scripts/runner.node.mjs index 2b433d7cd4fc..5b4f9aed33e3 100755 --- a/scripts/runner.node.mjs +++ b/scripts/runner.node.mjs @@ -786,9 +786,6 @@ async function runTests() { FORCE_COLOR: "0", NO_COLOR: "1", BUN_DEBUG_QUIET_LOGS: "1", - // Node parity: a node test process exits only when its event loop - // drains, and common.mustCall() verifies counts in 'exit' handlers. - BUN_TEST_DRAIN_EVENT_LOOP: "1", }; if (title.includes("test-util-styletext")) { // These assert styleText's own color decisions against a TTY, so they need a diff --git a/src/bun_core/env_var.rs b/src/bun_core/env_var.rs index c1d94863c740..25cc3222e26f 100644 --- a/src/bun_core/env_var.rs +++ b/src/bun_core/env_var.rs @@ -112,10 +112,6 @@ platform_specific_new!(pub C_INCLUDE_PATH: string, posix = "C_INCLUDE_PATH", win // Standard C compiler environment variable for library paths (colon-separated). // Used by bun:ffi's TinyCC integration for systems like NixOS. platform_specific_new!(pub LIBRARY_PATH: string, posix = "LIBRARY_PATH", windows = None, {}); -// Drain the event loop after a file's tests finish so node-style -// `process.on('exit')` checks (e.g. common.mustCall) see completed async work. -// Opt-in for the vendored node:test suite and run() children. -new!(pub BUN_TEST_DRAIN_EVENT_LOOP: boolean, "BUN_TEST_DRAIN_EVENT_LOOP", { default: false }); new!(pub BUN_TMPDIR: string, "BUN_TMPDIR", {}); new!(pub BUN_TRACY_PATH: string, "BUN_TRACY_PATH", {}); new!(pub BUN_WATCHER_TRACE: string, "BUN_WATCHER_TRACE", {}); diff --git a/src/js/node/test.ts b/src/js/node/test.ts index 75abf5fb79e3..69d3cb09ae04 100644 --- a/src/js/node/test.ts +++ b/src/js/node/test.ts @@ -476,7 +476,7 @@ async function runOneFile( const proc = Bun.spawn({ cmd: args, cwd: opts.cwd as string, - env: { ...(opts.env ?? process.env), BUN_TEST_DRAIN_EVENT_LOOP: "1", [kRunChildEnv]: kRunChildEnvValue }, + env: { ...(opts.env ?? process.env), [kRunChildEnv]: kRunChildEnvValue }, stdout: "pipe", stderr: "pipe", signal: opts.signal, diff --git a/src/runtime/cli/test_command.rs b/src/runtime/cli/test_command.rs index c3d0481ee43a..ab4fd8b7d81a 100644 --- a/src/runtime/cli/test_command.rs +++ b/src/runtime/cli/test_command.rs @@ -900,14 +900,6 @@ impl JunitReporter { } } -/// Drain the event loop after a file's tests finish, like a node process -/// would before exiting; the vendored-node-test runner opts in via -/// `BUN_TEST_DRAIN_EVENT_LOOP=1` so mustCall()-style exit checks see -/// completed async work. Off by default: bun suites keep exit-after-tests. -pub(crate) fn should_drain_event_loop() -> bool { - env_var::BUN_TEST_DRAIN_EVENT_LOOP.get().unwrap_or(false) -} - pub struct CommandLineReporter { // `TestRunner<'a>` borrows `TestOptions`/regex from the CLI ctx; the // reporter is held in a `Box` local to `TestCommand::exec` which never @@ -2250,6 +2242,7 @@ impl TestCommand { test_options: unsafe { bun_ptr::detach_lifetime_ref(&ctx.test_options) }, unhandled_errors_between_tests: 0, summary: Summary::default(), + node_test_module_used: false, }, last_dot: 0, repeat_count: 1, @@ -3053,18 +3046,41 @@ impl TestCommand { { vm.exit_handler.exit_code = 1; } - // Run `process.on('exit')` handlers like `bun run` does. Node's test - // harness verifies mustCall() counts from one, so skipping them made - // those assertions silently pass. Must precede the GC-root release - // below: handlers are user JS and may touch still-live state. - { + if reporter.jest.node_test_module_used { + // Node parity: a node test process exits only when its loop drains, + // and node's test harness verifies mustCall() counts from a + // `process.on('exit')` handler. `on_before_exit()` spins while + // `is_event_loop_alive()`, which cannot tell one file's handles + // from another's; gate the drain on single-file runs (the vendored + // node suite and run() children both spawn one file per process) so + // a leaked handle from an earlier file cannot wedge a mixed + // multi-file run. Must precede the GC-root release below; handlers + // are user JS and may touch still-live state. + let drain = reporter.summary().files <= 1; let vm_ptr: *mut VirtualMachine = vm; // SAFETY: `vm_ptr` reborrows the live `&mut VirtualMachine`; // `run_with_api_lock` takes `&self` only, so the closure holds the // unique mutable access on this single-threaded path. - vm.run_with_api_lock(|| unsafe { (*vm_ptr).on_exit() }); + vm.run_with_api_lock(|| unsafe { + let prev_unhandled = (*vm_ptr).unhandled_error_counter; + if drain { + (*vm_ptr).on_before_exit(); + } + (*vm_ptr).global().handle_rejected_promises(); + // The drain runs after `active_file` is cleared and after the + // exit-code decision above, so an uncaught throw it surfaces + // prints but reaches neither; propagate it here. A throw from + // inside an `'exit'` listener stays outside this check to keep + // main's existing behavior for `bun test`. + if (*vm_ptr).unhandled_error_counter > prev_unhandled { + (*vm_ptr).exit_handler.exit_code = 1; + } + (*vm_ptr).on_exit(); + }); + // on_exit() already set is_shutting_down; global_exit() asserts it. + } else { + vm.is_shutting_down = true; } - // on_exit() already set is_shutting_down; global_exit() asserts it. // Release `bun:test` GC roots before `global_exit()` so // `destructOnExit()`'s `collectNow()` can reach the closures they pin // (preload hooks, per-file describe/test callbacks). Clear `RUNNER` @@ -3377,14 +3393,6 @@ impl TestCommand { let el = vm.event_loop(); // SAFETY: el is the VM-owned event loop; vm is passed back as *mut. unsafe { (*el).tick_immediate_tasks(vm) }; - - // Node parity: a node test file exits only when its loop drains. - // on_before_exit() drains and dispatches 'beforeExit' like `bun run`; - // it early-returns when unhandled_error_counter > 0, which is fine - // here since such a file already failed. Opt-in; one file per process. - if should_drain_event_loop() { - vm.on_before_exit(); - } drop(buntest_strong); } diff --git a/src/runtime/test_runner/jest.rs b/src/runtime/test_runner/jest.rs index 1f25d9c376cc..20362ff0f8b4 100644 --- a/src/runtime/test_runner/jest.rs +++ b/src/runtime/test_runner/jest.rs @@ -146,6 +146,11 @@ pub struct TestRunner<'a> { pub unhandled_errors_between_tests: u32, pub summary: Summary, + /// Set the first time `node:test` registers something in this process. + /// Gates node-parity shutdown (event-loop drain, `process.on('exit')` + /// handlers) so bun:test-only runs keep exit-after-tests. + pub node_test_module_used: bool, + pub bun_test_root: bun_test::BunTestRoot, } @@ -523,8 +528,13 @@ pub(crate) fn js_file_generation( // registration, and an exclusive `&mut TestRunner` would invalidate the // `bun_test_root` pointer `test_command.rs` keeps live across the file run. // SAFETY: same invariant as `runner()` — RUNNER is only read on the JS thread. - let generation = - Jest::runner_ptr().map_or(0, |p| unsafe { (*p.as_ptr()).bun_test_root.file_generation }); + let generation = Jest::runner_ptr().map_or(0, |p| unsafe { + let runner = p.as_ptr(); + // `node_test_module_used` is disjoint from `bun_test_root`; the field + // projection does not overlap the caller's live `&BunTestRoot`. + (*runner).node_test_module_used = true; + (*runner).bun_test_root.file_generation + }); Ok(JSValue::from(generation)) } diff --git a/test/cli/test/bun-test.test.ts b/test/cli/test/bun-test.test.ts index 292564167e2f..c2b6436cc065 100644 --- a/test/cli/test/bun-test.test.ts +++ b/test/cli/test/bun-test.test.ts @@ -1457,8 +1457,8 @@ describe("bun test", () => { expect(output).toContain("app message"); }); - test("runs process.on('exit') handlers", async () => { - using dir = tempDir("bun-test-exit-handler", { + test("bun:test does not run process.on('exit') handlers", async () => { + using dir = tempDir("bun-test-no-exit-handler", { "exit.test.ts": ` import { test } from "bun:test"; process.on("exit", () => console.log("exit handler ran")); @@ -1473,16 +1473,92 @@ describe("bun test", () => { stderr: "pipe", }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stdout).not.toContain("exit handler ran"); + expect(stderr).toContain("1 pass"); + expect(exitCode).toBe(0); + }); + + test("node:test runs process.on('exit') handlers", async () => { + using dir = tempDir("bun-test-node-exit-handler", { + "exit.test.ts": ` + import { test } from "node:test"; + process.on("exit", () => console.log("exit handler ran")); + test("a test", () => {}); + `, + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "test", "exit.test.ts"], + env: bunEnv, + cwd: String(dir), + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); expect(stdout).toContain("exit handler ran"); expect(stderr).toContain("1 pass"); expect(exitCode).toBe(0); }); - test("an exit handler can fail the run, like node's common.mustCall()", async () => { - using dir = tempDir("bun-test-exit-handler-code", { - "exit-code.test.ts": ` + test("node:test in a multi-file run does not hang on a handle an earlier bun:test file leaked", async () => { + using dir = tempDir("bun-test-node-multi-file-drain", { + "a.test.ts": ` import { test } from "bun:test"; + setInterval(() => {}, 100); + test("a", () => {}); + `, + "b.test.ts": ` + import { test } from "node:test"; + process.on("exit", () => console.log("exit handler ran")); + test("b", () => {}); + `, + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "test", "a.test.ts", "b.test.ts"], + env: bunEnv, + cwd: String(dir), + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stdout).toContain("exit handler ran"); + expect(stderr).toContain("2 pass"); + expect(exitCode).toBe(0); + }); + + test("node:test: an uncaught throw surfaced by the event-loop drain fails the run", async () => { + using dir = tempDir("bun-test-node-drain-throw", { + "drain-throw.test.ts": ` + import { test } from "node:test"; + test("a", () => { + // Arm from inside the body so the timer is due after the per-file + // loop exits; the end-of-run drain is the first place that ticks it. + setTimeout(() => { throw new Error("boom") }, 20); + }); + process.on("exit", code => console.log("exit code seen:", code)); + `, + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "test", "drain-throw.test.ts"], + env: bunEnv, + cwd: String(dir), + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toContain("boom"); + expect(stdout).toContain("exit code seen: 1"); + expect(stderr).toContain("1 pass"); + expect(exitCode).toBe(1); + }); + + test("node:test: an exit handler can fail the run, like node's common.mustCall()", async () => { + using dir = tempDir("bun-test-node-exit-handler-code", { + "exit-code.test.ts": ` + import { test } from "node:test"; process.on("exit", () => process.exit(1)); test("a passing test", () => {}); `,