diff --git a/src/runtime/jsc_hooks.rs b/src/runtime/jsc_hooks.rs index 9f565cf3f15b..a5f5b36f41b3 100644 --- a/src/runtime/jsc_hooks.rs +++ b/src/runtime/jsc_hooks.rs @@ -1779,9 +1779,9 @@ unsafe fn retroactively_report_discovered_tests( agent: *mut bun_jsc::debugger::TestReporterHandle, next_test_id: i32, ) -> i32 { - use crate::test_runner::bun_test::{DescribeScope, Phase, TestScheduleEntry}; + use crate::test_runner::bun_test::{Phase, TestScheduleEntry}; use crate::test_runner::jest::Jest; - use bun_jsc::debugger::{TestReporterHandle, TestType}; + use bun_jsc::debugger::TestType; let Some(runner) = Jest::runner() else { return next_test_id; @@ -1808,77 +1808,58 @@ unsafe fn retroactively_report_discovered_tests( let mut max_id: i32 = next_test_id; - // Recursively report all discovered tests starting from root scope. - retroactively_report_scope( - agent, - &mut active_file.collection.root_scope, - -1, - &mut max_id, - &source_url, - ); - - return max_id; - - fn retroactively_report_scope( - agent: *mut TestReporterHandle, - scope: &mut DescribeScope, - parent_id: i32, - max_id: &mut i32, - source_url: &bun_core::String, - ) { - for entry in scope.entries.iter_mut() { - match entry { - TestScheduleEntry::Describe(describe) => { - if describe.base.test_id_for_debugger == 0 { - *max_id += 1; - let test_id = *max_id; - // Assign the ID so start/end events will fire during - // execution. - describe.base.test_id_for_debugger = test_id; - let name = bun_core::String::from_bytes( - describe.base.name.as_deref().unwrap_or(b"(unnamed)"), - ); - // SAFETY: `agent` is a live C++ handle (fn contract). - unsafe { &mut *agent }.report_test_found_with_location( - test_id, - &name, - TestType::Describe, - parent_id, - source_url, - describe.base.line_no as i32, - ); - // Recursively report children with this describe as - // parent. - retroactively_report_scope(agent, describe, test_id, max_id, source_url); - } else { - // Already has ID, just recurse with existing ID as - // parent. - let existing = describe.base.test_id_for_debugger; - retroactively_report_scope(agent, describe, existing, max_id, source_url); - } + // Explicit stack, not recursion: describe() nesting depth is unbounded user input. + let mut open: Vec<(core::slice::IterMut<'_, TestScheduleEntry>, i32)> = + vec![(active_file.collection.root_scope.entries.iter_mut(), -1)]; + while let Some((children, parent_id)) = open.last_mut() { + let parent_id = *parent_id; + let Some(entry) = children.next() else { + open.pop(); + continue; + }; + match entry { + TestScheduleEntry::Describe(describe) => { + if describe.base.test_id_for_debugger == 0 { + max_id += 1; + // Assign the ID so start/end events will fire during execution. + describe.base.test_id_for_debugger = max_id; + let name = bun_core::String::from_bytes( + describe.base.name.as_deref().unwrap_or(b"(unnamed)"), + ); + // SAFETY: `agent` is a live C++ handle (fn contract). + unsafe { &mut *agent }.report_test_found_with_location( + max_id, + &name, + TestType::Describe, + parent_id, + &source_url, + describe.base.line_no as i32, + ); } - TestScheduleEntry::TestCallback(test_entry) => { - if test_entry.base.test_id_for_debugger == 0 { - *max_id += 1; - let test_id = *max_id; - test_entry.base.test_id_for_debugger = test_id; - let name = bun_core::String::from_bytes( - test_entry.base.name.as_deref().unwrap_or(b"(unnamed)"), - ); - // SAFETY: `agent` is a live C++ handle (fn contract). - unsafe { &mut *agent }.report_test_found_with_location( - test_id, - &name, - TestType::Test, - parent_id, - source_url, - test_entry.base.line_no as i32, - ); - } + let describe_id = describe.base.test_id_for_debugger; + open.push((describe.entries.iter_mut(), describe_id)); + } + TestScheduleEntry::TestCallback(test_entry) => { + if test_entry.base.test_id_for_debugger == 0 { + max_id += 1; + test_entry.base.test_id_for_debugger = max_id; + let name = bun_core::String::from_bytes( + test_entry.base.name.as_deref().unwrap_or(b"(unnamed)"), + ); + // SAFETY: `agent` is a live C++ handle (fn contract). + unsafe { &mut *agent }.report_test_found_with_location( + max_id, + &name, + TestType::Test, + parent_id, + &source_url, + test_entry.base.line_no as i32, + ); } } } } + max_id } // ════════════════════════════════════════════════════════════════════════════ diff --git a/src/runtime/test_runner/Order.rs b/src/runtime/test_runner/Order.rs index 75fa2a0cc442..380ebf3d8df6 100644 --- a/src/runtime/test_runner/Order.rs +++ b/src/runtime/test_runner/Order.rs @@ -29,16 +29,6 @@ impl Order { } // `deinit` only freed `groups` / `sequences` — handled by Drop on Vec; no impl Drop needed. - pub(crate) fn generate_order_sub(&mut self, current: &mut TestScheduleEntry) -> JsResult<()> { - match current { - TestScheduleEntry::Describe(describe) => self.generate_order_describe(describe)?, - TestScheduleEntry::TestCallback(test_callback) => { - self.generate_order_test(NonNull::from(&mut **test_callback))? - } - } - Ok(()) - } - pub(crate) fn generate_all_order(&mut self, entries: &[Box]) -> JsResult { let start = self.groups.len(); for entry_box in entries.iter() { @@ -78,9 +68,42 @@ impl Order { Ok(AllOrderResult { start, end }) } - pub(crate) fn generate_order_describe(&mut self, current: &mut DescribeScope) -> JsResult<()> { + /// Explicit stack, not recursion: `describe()` nesting depth is unbounded user input. + pub(crate) fn generate_order_describe(&mut self, root: &mut DescribeScope) -> JsResult<()> { + let mut open: Vec> = Vec::new(); + if let Some(frame) = self.enter_describe(root)? { + open.push(frame); + } + while let Some(frame) = open.last_mut() { + let scope_only = frame.only; + let Some(entry) = frame.children.next() else { + let frame = open.pop().expect("non-empty"); + self.exit_describe(&frame)?; + continue; + }; + if scope_only == Only::Contains && entry.base().only == Only::No { + continue; + } + match entry { + TestScheduleEntry::Describe(describe) => { + if let Some(child) = self.enter_describe(describe)? { + open.push(child); + } + } + TestScheduleEntry::TestCallback(test_callback) => { + self.generate_order_test(NonNull::from(&mut **test_callback))? + } + } + } + Ok(()) + } + + fn enter_describe<'a>( + &mut self, + current: &'a mut DescribeScope, + ) -> JsResult>> { if current.failed { - return Ok(()); // do not schedule any tests in a failed describe scope + return Ok(None); // do not schedule any tests in a failed describe scope } let use_hooks = self.cfg.always_use_hooks || current.base.has_callback; @@ -96,22 +119,22 @@ impl Order { shuffle_with_index(random, &mut current.entries); } - // gather children - // reshaped for borrowck — iterate by index since generate_order_sub borrows &mut self. - let scope_only = current.base.only; - for i in 0..current.entries.len() { - if scope_only == Only::Contains && current.entries[i].base().only == Only::No { - continue; - } - self.generate_order_sub(&mut current.entries[i])?; - } + Ok(Some(OpenDescribe { + children: current.entries.iter_mut(), + only: current.base.only, + use_hooks, + beforeall_order, + after_all: ¤t.after_all, + })) + } + fn exit_describe(&mut self, frame: &OpenDescribe<'_>) -> JsResult<()> { // update skip_to values for beforeAll to skip to the first afterAll - beforeall_order.set_failure_skip_to(self); + frame.beforeall_order.set_failure_skip_to(self); // gather afterAll - let afterall_order: AllOrderResult = if use_hooks { - self.generate_all_order(¤t.after_all)? + let afterall_order: AllOrderResult = if frame.use_hooks { + self.generate_all_order(frame.after_all)? } else { AllOrderResult::EMPTY }; @@ -242,6 +265,15 @@ impl Order { } } +/// A describe scope whose children `generate_order_describe` is still scheduling. +struct OpenDescribe<'a> { + children: core::slice::IterMut<'a, TestScheduleEntry>, + only: Only, + use_hooks: bool, + beforeall_order: AllOrderResult, + after_all: &'a [Box], +} + pub(crate) struct AllOrderResult { pub(crate) start: usize, pub(crate) end: usize, diff --git a/src/runtime/test_runner/bun_test.rs b/src/runtime/test_runner/bun_test.rs index b67a04b69cfc..b51fec393607 100644 --- a/src/runtime/test_runner/bun_test.rs +++ b/src/runtime/test_runner/bun_test.rs @@ -1743,6 +1743,19 @@ pub(crate) struct DescribeScope { pub(crate) failed: bool, } +impl Drop for DescribeScope { + /// Iterative: the default drop recurses once per nested `describe()`, an unbounded depth. + fn drop(&mut self) { + let mut pending: Vec = core::mem::take(&mut self.entries); + while let Some(entry) = pending.pop() { + if let TestScheduleEntry::Describe(mut scope) = entry { + pending.append(&mut scope.entries); + // `scope.entries` is empty now, so dropping `scope` here does not recurse. + } + } + } +} + impl DescribeScope { pub(crate) fn create(base: BaseScope) -> Box { Box::new(DescribeScope { @@ -1755,7 +1768,6 @@ impl DescribeScope { failed: false, }) } - // destroy → Drop on Box; all fields own their contents. fn mark_contains_only(&mut self) { let mut target: Option<*mut DescribeScope> = Some(std::ptr::from_mut(self)); diff --git a/test/cli/inspect/test-reporter.test.ts b/test/cli/inspect/test-reporter.test.ts index 45dd1d10d451..62d2e33e6d81 100644 --- a/test/cli/inspect/test-reporter.test.ts +++ b/test/cli/inspect/test-reporter.test.ts @@ -1,6 +1,6 @@ import { Subprocess, spawn, write } from "bun"; import { afterEach, describe, expect, test } from "bun:test"; -import { bunEnv, bunExe, isPosix, tempDir } from "harness"; +import { bunEnv, bunExe, isDebug, isLinux, isPosix, tempDir } from "harness"; import { join } from "node:path"; import { InspectorSession, connect } from "./junit-reporter"; import { SocketFramer } from "./socket-framer"; @@ -455,4 +455,138 @@ afterAll(async () => { } expect(exitCode).toBe(0); }); + + test("retroactively reports a deeply nested describe() tree", async () => { + // describe() callbacks run from a queue, so nothing caps how deep the collected tree + // gets. The retroactive walk must not recurse once per level. Linux sizes the main + // thread's stack from `ulimit -s`: at the 1 MB pinned below, a native frame per level + // overflows it at this depth. A debug build has larger frames and spends about 0.5 ms + // per `found` event, so it gets a shallower tree. Release builds, sanitizer builds + // included, need the deep tree: their frames are small enough to survive 2_000 levels. + // + // This uses the WebSocket transport. The `found` events arrive as one burst, and the + // WebSocket writer queues messages across partial writes. + const depth = isDebug ? 2_000 : 20_000; + using dir = tempDir("test-reporter-deep-nesting", { + "deep.test.ts": ` +import { afterAll, describe, test, expect } from "bun:test"; +import { existsSync } from "node:fs"; + +let depth = ${depth}; +function nest() { + if (depth-- <= 0) { + test("leaf", async () => { + console.log("__LEAF_RUNNING__"); + while (!existsSync("leaf-gate")) await Bun.sleep(10); + expect(1).toBe(1); + }); + return; + } + describe("d" + depth, nest); +} +nest(); + +afterAll(async () => { + while (!existsSync("done-gate")) await Bun.sleep(10); +}); +`, + }); + + const leafGatePath = join(String(dir), "leaf-gate"); + const doneGatePath = join(String(dir), "done-gate"); + + const cmd = [bunExe(), "--inspect-wait=127.0.0.1:0", "test", "--timeout", "30000", "deep.test.ts"]; + proc = spawn({ + cmd: isLinux ? ["sh", "-c", `ulimit -s 1024 && exec "$@"`, "sh", ...cmd] : cmd, + env: bunEnv, + cwd: String(dir), + stdout: "pipe", + stderr: "pipe", + }); + + // The inspector prints `ws://127.0.0.1:/` on stderr, then waits for a client. + let stderr = ""; + const stderrReader = proc.stderr.getReader(); + const decoder = new TextDecoder(); + let url: string | undefined; + while (url === undefined) { + const { value, done } = await stderrReader.read(); + if (done) throw new Error(`bun test exited before it printed the inspector URL. stderr:\n${stderr}`); + stderr += decoder.decode(value, { stream: true }); + url = stderr.match(/(ws:\/\/\S+)\r?\n/)?.[1]; + } + const stderrDone = (async () => { + while (true) { + const { value, done } = await stderrReader.read(); + if (done) return; + stderr += decoder.decode(value, { stream: true }); + } + })(); + + const session = new TestReporterSession(); + let foundCount = 0; + session.addEventListener("TestReporter.found", () => void foundCount++); + + // A stack overflow is a silent SIGSEGV. Every wait below also ends, with the + // subprocess's stderr, when the subprocess exits. + const exitedEarly = proc.exited.then(async () => { + await stderrDone; + throw new Error( + `bun test exited (code ${proc!.exitCode}, signal ${proc!.signalCode}) after ${foundCount} of ${depth + 1} found events. stderr:\n${stderr}`, + ); + }); + exitedEarly.catch(() => {}); + const orExit = (promise: Promise) => Promise.race([promise, exitedEarly]); + + const ws = new WebSocket(url); + await orExit( + new Promise((resolve, reject) => { + ws.onopen = () => resolve(); + ws.onerror = cause => reject(new Error("WebSocket error", { cause })); + }), + ); + ws.onmessage = event => session.onMessage(String(event.data)); + session.framer = { send: (_socket: unknown, data: string) => ws.send(data) } as unknown as SocketFramer; + + session.enableInspector(); + session.send("Console.enable"); + const leafStarted = session.waitForConsoleMessage("__LEAF_RUNNING__", 15000); + session.initialize(); + + // Collection is done and the leaf test is running: every scope is discovered but none + // has been reported yet. + await orExit(leafStarted); + session.enableTestReporter(); + + // depth describes + 1 test, all via the retroactive walk. + const foundTests = await orExit(session.waitForFoundTests(depth + 1, 15000)); + expect(foundTests.size).toBe(depth + 1); + + // The outermost describe has no parent, each inner describe hangs off the one around + // it, and the leaf test off the innermost describe. + const byName = new Map([...foundTests.values()].map(t => [t.name, t])); + expect(byName.get(`d${depth - 1}`)).toEqual(expect.objectContaining({ type: "describe" })); + expect(byName.get(`d${depth - 1}`).parentId).toBeUndefined(); + const misparented: unknown[] = []; + for (let i = 0; i < depth; i++) { + const child = i === 0 ? byName.get("leaf") : byName.get(`d${i - 1}`); + const parent = byName.get(`d${i}`); + if (parent?.type !== "describe" || child?.parentId !== parent.id) misparented.push({ child, parent }); + } + expect(misparented).toEqual([]); + expect(byName.get("leaf").type).toBe("test"); + + await write(leafGatePath, "go"); + const endedTests = await orExit(session.waitForEndedTests(1, 15000)); + expect(endedTests.size).toBe(1); + + await write(doneGatePath, "go"); + const exitCode = await proc.exited; + await stderrDone; + ws.close(); + if (exitCode !== 0) { + expect(stderr).toBe(""); + } + expect(exitCode).toBe(0); + }); }); diff --git a/test/js/bun/test/describe.test.ts b/test/js/bun/test/describe.test.ts index ec0a6740de97..64d0df67e160 100644 --- a/test/js/bun/test/describe.test.ts +++ b/test/js/bun/test/describe.test.ts @@ -1,6 +1,6 @@ import { spawnSync } from "bun"; import { describe, expect, jest, test } from "bun:test"; -import { bunEnv, bunExe, tempDir } from "harness"; +import { bunEnv, bunExe, isLinux, tempDir } from "harness"; describe("blocks should handle a number, string, anonymous class, named class, or function for the first arg", () => { const numberMock = jest.fn(); @@ -223,3 +223,41 @@ describe("passing arrow function as args", () => { expect(fullOutput).toInclude("1 fail"); }); }); + +test("deeply nested describe() does not overflow the native stack", async () => { + // describe() callbacks run from a queue, so the fixture nests 10_000 scopes deep with no JS + // recursion. Scheduling and tearing down that tree must not recurse once per level either. + using test_dir = tempDir(".", { + "describe-deep.test.js": ` + import { describe, test, expect } from "bun:test"; + + let depth = 10000; + function nest() { + if (depth-- <= 0) { + test("leaf", () => { + expect(1).toBe(1); + }); + return; + } + describe("d" + depth, nest); + } + nest(); + `, + }); + const cmd = [bunExe(), "test", "./describe-deep.test.js"]; + await using proc = Bun.spawn({ + // Linux sizes the main thread's stack from `ulimit -s`. At the 1 MB pinned here, a native + // frame per level overflows it at this depth (a release build needs about 4_000 levels). + cmd: isLinux ? ["sh", "-c", `ulimit -s 1024 && exec "$@"`, "sh", ...cmd] : cmd, + cwd: String(test_dir), + stdout: "pipe", + stderr: "pipe", + env: bunEnv, + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect(stderr).toInclude(" > d2 > d1 > d0 > leaf"); + expect(stderr).toInclude("\n 1 pass"); + expect(stderr).toInclude("\n 0 fail"); + expect(exitCode).toBe(0); +});