Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions scripts/runner.node.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 0 additions & 4 deletions src/bun_core/env_var.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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", {});
Expand Down
2 changes: 1 addition & 1 deletion src/js/node/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,7 @@
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 },

Check warning on line 479 in src/js/node/test.ts

View check run for this annotation

Claude / Claude Code Review

run() children that don't import node:test lose drain and exit handlers

Dropping `BUN_TEST_DRAIN_EVENT_LOOP` from the `runOneFile` env means a `run()` child whose target file never imports `node:test` (e.g. a plain `common.mustCall()` + async-callback script) no longer drains its event loop or dispatches `process.on('exit')` — `node_test_module_used` is only set via `getRootNode()`, which such a file never reaches. Node's `run({isolation:'process'})` runs each file as a full node process that always drains and fires `'exit'` regardless of whether it imported `node:t
Comment thread
robobun marked this conversation as resolved.
stdout: "pipe",
stderr: "pipe",
signal: opts.signal,
Expand Down
33 changes: 16 additions & 17 deletions src/runtime/cli/test_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -900,14 +900,6 @@
}
}

/// 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
Expand Down Expand Up @@ -2250,6 +2242,7 @@
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,
Expand Down Expand Up @@ -3053,18 +3046,23 @@
{
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 {
// 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.
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 {
(*vm_ptr).global().handle_rejected_promises();
(*vm_ptr).on_exit();
});
Comment thread
robobun marked this conversation as resolved.
// on_exit() already set is_shutting_down; global_exit() asserts it.
} else {
vm.is_shutting_down = true;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
// 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`
Expand Down Expand Up @@ -3381,10 +3379,11 @@
// 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() {
// here since such a file already failed. bun:test-only files keep
// exit-after-tests.
if reporter.jest.node_test_module_used {
vm.on_before_exit();
}

Check failure on line 3386 in src/runtime/cli/test_command.rs

View check run for this annotation

Claude / Claude Code Review

Per-file on_before_exit() drain is unsound in multi-file bun test runs

The per-file `on_before_exit()` drain at line 3384 is unsound in multi-file `bun test` runs: `node_test_module_used` is process-lifetime state that is never reset between files, and even when correctly set for a node:test file, the drain's `while is_event_loop_alive()` loop sees ref'd handles left by *earlier* bun:test files (a leaked `setInterval`, an unclosed `Bun.serve`) and hangs the whole process. The removed env var was explicitly "Opt-in; one file per process" — its only setters (`runner.
Comment thread
robobun marked this conversation as resolved.
Outdated
drop(buntest_strong);
}

Expand Down
14 changes: 12 additions & 2 deletions src/runtime/test_runner/jest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}

Expand Down Expand Up @@ -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))
}

Expand Down
32 changes: 27 additions & 5 deletions test/cli/test/bun-test.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
Expand All @@ -1473,16 +1473,38 @@ 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", {
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 "bun:test";
import { test } from "node:test";
process.on("exit", () => process.exit(1));
test("a passing test", () => {});
`,
Expand Down
Loading