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
17 changes: 8 additions & 9 deletions src/runtime/cli/test/parallel/runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -878,6 +878,12 @@ impl<'a> WorkerLoop<'a> {
resident_at_start: start_measuring_growth(),
});

let isolation = if vm.test_isolation_enabled {
test_command::FileIsolation::SwapGlobal
} else {
test_command::FileIsolation::KeepGlobal
};

// A worker never knows which file is its last, so preload-level hooks wrap every file (with or without --isolate).
if let Err(err) = TestCommand::run(
self.reporter,
Expand All @@ -887,18 +893,11 @@ impl<'a> WorkerLoop<'a> {
first: true,
last: true,
},
isolation,
) {
test_command::handle_top_level_test_error_before_javascript_start(&err);
}
if vm.test_isolation_enabled {
crate::jsc_hooks::stop_active_handles_for_test_isolation(vm);
vm.swap_global_for_test_isolation();
self.reporter
.jest
.bun_test_root
.reset_hook_scope_for_test_isolation();
crate::test_runner::Expect::reset_for_test_isolation(vm.global());
} else {
if !vm.test_isolation_enabled {
Global::mimalloc_cleanup(false);
}
self.reporter.jest.default_timeout_override = u32::MAX;
Expand Down
79 changes: 58 additions & 21 deletions src/runtime/cli/test_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3954,6 +3954,12 @@ extern "C" fn BunTest__shouldGenerateCodeCoverage(test_name_str: &bun_core::Stri
true
}

#[derive(Clone, Copy, PartialEq, Eq)]
pub(crate) enum FileIsolation {
KeepGlobal,
SwapGlobal,
}

pub(crate) struct TestCommand;

impl TestCommand {
Expand Down Expand Up @@ -5180,6 +5186,11 @@ impl TestCommand {
first: isolate || i == 0,
last: isolate,
},
if isolate {
FileIsolation::SwapGlobal
} else {
FileIsolation::KeepGlobal
},
) {
handle_top_level_test_error_before_javascript_start(&err);
}
Expand All @@ -5188,15 +5199,6 @@ impl TestCommand {
}
reporter.jest.default_timeout_override = u32::MAX;
Global::mimalloc_cleanup(false);
if isolate {
crate::jsc_hooks::stop_active_handles_for_test_isolation(vm);
vm.swap_global_for_test_isolation();
reporter
.jest
.bun_test_root
.reset_hook_scope_for_test_isolation();
crate::test_runner::Expect::reset_for_test_isolation(vm.global());
}
}
}

Expand All @@ -5210,6 +5212,7 @@ impl TestCommand {
first: isolate || files.len() == 1,
last: true,
},
FileIsolation::KeepGlobal,
) {
handle_top_level_test_error_before_javascript_start(&err);
}
Expand Down Expand Up @@ -5330,6 +5333,7 @@ impl TestCommand {
vm: &mut VirtualMachine,
file_name: &[u8],
first_last: bun_test::FirstLast,
isolation: FileIsolation,
) -> crate::Result<()> {
let _print_log = PrintLogAfterFile(vm.log);
let restore_only = RestoreOnlyAfterFile {
Expand Down Expand Up @@ -5393,10 +5397,16 @@ impl TestCommand {
let next_load_shares_global = !vm.test_isolation_enabled
|| vm.test_isolation_state.shares_global
|| repeat_index + 1 < repeat_count;
let _entered_file = EnteredFile {
let mut entered_file = EnteredFile {
bun_test_root: bun_test_root_ptr,
global,
next_load_shares_global,
isolation,
// Early returns finalize too; only successful intermediate repeats retain the global.
finalize_file: true,
end_output_group: Output::is_github_action()
&& (reporter.worker_ipc_file_idx.is_none()
|| reporter.worker_stands_for_one_process),
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
};

// Recompute on every repeat because the reporters overwrite `relative`'s buffer while the file runs.
Expand Down Expand Up @@ -5590,14 +5600,6 @@ impl TestCommand {
// Must run after the cleanup above, so rejections from JS it runs are reported in this file.
let _ = vm.global().handle_rejected_promises();

if Output::is_github_action()
&& (reporter.worker_ipc_file_idx.is_none()
|| reporter.worker_stands_for_one_process)
{
pretty_errorln!("<r>\n::endgroup::\n");
Output::flush();
}

if !vm.test_isolation_enabled {
// Ensure these never linger across files. Under --isolate this
// is done by swapGlobalForTestIsolation() (kill+clear) and we
Expand All @@ -5615,6 +5617,7 @@ impl TestCommand {
if repeat_index == repeat_count {
environment.teardown(vm);
}
entered_file.finalize_file = repeat_index == repeat_count;
}
if let Some(junit) = reporter.reporters.junit.as_mut() {
let _ = junit.end_file(Some(junit_file_name(file_path)), None);
Expand Down Expand Up @@ -5661,15 +5664,49 @@ struct EnteredFile<'a> {
bun_test_root: *mut bun_test::BunTestRoot,
global: &'a jsc::JSGlobalObject,
next_load_shares_global: bool,
isolation: FileIsolation,
finalize_file: bool,
end_output_group: bool,
}

impl Drop for EnteredFile<'_> {
fn drop(&mut self) {
// SAFETY: `bun_test_root` belongs to `Jest::runner()`, which lives as long as the process.
unsafe { (*self.bun_test_root).exit_file() };
// A mock.module() patch still pending must not hold up the next file.
let swap = self.finalize_file && self.isolation == FileIsolation::SwapGlobal;
if swap {
// SAFETY: the root and active file outlive this guard. End scheduling before
// close callbacks can settle promises from an abandoned test or hook.
if let Some(active) = unsafe { (*self.bun_test_root).clone_active_file() } {
active.get().begin_file_teardown();
}
}
// Pending patches and mocks belong to the outgoing global, before it is retired.
bun_jsc::cpp::JSMock__forgetPendingModulePatches(self.global);
bun_jsc::cpp::JSMock__undoModuleMocksOfTestFile(self.global, self.next_load_shares_global);
if swap {
let vm = self.global.bun_vm().as_mut();
// Keep the active file and its reporter through close callbacks and the
// final microtask drain so teardown errors retain their file attribution.
crate::jsc_hooks::stop_active_handles_for_test_isolation(vm);
vm.swap_global_for_test_isolation();
}
// SAFETY: the process-owned root is live; all local BunTest clones have dropped.
unsafe { (*self.bun_test_root).exit_file() };
if swap {
// SAFETY: exit_file releases the old BunTest before its preload scopes.
unsafe { (*self.bun_test_root).reset_hook_scope_for_test_isolation() };
crate::test_runner::Expect::reset_for_test_isolation(
self.global.bun_vm().as_mut().global(),
);
}
if self.end_output_group
&& jest::Jest::runner()
.unwrap()
.current_file
.has_printed_header()
{
pretty_errorln!("<r>\n::endgroup::\n");
Output::flush();
}
}
}

Expand Down
11 changes: 11 additions & 0 deletions src/runtime/test_runner/bun_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1128,6 +1128,17 @@ impl BunTest {
let _ = self.result_queue.write_item(result); // OOM/capacity: fire-and-forget
}

/// Retain error attribution while closing a file, including an abandoned load.
/// Close callbacks must not resume its tests or hooks.
pub(crate) fn begin_file_teardown(&mut self) {
debug_assert!(!self.in_run_loop);
self.phase = Phase::Done;
self.wants_wakeup = false;
if self.timer.state == EventLoopTimerState::ACTIVE {
vm_timer().remove(&raw mut self.timer);
}
}

pub(crate) fn run(this_strong: &BunTestPtr, global_this: &JSGlobalObject) -> JsResult<()> {
let _g = group_begin!();
// `Collection::step` / `Execution::step` re-enter and call `.get()` on
Expand Down
4 changes: 4 additions & 0 deletions src/runtime/test_runner/jest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,10 @@ impl CurrentFile {
Output::flush();
}

pub(crate) fn has_printed_header(&self) -> bool {
self.has_printed_filename
}

pub(crate) fn print_if_needed(&mut self) {
if self.has_printed_filename {
return;
Expand Down
60 changes: 60 additions & 0 deletions test/cli/test/bun-test.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,66 @@ describe("bun test", () => {
}, 10000);
});
describe("support for Github Actions", () => {
test.concurrent.each(
["--reporter=dots", "--only-failures"].flatMap(reporter =>
[
{ mode: "serial", args: ["--isolate"], repeats: 1 },
{ mode: "repeated", args: ["--isolate", "--rerun-each", "2"], repeats: 2 },
{ mode: "worker", args: ["--parallel=2"], repeats: 1 },
].flatMap(mode =>
["pass", "fail", "load error", "teardown error"].map(outcome => ({ reporter, ...mode, outcome })),
),
),
)("balances quiet file groups: $reporter, $mode, $outcome", async ({ reporter, args, repeats, outcome }) => {
using dir = tempDir("test-quiet-groups", {
"a.test.ts":
outcome === "teardown error"
? `import { test } from "bun:test";
test("owner", async () => {
const { promise, resolve } = Promise.withResolvers();
const listener = Bun.listen({
hostname: "127.0.0.1", port: 0,
socket: {
open() { resolve(); }, data() {},
close() { throw new Error("group teardown sentinel"); },
},
});
await Bun.connect({ hostname: "127.0.0.1", port: listener.port, socket: { data() {} } });
await promise;
});`
: outcome === "load error"
? `throw new Error("group load sentinel");`
: `import { test, expect } from "bun:test";
test("owner", () => expect(1).toBe(${outcome === "pass" ? 1 : 2}));`,
"b.test.ts": `import { test } from "bun:test"; test("observer", () => {});`,
});
await using child = Bun.spawn({
cmd: [bunExe(), "test", ...args, reporter, "./a.test.ts", "./b.test.ts"],
cwd: String(dir),
env: { ...bunEnv, AGENT: "0", GITHUB_ACTIONS: "true", BUN_TEST_PARALLEL_SCALE_MS: "60000" },
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([child.stdout.text(), child.stderr.text(), child.exited]);
const groups = outcome === "pass" ? 0 : outcome === "fail" ? repeats : 1;
expect(stderr.match(/::(?:endgroup|group)::/g) ?? []).toEqual(
Array.from({ length: groups }, () => ["::group::", "::endgroup::"]).flat(),
);
if (outcome === "load error" || outcome === "teardown error") {
const sentinel = outcome === "load error" ? "group load sentinel" : "group teardown sentinel";
const sentinelAt = stderr.indexOf(sentinel);
expect(sentinelAt).toBeGreaterThan(stderr.indexOf("::group::"));
expect(sentinelAt).toBeLessThan(stderr.indexOf("::endgroup::"));
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
// Load failure stops repeats; global teardown follows the final repeat.
const run = outcome === "load error" ? 1 : repeats;
expect(stderr.split("\n").filter(line => line.startsWith("::group::"))).toEqual([
`::group::a.test.ts:${repeats > 1 ? ` (run #${run})` : ""}`,
]);
}
if (args.includes("--parallel=2")) expect(stdout).toContain("PARALLEL");
expect(exitCode).toBe(outcome === "pass" ? 0 : 1);
});

test("should not group logs by default", () => {
const stderr = runTest({
env: {
Expand Down
Loading