Skip to content
Open
Show file tree
Hide file tree
Changes from 9 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
24 changes: 23 additions & 1 deletion src/jsc/VirtualMachine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1059,6 +1059,22 @@ impl VirtualMachine {
|| !el.next_immediate_tasks.is_empty()
}

/// Count of ref'd handles and outstanding tasks keeping the loop alive.
/// The test runner's idle-after-preloads gate only drains a script file
/// when this was zero before it loaded (prior handles skip the drain).
pub fn active_keepalive_count(&self) -> usize {
let el = self.event_loop_shared();
let active = self
.platform_loop_opt()
.map(|h| h.active_count() as usize)
.unwrap_or(0);
let concurrent = el
.concurrent_ref
.load(core::sync::atomic::Ordering::SeqCst)
.max(0) as usize;
active + self.active_tasks + concurrent
}

pub fn wakeup(&mut self) {
self.event_loop_mut().wakeup();
}
Expand Down Expand Up @@ -4592,6 +4608,7 @@ impl VirtualMachine {
pub fn reload_entry_point_for_test_runner(
&mut self,
entry_path: &[u8],
after_preloads: impl FnOnce(&Self),
) -> crate::CrateResult<*mut JSInternalPromise> {
self.has_loaded = false;
self.set_main(entry_path);
Expand All @@ -4618,6 +4635,8 @@ impl VirtualMachine {
}
}

after_preloads(self);

// Note: reshaped for borrowck.
let global = self.global;
let main_str = bun_core::String::from_bytes(self.main());
Expand Down Expand Up @@ -4648,11 +4667,14 @@ impl VirtualMachine {
}

/// Loads a test-file entry point and waits for the load promise to settle.
/// `after_preloads` runs between preload completion and entry-point
/// evaluation so the caller can observe preload-created handles.
Comment thread
robobun marked this conversation as resolved.
Outdated
pub fn load_entry_point_for_test_runner(
&mut self,
entry_path: &[u8],
after_preloads: impl FnOnce(&Self),
) -> crate::CrateResult<*mut JSInternalPromise> {
let promise = self.reload_entry_point_for_test_runner(entry_path)?;
let promise = self.reload_entry_point_for_test_runner(entry_path, after_preloads)?;

// pending_internal_promise can change if hot module reloading is enabled
if self.is_watcher_enabled() {
Expand Down
4 changes: 4 additions & 0 deletions src/libuv_sys/libuv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,10 @@ impl Loop {
unsafe { uv_loop_alive(self) != 0 }
}
#[inline]
pub fn active_count(&self) -> u32 {
self.active_handles
}
#[inline]
pub fn tick(&mut self) {
// SAFETY: self is a live loop.
let _ = unsafe { uv_run(self, RunMode::Default) };
Expand Down
45 changes: 44 additions & 1 deletion src/runtime/cli/test_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3142,7 +3142,12 @@

// need to wake up so autoTick() doesn't wait for 16-100ms after loading the entrypoint
vm.wakeup();
let promise = vm.load_entry_point_for_test_runner(file_path)?;
// Record whether the loop was idle after preloads so the post-test
// drain (below) only runs when every ref'd handle is this file's.
let mut idle_after_preloads = false;
let promise = vm.load_entry_point_for_test_runner(file_path, |vm| {
idle_after_preloads = script_keepalive_count(vm) == 0;
})?;
// Only count the file once, not once per repeat
if repeat_index == 0 {
reporter.summary().files += 1;
Expand Down Expand Up @@ -3233,6 +3238,29 @@
}
}

// A file with no bun:test registrations is a plain script: drain
// the loop so late errors surface (like `bun <file>`). Gated on
// an idle loop after preloads so nothing else can hang it.
if idle_after_preloads
&& buntest.collection.root_scope.is_bare()
&& buntest.bun_test_root.get().hook_scope.is_bare()
{
let drain_base = vm.unhandled_error_counter;
// Bound by the default test timeout so a handle created by a
// prior file's unref'd callback can't hang the run forever.
let deadline = bun_core::Timespec::now(bun_core::TimespecMockMode::ForceRealTime)
.add_ms(i64::from(reporter.jest.default_timeout_ms));

Check warning on line 3252 in src/runtime/cli/test_command.rs

View check run for this annotation

Claude / Claude Code Review

Drain deadline reads raw default_timeout_ms, ignoring 0=unlimited and setDefaultTimeout() override

The deadline reads `reporter.jest.default_timeout_ms` directly, which diverges from the effective-timeout precedence tests actually use (`ScopeFunctions.rs:740-747`: per-test option → `default_timeout_override` → `default_timeout_ms`, with **0 = unlimited**). With `--timeout=0` (accepted by `Arguments.rs:1562` and treated as unlimited elsewhere), `deadline == now` so `.is_lt()` is false on the first check and the drain body never runs — defeating the fix. Separately, a preload calling `setDefaul
Comment thread
robobun marked this conversation as resolved.
Outdated
while drain_base == vm.unhandled_error_counter
&& script_keepalive_count(vm) > 0
&& bun_core::Timespec::now(bun_core::TimespecMockMode::ForceRealTime)
.order(&deadline)
.is_lt()
{
vm.event_loop_ref().tick();
vm.event_loop_ref().auto_tick();
}
Comment thread
robobun marked this conversation as resolved.
Outdated
}

let el = vm.event_loop();
// SAFETY: el is the VM-owned event loop; vm is passed back as *mut.
unsafe { (*el).tick_immediate_tasks(vm) };
Expand Down Expand Up @@ -3260,6 +3288,21 @@
}
}

/// Count of ref'd handles plus ref'd JS timers (they share one loop ref).
/// Zero at `after_preloads` means every handle observed during the post-test
/// drain was created by this file.
fn script_keepalive_count(vm: &VirtualMachine) -> usize {
let state = crate::jsc_hooks::runtime_state();
let timers = if state.is_null() {
0
} else {
// SAFETY: `runtime_state()` returns the live per-thread `RuntimeState`;
// `active_timer_count` is plain data read on the owning JS thread.
unsafe { (*state).timer.active_timer_count.max(0) as usize }
};
vm.active_keepalive_count() + timers
}

pub(crate) fn handle_top_level_test_error_before_javascript_start(err: &crate::Error) -> ! {
if cfg!(debug_assertions) {
if !matches!(err, crate::Error::ModuleNotFound) {
Expand Down
9 changes: 9 additions & 0 deletions src/runtime/test_runner/bun_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1746,6 +1746,15 @@ pub struct DescribeScope {
}

impl DescribeScope {
/// True iff no test(), describe() or lifecycle hook was registered here.
pub fn is_bare(&self) -> bool {
self.entries.is_empty()
&& self.before_all.is_empty()
&& self.before_each.is_empty()
&& self.after_each.is_empty()
&& self.after_all.is_empty()
}

pub fn create(base: BaseScope) -> Box<DescribeScope> {
Box::new(DescribeScope {
base,
Expand Down
5 changes: 5 additions & 0 deletions src/uws_sys/Loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,11 @@ impl PosixLoop {
self.active > 0
}

#[inline]
pub fn active_count(&self) -> u32 {
self.active
}

// This exists as a method so that we can stick a debugger in here
pub fn add_active(&mut self, value: u32) {
bun_core::scoped_log!(
Expand Down
198 changes: 198 additions & 0 deletions test/cli/test/bun-test.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1458,6 +1458,204 @@ describe("bun test", () => {
});
});

// https://github.com/oven-sh/bun/issues/34859
describe.concurrent("script files with no test() registrations", () => {
test("fails on a delayed unhandled rejection from an async IIFE", async () => {
using dir = tempDir("script-delayed-rejection", {
"delayed.test.js": `
'use strict';
(async () => {
await new Promise(r => setTimeout(r, 20));
throw new Error("delayed rejection should fail the test run");
})();
`,
});
await using proc = Bun.spawn({
cmd: [bunExe(), "test", "./delayed.test.js"],
env: bunEnv,
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("delayed rejection should fail the test run");
expect(stderr).toContain("Unhandled error between tests");
expect(stderr).toContain("1 error");
expect(exitCode).toBe(1);
});

test("fails on a delayed uncaught exception from a timer", async () => {
using dir = tempDir("script-delayed-throw", {
"delayed.test.js": `setTimeout(() => { throw new Error("boom from timer"); }, 20);`,
});
await using proc = Bun.spawn({
cmd: [bunExe(), "test", "./delayed.test.js"],
env: bunEnv,
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("boom from timer");
expect(stderr).toContain("Unhandled error between tests");
expect(exitCode).toBe(1);
});

test("waits for ref'd async work to complete", async () => {
using dir = tempDir("script-drain", {
"drain.test.js": `
const assert = require("assert");
(async () => {
await new Promise(r => setTimeout(r, 20));
assert.strictEqual(1, 1);
console.log("reached end of async iife");
})();
`,
});
await using proc = Bun.spawn({
cmd: [bunExe(), "test", "./drain.test.js"],
env: bunEnv,
cwd: String(dir),
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stdout).toContain("reached end of async iife");
expect(stderr).toContain("0 fail");
expect(exitCode).toBe(0);
});

test("drains per file so a later script file's delayed rejection is caught", async () => {
using dir = tempDir("script-two-files", {
"a.test.js": `
(async () => {
await Promise.resolve();
throw new Error("file A microtask error");
})();
`,
"b.test.js": `
(async () => {
await new Promise(r => setTimeout(r, 20));
throw new Error("file B delayed error");
})();
`,
});
await using proc = Bun.spawn({
cmd: [bunExe(), "test", "./a.test.js", "./b.test.js"],
env: bunEnv,
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("file A microtask error");
expect(stderr).toContain("file B delayed error");
expect(exitCode).toBe(1);
});

test("does not drain when the file registered a test()", async () => {
using dir = tempDir("registered-test-no-drain", {
"with.test.js": `
const { test } = require("bun:test");
setInterval(() => {}, 60_000);
test("noop", () => {});
`,
});
await using proc = Bun.spawn({
cmd: [bunExe(), "test", "./with.test.js"],
env: bunEnv,
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("1 pass");
expect(exitCode).toBe(0);
});

test("does not drain when a prior file left a ref'd handle", async () => {
using dir = tempDir("prior-file-leak", {
"a.test.js": `
const { test } = require("bun:test");
setInterval(() => {}, 60_000);
test("noop", () => {});
`,
"b.test.js": `require("assert").strictEqual(1, 1);`,
});
await using proc = Bun.spawn({
cmd: [bunExe(), "test", "./a.test.js", "./b.test.js"],
env: bunEnv,
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("1 pass");
expect(stderr).toContain("0 fail");
expect(exitCode).toBe(0);
});

test("does not drain when a preload left a ref'd handle", async () => {
using dir = tempDir("preload-leak", {
"setup.js": `setInterval(() => {}, 60_000);`,
"a.test.js": `require("assert").strictEqual(1, 1);`,
});
await using proc = Bun.spawn({
cmd: [bunExe(), "test", "--preload", "./setup.js", "./a.test.js"],
env: bunEnv,
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("0 fail");
expect(exitCode).toBe(0);
});

test("does not drain when a preload registered a beforeAll hook", async () => {
using dir = tempDir("preload-hook", {
"setup.js": `
import { beforeAll, afterAll } from "bun:test";
beforeAll(() => { globalThis.srv = Bun.serve({ port: 0, fetch: () => new Response() }); });
afterAll(() => { globalThis.srv.stop(); });
`,
"a.test.js": `require("assert").strictEqual(1, 1);`,
"b.test.js": `import { test } from "bun:test"; test("noop", () => {});`,
});
await using proc = Bun.spawn({
cmd: [bunExe(), "test", "--preload", "./setup.js", "./a.test.js", "./b.test.js"],
env: bunEnv,
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("1 pass");
expect(exitCode).toBe(0);
});

test("drain is bounded by --timeout when a prior file's unref'd callback creates a handle", async () => {
using dir = tempDir("unref-prior", {
"a.test.js": `
const { test } = require("bun:test");
setTimeout(() => setInterval(() => {}, 60_000), 5).unref();
test("noop", () => {});
`,
"b.test.js": `setTimeout(() => {}, 100);`,
});
await using proc = Bun.spawn({
cmd: [bunExe(), "test", "--timeout=500", "./a.test.js", "./b.test.js"],
env: bunEnv,
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("1 pass");
expect(exitCode).toBe(0);
});
});

function createTest(input?: string | (string | { filename: string; contents: string })[], filename?: string): string {
const cwd = tmpdirSync();
const inputs = Array.isArray(input) ? input : [input ?? ""];
Expand Down
Loading