Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
7 changes: 3 additions & 4 deletions src/runtime/cli/test/parallel/runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -306,10 +306,9 @@ fn build_worker_argv(ctx: &Command::ContextData) -> crate::Result<Box<[bun_spawn
if let Some(seed) = opts.seed {
argv.push(print_z(format_args!("--seed={}", seed))?);
}
// --bail is intentionally NOT forwarded: workers Global.exit(1) on bail
// (see test_command.rs handle_test_completed), which the coordinator would
// misread as a crash. Cross-worker bail is handled at file granularity by
// the coordinator instead.
// --bail is not forwarded: a worker would exit(1) on bail (test_command.rs
// `bail_out`), which the coordinator would misread as a crash; the
// coordinator bails at file granularity itself.
Comment thread
robobun marked this conversation as resolved.
if opts.repeat_count > 0 {
argv.push(print_z(format_args!("--rerun-each={}", opts.repeat_count))?);
}
Expand Down
77 changes: 40 additions & 37 deletions src/runtime/cli/test_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1517,16 +1517,7 @@ impl CommandLineReporter {
this.summary().fail += 1;

if this.summary().fail == this.jest.bail {
this.print_summary();
pretty_error!(
"\nBailed out after {} failure{}<r>\n",
this.jest.bail,
if this.jest.bail == 1 { "" } else { "s" }
);
Output::flush();
this.write_junit_report_if_needed();
this.write_timings_if_needed();
Global::exit(1);
this.bail_out(VirtualMachine::get().as_mut());
}
}
}
Expand All @@ -1552,6 +1543,42 @@ impl CommandLineReporter {
Output::print_start_end(bun::start_time(), bun::time::nano_timestamp());
}

/// `--bail` reached its failure count: report, then exit 1 through the
/// same VM teardown as the end of a normal run (`exec`). A bare
/// `Global::exit` would skip the `BUN_DESTRUCT_VM_ON_EXIT` teardown and
/// leave every JSC-finalizer-owned wrapper box for the leak checker.
Comment thread
robobun marked this conversation as resolved.
pub(crate) fn bail_out(&mut self, vm: &mut VirtualMachine) -> ! {
self.print_summary();
pretty_error!(
"\nBailed out after {} failure{}<r>\n",
self.jest.bail,
if self.jest.bail == 1 { "" } else { "s" }
);
Output::flush();
self.write_junit_report_if_needed();
self.write_timings_if_needed();

vm.exit_handler.exit_code = 1;
vm.exit_handler.skip_exit_listeners = skip_exit_listeners(self);
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() });
// Exit listeners (user JS) ran above while the runner was intact; now
// release the `bun:test` GC roots so the teardown GC can't observe a
// half-torn-down `TestRunner`.
Comment thread
robobun marked this conversation as resolved.
self.jest.bun_test_root.deinit_for_exit();
// SAFETY: `RUNNER` is a `RacyCell` touched only from the single JS
// thread; no concurrent reader exists on this shutdown path.
unsafe {
jest::Jest::RUNNER.write(None);
}
// SAFETY: as above; `global_exit()` diverges, so the closure is the
// sole mutator.
vm.run_with_api_lock(|| unsafe { (*vm_ptr).global_exit() })
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

/// Like the JUnit report, called before every exit path (including bail) so measured durations aren't lost.
pub(crate) fn write_timings_if_needed(&mut self) {
if self.jest.test_options.update_timings
Expand Down Expand Up @@ -3294,33 +3321,9 @@ impl TestCommand {
reporter.summary().fail += 1;

if reporter.jest.bail == reporter.summary().fail {
reporter.print_summary();
pretty_error!(
"\nBailed out after {} failure{}<r>\n",
reporter.jest.bail,
if reporter.jest.bail == 1 { "" } else { "s" }
);
reporter.write_junit_report_if_needed();
reporter.write_timings_if_needed();

vm.exit_handler.exit_code = 1;
vm.is_shutting_down = true;
// `global_exit()` diverges, so the `exit_file()` defer
// above never fires. Release the active file's
// `Strong`s and the preload-hook scope here so
// `Zig__GlobalObject__destructOnExit()`'s `collectNow()` can reclaim them,
// then clear `RUNNER` so finalizers can't observe a
// partially-torn-down `TestRunner`.
// SAFETY: single-threaded; raw-ptr reborrow mirrors the
// defer's escape.
unsafe {
(*bun_test_root_ptr).deinit_for_exit();
jest::Jest::RUNNER.write(None);
}
let vm_ptr = std::ptr::from_mut::<VirtualMachine>(vm);
// SAFETY: global_exit diverges; `vm_ptr` is a fresh
// raw-ptr reborrow of the exclusive `vm` borrow.
unsafe { (*vm_ptr).run_with_api_lock(|| (&mut *vm_ptr).global_exit()) };
// Diverges, so the `exit_file()` defer above never
// fires; `bail_out` releases the active file itself.
Comment thread
robobun marked this conversation as resolved.
reporter.bail_out(vm);
}

return Ok(());
Expand Down
203 changes: 202 additions & 1 deletion test/cli/test/bun-test.test.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,21 @@
import { spawnSync } from "bun";
import { beforeAll, describe, expect, it, test } from "bun:test";
import { bunEnv, bunExe, tempDir, tempDirWithFiles, tmpdirSync } from "harness";
import { bunEnv, bunExe, isASAN, tempDir, tempDirWithFiles, tmpdirSync } from "harness";
import { mkdirSync, rmSync, writeFileSync } from "node:fs";
import { dirname, join, resolve } from "node:path";

// The environment the sanitizer lane (scripts/runner.node.mjs) runs test files under, which their
// bunEnv children inherit. This file is exempted in test/no-validate-leaksan.txt, so the children
// that need it get it explicitly. Under it, an exit that skips the VM teardown leaves the objects
// the bun:test finalizers own allocated, and LeakSanitizer aborts the process (exit 134) in place
// of the exit code the run meant to return.
const leakCheckEnv = {
...bunEnv,
BUN_DESTRUCT_VM_ON_EXIT: "1",
ASAN_OPTIONS: "allow_user_segv_handler=1:disable_coredump=0:detect_leaks=1:abort_on_error=1",
LSAN_OPTIONS: `malloc_context_size=30:print_suppressions=0:suppressions=${join(import.meta.dir, "../../leaksan.supp")}`,
};

describe("bun test", () => {
test("running a non-existent absolute file path is a 1 exit code", () => {
const spawn = Bun.spawnSync({
Expand Down Expand Up @@ -329,6 +341,7 @@ describe("bun test", () => {
expect(true).toBe(true);
});
`,
expectExitCode: 1,
});
expect(stderr).toContain("Bailed out after 1 failure");
expect(stderr).not.toContain("test #2");
Expand All @@ -352,11 +365,160 @@ describe("bun test", () => {
expect(true).toBe(true);
});
`,
expectExitCode: 1,
});
expect(stderr).toContain("Bailed out after 3 failures");
expect(stderr).not.toContain("test #4");
});

// Bailing out has to exit through the same VM teardown as the end of a normal run (see
// leakCheckEnv), not through a bare exit.
describe.concurrent.skipIf(!isASAN)("exits cleanly under LeakSanitizer", () => {
/** Runs `bun test --bail` and returns what the test files logged to stdout. */
async function runUntilBail(files: Record<string, string>, ...args: string[]): Promise<string[]> {
using dir = tempDir("bun-test-bail-leak-check", files);
await using proc = Bun.spawn({
cmd: [bunExe(), "test", "--bail", ...args],
env: leakCheckEnv,
cwd: String(dir),
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stderr).toContain("Bailed out after 1 failure");
expect(stderr).not.toContain("LeakSanitizer");
expect({ exitCode, signalCode: proc.signalCode }).toEqual({ exitCode: 1, signalCode: null });
// Everything else on stdout is the "bun test vX.Y.Z" banner.
return stdout.split("\n").filter(line => line.startsWith("ran: "));
}

test("after a test fails", async () => {
const ran = await runUntilBail({
"a.test.ts": `
import { test, expect } from "bun:test";
test("fails", () => {
console.log("ran: fails");
expect(1).toBe(2);
});
test("unreachable", () => {
console.log("ran: unreachable");
});
`,
});
expect(ran).toEqual(["ran: fails"]);
});

// The failure arrives from a promise reaction here, so the exit starts inside a JS frame.
test("after an async test fails while a timer is still pending", async () => {
const ran = await runUntilBail({
"a.test.ts": `
import { test, expect } from "bun:test";
test("fails", async () => {
setTimeout(() => console.log("ran: timer"), 60_000);
await Bun.sleep(1);
console.log("ran: fails");
expect(1).toBe(2);
});
test("unreachable", () => {
console.log("ran: unreachable");
});
`,
});
expect(ran).toEqual(["ran: fails"]);
});

test("after a later file fails, with preload hooks registered", async () => {
const ran = await runUntilBail(
{
"preload.ts": `
import { afterEach, beforeAll, beforeEach } from "bun:test";
beforeAll(() => console.log("ran: preload beforeAll"));
beforeEach(() => {});
afterEach(() => {});
`,
"a.test.ts": `
import { describe, test, expect } from "bun:test";
describe("a", () => {
test("passes", () => {
console.log("ran: a");
expect(1).toBe(1);
});
});
`,
"b.test.ts": `
import { describe, test, expect } from "bun:test";
describe("b", () => {
test("fails", () => {
console.log("ran: b");
expect(1).toBe(2);
});
});
`,
},
"--preload=./preload.ts",
"./a.test.ts",
"./b.test.ts",
);
expect(ran).toEqual(["ran: preload beforeAll", "ran: a", "ran: b"]);
});

test("after a file fails to load", async () => {
const ran = await runUntilBail(
{
"a.test.ts": `
import { test, expect } from "bun:test";
test("passes", () => {
console.log("ran: a");
expect(1).toBe(1);
});
`,
"b.test.ts": `
console.log("ran: b (load)");
throw new Error("b failed to load");
`,
},
"./a.test.ts",
"./b.test.ts",
);
expect(ran).toEqual(["ran: a", "ran: b (load)"]);
});
});
});

// Without --bail a failing run exits through the end of the run (and, under --parallel, through
// the worker's exit path). Printing the failure is what allocates the source map entry the bail
// exit used to leak, so these pin down that both paths tear the VM down too.
describe.concurrent.skipIf(!isASAN)("a failing run exits 1 cleanly under LeakSanitizer", () => {
test.each([
["serial", []],
["--parallel", ["--parallel=2"]],
])("%s", async (_, args) => {
using dir = tempDir("bun-test-fail-leak-check", {
"a.test.ts": `
import { test, expect } from "bun:test";
test("fails", () => expect(1).toBe(2));
`,
"b.test.ts": `
import { test, expect } from "bun:test";
test("passes", () => expect(1).toBe(1));
`,
});
await using proc = Bun.spawn({
cmd: [bunExe(), "test", ...args],
env: leakCheckEnv,
cwd: String(dir),
stdout: "ignore",
stderr: "pipe",
});
const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]);
// A --parallel worker aborting does not change the coordinator's exit code; the workers'
// stderr is inherited, so the report check is what covers the worker exit path.
expect(stderr).not.toContain("LeakSanitizer");
expect(stderr).toContain("Ran 2 tests across 2 files.");
expect({ exitCode, signalCode: proc.signalCode }).toEqual({ exitCode: 1, signalCode: null });
});
});

describe("--timeout", () => {
test("must provide a number timeout", () => {
const stderr = runTest({
Expand Down Expand Up @@ -1708,6 +1870,45 @@ describe("bun test", () => {
expect(exitCode).toBe(1);
});

// --bail exits through the same on_exit() as the end of a run, so the same gate applies.
test("are not run when --bail stops a bun:test file", async () => {
const { stdout, stderr, exitCode } = await runFiles(
{
"bail.test.ts": `
import { test, expect } from "bun:test";
process.on("exit", () => {
console.log("exit listener ran");
process.exit(7);
});
test("fails", () => expect(1).toBe(2));
`,
},
"--bail",
"bail.test.ts",
);
expect(stdout).not.toContain("exit listener ran");
expect(stderr).toContain("Bailed out after 1 failure");
expect(exitCode).toBe(1);
});

test("run when --bail stops a file that registered a node:test test", async () => {
const { stdout, stderr, exitCode } = await runFiles(
{
"node-bail.test.ts": `
import { test } from "node:test";
import assert from "node:assert";
process.on("exit", code => console.log("exit listener ran with", code));
test("fails", () => assert.strictEqual(1, 2));
`,
},
"--bail",
"node-bail.test.ts",
);
expect(stdout).toContain("exit listener ran with 1");
expect(stderr).toContain("Bailed out after 1 failure");
expect(exitCode).toBe(1);
});

test("run for a bun:test file under BUN_TEST_DRAIN_EVENT_LOOP, which the vendored node tests set", async () => {
using dir = tempDir("bun-test-exit-listener", { "drain.test.ts": bunTestFile(1) });
await using proc = Bun.spawn({
Expand Down
1 change: 0 additions & 1 deletion test/no-validate-leaksan.txt
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,6 @@ test/cli/test/bun-test.test.ts
test/cli/install/bun-install-security-provider.test.ts
test/js/node/test/parallel/test-tls-fast-writing.js
test/js/bun/sqlite/sqlite.test.js
test/regression/issue/12250.test.ts
test/js/bun/test/parallel/test-http-10177-response.write-with-non-ascii-latin1-should-not-cause-duplicated-character-or-segfault.ts
test/cli/install/minimum-release-age.test.ts

Expand Down
Loading