Skip to content
Closed
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
44 changes: 40 additions & 4 deletions src/runtime/cli/test_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -769,6 +769,12 @@ pub struct CommandLineReporter {
pub skips_to_repeat_buf: Vec<u8>,
pub todos_to_repeat_buf: Vec<u8>,

/// Set once `summary.fail` reaches `jest.bail`. Stops the per-file loop
/// and the `--rerun-each` repeat loop so control returns to `exec` and,
/// under `--watch`, reaches `run_event_loop_for_watch` instead of having
/// the bail path `Global::exit(1)` and kill the watcher.
pub bailed: bool,

pub reporters: ReportersConfig,
}

Expand Down Expand Up @@ -1337,15 +1343,28 @@ impl CommandLineReporter {
this.summary().fail += 1;

if this.summary().fail == this.jest.bail {
this.print_summary();
this.bailed = true;
// Stop the current file's step loops so no further
// sequences run; the per-file loop in `run_all_tests` and
// the `--rerun-each` loop in `run` both check `bailed`.
buntest.execution.aborted = true;
let watching =
VirtualMachine::get().hot_reload == jsc::virtual_machine::HOT_RELOAD_WATCH;
if !watching {
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();
Global::exit(1);
if !watching {
this.write_junit_report_if_needed();
Global::exit(1);
}
// Under --watch the run unwinds back to `exec`, which
// prints the full summary and enters the watch loop.
}
}
}
Expand Down Expand Up @@ -2102,6 +2121,7 @@ impl TestCommand {
failures_to_repeat_buf: Vec::new(),
skips_to_repeat_buf: Vec::new(),
todos_to_repeat_buf: Vec::new(),
bailed: false,
reporters: ReportersConfig::default(),
});
// `defer { if (reporter.reporters.junit) |fr| fr.deinit() }` — handled by Drop.
Expand Down Expand Up @@ -2967,6 +2987,9 @@ impl TestCommand {
) {
handle_top_level_test_error_before_javascript_start(&err);
}
if reporter.bailed {
return;
}
reporter.jest.default_timeout_override = u32::MAX;
Global::mimalloc_cleanup(false);
if isolate {
Expand Down Expand Up @@ -3136,12 +3159,22 @@ impl TestCommand {
reporter.summary().fail += 1;

if reporter.jest.bail == reporter.summary().fail {
reporter.print_summary();
reporter.bailed = true;
let watching = vm.hot_reload == jsc::virtual_machine::HOT_RELOAD_WATCH;
if !watching {
reporter.print_summary();
}
pretty_error!(
"\nBailed out after {} failure{}<r>\n",
reporter.jest.bail,
if reporter.jest.bail == 1 { "" } else { "s" }
);
if watching {
// Under --watch the run unwinds back to `exec`,
// which prints the full summary and enters the
// watch loop.
return Ok(());
}
reporter.write_junit_report_if_needed();
Comment thread
robobun marked this conversation as resolved.

vm.exit_handler.exit_code = 1;
Expand Down Expand Up @@ -3230,6 +3263,9 @@ impl TestCommand {
vm.auto_killer.disable();
}

if reporter.bailed {
break;
}
repeat_index += 1;
}
Ok(())
Expand Down
17 changes: 17 additions & 0 deletions src/runtime/test_runner/Execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,11 @@ pub struct Execution {
/// `t.skip()`/`t.todo()` mark lands there before the DoneCallback is
/// stamped).
pub on_stack_entry_data: core::cell::Cell<Option<super::bun_test::EntryData>>,
/// Set by `--bail` from within `handle_test_completed` to stop the step
/// loops mid-run so no further sequences execute. Checked by `step_group`,
/// `step_group_one`, and `step`'s per-group loop; lets the run unwind
/// cleanly instead of `Global::exit(1)` when `--watch` is active.
pub aborted: bool,
}

pub struct ConcurrentGroup {
Expand Down Expand Up @@ -282,6 +287,7 @@ impl Execution {
group_index: 0,
on_stack_entry: core::cell::Cell::new(None),
on_stack_entry_data: core::cell::Cell::new(None),
aborted: false,
}
}

Expand Down Expand Up @@ -384,6 +390,9 @@ impl Execution {
// re-slice from `this` each iteration via the group's range; carry
// `group` as NonNull so no `&mut ConcurrentGroup` aliases `&mut Execution`.
loop {
if this.aborted {
return Ok(StepResult::Complete);
}
// SAFETY: group_ptr points into this.groups (disjoint from this.sequences).
let group = unsafe { &mut *group_ptr.as_ptr() };
let seq_len = group.sequence_end - group.sequence_start;
Expand Down Expand Up @@ -798,6 +807,9 @@ pub(crate) fn step_group(
let this = &mut buntest.execution;

loop {
if this.aborted {
return Ok(StepResult::Complete);
}
// Carry the active group as NonNull so it does not alias `&mut Execution` re-derived
// inside step_group_one.
let group_ptr: NonNull<ConcurrentGroup> = match this.active_group() {
Expand Down Expand Up @@ -879,6 +891,11 @@ fn step_group_one(
g.sequence_end - g.sequence_start
};
for sequence_index in 0..len {
// Fresh short-lived reborrow each iteration so it does not span
// `step_sequence`'s own `.get()` (stacked-borrows hygiene).
if buntest_strong.get().execution.aborted {
break;
}
let sequence_status =
step_sequence(buntest_strong, global_this, group, sequence_index, now)?;
match sequence_status {
Expand Down
60 changes: 59 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, isWindows, tempDir, tempDirWithFiles, tmpdirSync } from "harness";
import { mkdirSync, rmSync, writeFileSync } from "node:fs";
import { dirname, join, resolve } from "node:path";

Expand Down Expand Up @@ -356,6 +356,64 @@
expect(stderr).toContain("Bailed out after 3 failures");
expect(stderr).not.toContain("test #4");
});

// On Windows, `bun test --watch` runs a parent watcher-manager that
// respawns a child on change rather than exec()-in-place, which makes
// the stderr stream sync points here unreliable.
test.skipIf(isWindows)(
"--bail with --watch keeps watching after a bail",
async () => {
using dir = tempDir("bun-test-bail-watch", {
"package.json": "{}",
"v.ts": `export const v = 0;\n`,
"bail.test.ts": `
import { test, expect } from "bun:test";
import { v } from "./v";
test("test #1", () => { expect(v).toBe(1); });
test("test #2", () => { expect(2).toBe(2); });
`,
});

await using proc = Bun.spawn({
cmd: [bunExe(), "test", "--watch", "--bail", "--no-clear-screen"],
cwd: String(dir),
env: bunEnv,
stdout: "ignore",
stderr: "pipe",
stdin: "ignore",
});

const reader = proc.stderr.getReader();
const decoder = new TextDecoder();
let buf = "";

async function waitFor(needle: string, from = 0): Promise<number> {
while (!buf.slice(from).includes(needle)) {
const { value, done } = await reader.read();
if (done) throw new Error(`stream closed before seeing ${JSON.stringify(needle)}\n${buf}`);
buf += decoder.decode(value, { stream: true });
}
return buf.length;
}

// First run: test #1 fails, bail fires, test #2 must not run, and
// the normal end-of-run summary must still print (not a hard exit
// mid-run).
const afterBail = await waitFor("Bailed out after 1 failure");
await waitFor("Ran 1 test across 1 file");
Comment thread
robobun marked this conversation as resolved.
expect(buf).not.toContain("test #2");
expect(proc.exitCode).toBeNull();

// Fix the failing test. The watcher should still be alive, pick up
// the change, and re-run to green.
writeFileSync(join(String(dir), "v.ts"), "export const v = 1;\n");
await waitFor("2 pass", afterBail);

proc.kill();

Check warning on line 412 in test/cli/test/bun-test.test.ts

View check run for this annotation

Claude / Claude Code Review

Redundant proc.kill()/reader.releaseLock() alongside await using

The trailing `proc.kill()` / `reader.releaseLock()` are redundant alongside `await using proc = Bun.spawn(...)` — Subprocess's `[Symbol.asyncDispose]` already kills and awaits exit on scope exit (success *or* thrown assertion), so these lines only run on the success path where they add nothing and are skipped on the failure path where cleanup matters. REVIEW.md's hermeticity rule is "no manual close alongside `using`"; consider dropping both. (Pre-existing convention: the sibling `--changed --wa
Comment thread
robobun marked this conversation as resolved.
reader.releaseLock();
},
30_000,
);
});
describe("--timeout", () => {
test("must provide a number timeout", () => {
Expand Down
Loading