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
8 changes: 4 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,10 @@ 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 intentionally NOT forwarded: a worker would exit(1) on bail
// (see test_command.rs `bail_out`), which the coordinator would misread as
// a crash. Cross-worker bail is handled at file granularity by the
// coordinator instead.
Comment thread
robobun marked this conversation as resolved.
Outdated
if opts.repeat_count > 0 {
argv.push(print_z(format_args!("--rerun-each={}", opts.repeat_count))?);
}
Expand Down
83 changes: 46 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,48 @@ 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
/// teardown as the end of a normal run (`exec`). A bare `Global::exit` here
/// would skip the `BUN_DESTRUCT_VM_ON_EXIT` teardown, and the leak-check
/// lanes then report every wrapper box the JSC finalizers still own.
///
/// Reached mid-file (`handle_test_completed`, with the file's `BunTest`
/// still held by `TestCommand::run`) and after a failed module evaluation;
/// `process.exit()` inside a test tears the VM down from the same state.
Comment thread
robobun marked this conversation as resolved.
Outdated
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() });
// Same order as `exec`: exit listeners (user JS) ran above while the
// runner was intact; now release the `bun:test` state and unpublish
// `RUNNER` so nothing running inside the teardown GC can observe a
// half-torn-down `TestRunner`. The reporter itself stays allocated:
// both callers still borrow it.
Comment thread
robobun marked this conversation as resolved.
Outdated
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 +3327,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
165 changes: 164 additions & 1 deletion test/cli/test/bun-test.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
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";

Expand Down Expand Up @@ -329,6 +329,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,10 +353,133 @@ describe("bun test", () => {
expect(true).toBe(true);
});
`,
expectExitCode: 1,
});
expect(stderr).toContain("Bailed out after 3 failures");
expect(stderr).not.toContain("test #4");
});

// The sanitizer lane (scripts/runner.node.mjs) runs every child with this environment. Bailing
// out has to exit through the same VM teardown as the end of a normal run; a bare exit leaves
// the objects the bun:test finalizers own allocated, and LeakSanitizer then aborts the process
// (exit 134) instead of it exiting 1.
describe.concurrent.skipIf(!isASAN)("exits cleanly under LeakSanitizer", () => {
const env = {
...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")}`,
};

/** 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,
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)"]);
});
});
});
describe("--timeout", () => {
test("must provide a number timeout", () => {
Expand Down Expand Up @@ -1708,6 +1832,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