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
18 changes: 18 additions & 0 deletions src/jsc/VirtualMachine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1059,6 +1059,24 @@
|| !el.next_immediate_tasks.is_empty()
}

/// Like [`is_event_loop_alive`] without the `unhandled_error_counter == 0`
/// gate: true iff there are ref'd handles or queued tasks keeping the loop
/// alive. Used by the test runner to drain a script file after tests finish.
pub fn event_loop_has_pending_work(&self) -> bool {
let el = self.event_loop_shared();
let active = self
.platform_loop_opt()
.map(|h| h.is_active())
.unwrap_or(false);
(active as usize)
+ self.active_tasks
+ el.tasks.readable_length()
+ (el.has_pending_refs() as usize)
> 0
|| !el.immediate_tasks.is_empty()
|| !el.next_immediate_tasks.is_empty()
}

Check warning on line 1078 in src/jsc/VirtualMachine.rs

View check run for this annotation

Claude / Claude Code Review

event_loop_has_pending_work() copy-pastes the is_event_loop_alive() predicate

This copy-pastes the four-term keep-alive sum and both immediate-task checks from `is_event_loop_alive_excluding_immediates()`/`is_event_loop_alive()` — a future keep-alive source added to one predicate will silently miss the other. Extract the sum portion as a shared private helper and have both callers use it. Note that the obvious collapse `is_event_loop_alive() = self.unhandled_error_counter == 0 && self.event_loop_has_pending_work()` is *not* behavior-preserving (the existing immediate-task
Comment thread
claude[bot] marked this conversation as resolved.
Outdated

pub fn wakeup(&mut self) {
self.event_loop_mut().wakeup();
}
Expand Down
12 changes: 12 additions & 0 deletions src/runtime/cli/test_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3233,6 +3233,18 @@
}
}

// A file that registered no test()/describe() is a plain script:
// drain ref'd handles so late uncaught errors surface, like `bun <file>`.
if buntest.collection.root_scope.entries.is_empty() {
let drain_base = vm.unhandled_error_counter;
while drain_base == vm.unhandled_error_counter
&& vm.event_loop_has_pending_work()
{
vm.event_loop_ref().tick();
vm.event_loop_ref().auto_tick();
}
}

Check failure on line 3246 in src/runtime/cli/test_command.rs

View check run for this annotation

Claude / Claude Code Review

Drain loop's VM-wide pending-work check hangs on handles not owned by the current file

The drain gate checks only the *current file's* `entries.is_empty()`, but the loop condition `vm.event_loop_has_pending_work()` is VM-wide — so any ref'd handle not created by this file keeps the drain spinning forever. Two concrete regressions (both exited 0 before this PR): (1) a prior test file or `--preload` script that leaks a `setInterval`/server makes every subsequent no-`test()` file hang on all platforms; (2) under `--parallel` on Windows, the worker's own IPC pipe is deliberately kept
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
118 changes: 118 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,124 @@ 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 () => {
// Files that use bun:test keep today's behavior: the runner does not
// block on leaked handles after the last test finishes.
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);
});
});

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