diff --git a/src/js/node/test.ts b/src/js/node/test.ts index 79c0eba91e9b..1e483375f26f 100644 --- a/src/js/node/test.ts +++ b/src/js/node/test.ts @@ -922,6 +922,9 @@ const fileGeneration = $newRustFunction("jest.rs", "jsFileGeneration", 0); // `done` binds the intended sequence so a late call after the bun:test watchdog // moved on cannot write onto the currently-running test. const markCurrentResult = $newRustFunction("jest.rs", "jsNodeTestMarkResult", 2); +// Books a late subtest's parentAlreadyFinished failure into the reporter so the +// run prints it and exits 1. bun:test has no entry to fail for a late subtest. +const reportLateFailure = $newRustFunction("jest.rs", "jsNodeTestReportLateFailure", 2); let rootNode: TestNode | undefined; let rootGeneration = -1; @@ -1629,6 +1632,56 @@ function recordSuiteFailure(suite: TestNode, err: unknown) { suite.firstSubtestError ??= err ?? makeTestFailure("suite failed"); } +function runLateSubtest( + parent: TestNode, + name: string, + options: TestOptions, + fn: TestFn, + mode: "skip" | "todo" | undefined, + isSuite: boolean, +): Promise { + // isExecutionPhase=true keeps isRunning() true without `started`, so a nested + // t.test() inside the body goes to scheduleSubtest instead of recursing here. + const child = new TestNode(name, parent, options, isSuite, true); + if (mode === "todo" || options.todo) child.todoFlag = true; + if (mode === "skip" || options.skip) child.skipped = true; + const failure = makeTestFailure("test could not be started because its parent finished"); + (failure as { failureType?: string }).failureType = "parentAlreadyFinished"; + child.error = failure; + // Node still counts a late skip/todo as skip/todo (exit 0); only a plain late + // subtest flips the run to failing. + if (!child.todoFlag && !child.skipped) { + reportLateFailure(child.fullName, failure); + } + // Node replaces a {skip:true} body with a noop, so a late skip's body never + // runs; a late todo's body does (Node runs todo bodies). + if (child.skipped) return Promise.resolve(undefined); + // Node runs the body and awaits it; its outcome is discarded in favour of the + // parentAlreadyFinished failure above, and the returned promise resolves. A + // no-op done keeps a `(t, done)` body from throwing on `done()`. + const ctx = isSuite ? child.getSuiteCtx() : child.getCtx(); + let body: unknown; + try { + body = runWithNode(child, () => + fn.length === 2 ? fn.$call(undefined, ctx, kDefaultFunction) : fn.$call(undefined, ctx), + ); + } catch {} + // Mirror executeTestNode: drain nested subtests the body scheduled, then + // reset mocks so a late body's t.mock.method() does not leak into later + // tests (Node's postRun() does both). + const settle = () => { + child.finished = true; + try { + child.mockTracker?.reset(); + } catch {} + return undefined; + }; + return Promise.resolve(body) + .catch(kDefaultFunction) + .then(() => drainSubtestChain(child)) + .then(settle, settle); +} + // Awaits a node's subtest chain, including links appended while waiting. async function drainSubtestChain(node: TestNode) { let chain; @@ -1754,9 +1807,10 @@ function addTest( const runningNode = executionParent ?? currentNode(); if (runningNode !== undefined) { if (runningNode.finished) { - // t.test() escaped its parent: Node fails the late subtest but resolves - // the promise; don't fall through to bun:test's internal-phase throw. - return Promise.resolve(undefined); + // t.test() escaped its parent: Node runs the body, fails the late subtest + // with parentAlreadyFinished (resolving the returned promise), and reports + // it at the root so the run exits 1. + return runLateSubtest(runningNode, name, options, fn, mode, false); } if (runningNode.isRunning()) { // Subtest of a running test (or of an inline suite created inside one). @@ -1821,7 +1875,7 @@ function addSuite( const runningNode = executionParent ?? currentNode(); if (runningNode !== undefined && runningNode.finished) { - return Promise.resolve(undefined); + return runLateSubtest(runningNode, name, options, fn, mode, true); } if (runningNode !== undefined && runningNode.isRunning()) { const suite = new TestNode(name, runningNode, options, true, true); diff --git a/src/runtime/test_runner/jest.rs b/src/runtime/test_runner/jest.rs index bc605d9ab39b..d96eb125db75 100644 --- a/src/runtime/test_runner/jest.rs +++ b/src/runtime/test_runner/jest.rs @@ -605,6 +605,65 @@ pub(crate) fn js_node_test_mark_result( Ok(JSValue::UNDEFINED) } +/// Reached only from `node:test` when `t.test()` is called after its parent +/// finished: Node fails the late subtest with `parentAlreadyFinished` and exits +/// non-zero. bun:test has no entry for it, so book the failure here directly. +pub(crate) fn js_node_test_report_late_failure( + global: &JSGlobalObject, + callframe: &CallFrame, +) -> JsResult { + let [name, error] = callframe.arguments_as_array::<2>(); + let name_slice = name.to_slice(global)?; + let Some(buntest_strong) = bun_test::clone_active_strong() else { + return Ok(JSValue::UNDEFINED); + }; + let (rep, worker_idx) = { + // SAFETY: single-threaded JS VM; short-lived borrow for the + // reporter read and the on_before_print dot-break. + let buntest = unsafe { bun_test::buntest_as_mut(&buntest_strong) }; + let Some(rep) = buntest.reporter else { + return Ok(JSValue::UNDEFINED); + }; + buntest.bun_test_root.on_before_print(); + // SAFETY: same reporter write-provenance invariant as below. + (rep, unsafe { (*rep.as_ptr()).worker_ipc_file_idx }) + }; + let mut line = Vec::::new(); + crate::cli::test_command::write_test_status_line(super::execution::Result::Fail, &mut line); + let _ = writeln!(&mut line, " {}", bstr::BStr::new(name_slice.slice())); + if let Some(idx) = worker_idx { + crate::cli::test::parallel_runner::worker_emit_test_done(idx, &line); + } else { + let _ = Output::error_writer().write_all(&line); + Output::flush(); + } + if !error.is_empty_or_undefined_or_null() { + global.bun_vm().as_mut().run_error_handler(error, None); + Output::flush(); + } + // No JUnit entry: `maybe_print_junit_line` needs a bun:test sequence/entry + // that a late subtest doesn't have (same as `unhandled_errors_between_tests`). + // SAFETY: `BunTest.reporter` carries write provenance from `enter_file`'s + // `&mut`; single-threaded test runner, sole writer for this update. + let reporter: &mut CommandLineReporter = unsafe { &mut *rep.as_ptr() }; + if !reporter.reporters.dots && !reporter.reporters.only_failures { + reporter.failures_to_repeat_buf.extend_from_slice(&line); + } + reporter.summary().fail += 1; + if reporter.summary().fail == reporter.jest.bail { + reporter.print_summary(); + bun_core::pretty_error!( + "\nBailed out after {} failure{}\n", + reporter.jest.bail, + if reporter.jest.bail == 1 { "" } else { "s" } + ); + Output::flush(); + reporter.write_junit_report_if_needed(); + bun_core::Global::exit(1); + } + Ok(JSValue::UNDEFINED) +} + pub mod on_unhandled_rejection { use super::*; diff --git a/test/js/node/test_runner/fixtures/16-plan-and-late-subtest.js b/test/js/node/test_runner/fixtures/16-plan-and-late-subtest.js index 7548f769db44..a83f76a89684 100644 --- a/test/js/node/test_runner/fixtures/16-plan-and-late-subtest.js +++ b/test/js/node/test_runner/fixtures/16-plan-and-late-subtest.js @@ -22,7 +22,9 @@ test.describe("plan capture at first t.assert access", () => { // t.test() after the parent finished: Node fails the late subtest with // parentAlreadyFinished but resolves the returned promise (undefined); it must -// not reject or fall through to bun:test's internal-phase throw. +// not reject or fall through to bun:test's internal-phase throw. The late +// subtest is also booked as a run failure; see fixture 25 for the exit-code +// assertion. skip:true here keeps this fixture's own exit code at 0. test("late subtest after parent finished", async t => { let saved; await t.test("parent", pt => { @@ -30,7 +32,7 @@ test("late subtest after parent finished", async t => { }); let outcome; await saved - .test("late", () => {}) + .test("late", { skip: true }, () => {}) .then( v => (outcome = { resolved: true, value: v }), e => (outcome = { rejected: true, code: e?.code }), diff --git a/test/js/node/test_runner/fixtures/25-late-subtest-failure.js b/test/js/node/test_runner/fixtures/25-late-subtest-failure.js new file mode 100644 index 000000000000..bcc02ec176db --- /dev/null +++ b/test/js/node/test_runner/fixtures/25-late-subtest-failure.js @@ -0,0 +1,54 @@ +const { test } = require("node:test"); +const assert = require("node:assert"); + +// A t.test() that escapes its parent (the forgot-to-await shape): Node runs the +// body, resolves the returned promise with undefined, and records the late +// subtest as a parentAlreadyFinished failure so the run exits 1. Before the +// fix, bun resolved the promise but dropped the failure and exited 0. +let saved; +let bodyRan = false; +let doneBodyRan = false; +let suiteBodyRan = false; +let resolvedWith = "unset"; + +test("parent", t => { + saved = t; +}); + +test("observer", async () => { + const result = await saved.test("late", () => { + bodyRan = true; + }); + resolvedWith = result; + console.log("RESOLVED_WITH=" + String(resolvedWith)); + console.log("BODY_RAN=" + String(bodyRan)); + // A (t, done) body must receive a callable done so the body runs to completion. + await saved.test("late-done", (_t, done) => { + done(); + doneBodyRan = true; + }); + console.log("DONE_BODY_RAN=" + String(doneBodyRan)); + // t.describe() after the parent finished takes the same path (isSuite=true). + await saved.describe("late-suite", () => { + suiteBodyRan = true; + }); + console.log("SUITE_BODY_RAN=" + String(suiteBodyRan)); + // A late skip/todo subtest is not counted as a failure (Node exits 0 for + // those); these must not add further fail entries to this run. Node replaces + // a {skip:true} body with a noop, so it must not run; a {todo:true} body does. + let skipBodyRan = false; + let todoBodyRan = false; + await saved.test("late-skip", { skip: true }, () => { + skipBodyRan = true; + }); + await saved.test("late-todo", { todo: true }, () => { + todoBodyRan = true; + }); + console.log("SKIP_BODY_RAN=" + String(skipBodyRan)); + console.log("MARKED_BODY_RAN=" + String(todoBodyRan)); +}); + +process.on("exit", () => { + assert.strictEqual(bodyRan, true, "late subtest body must run (Node runs it)"); + assert.strictEqual(resolvedWith, undefined, "late subtest promise must resolve to undefined"); +}); diff --git a/test/js/node/test_runner/node-test.test.ts b/test/js/node/test_runner/node-test.test.ts index 2b480dddb050..9df69911100b 100644 --- a/test/js/node/test_runner/node-test.test.ts +++ b/test/js/node/test_runner/node-test.test.ts @@ -249,6 +249,28 @@ describe("node:test", () => { }); }); + test("should fail the run when t.test() is called after its parent finished", async () => { + const { exitCode, stdout, stderr } = await runTests(["25-late-subtest-failure.js"]); + expect(stdout).toContain("RESOLVED_WITH=undefined"); + expect(stdout).toContain("BODY_RAN=true"); + expect(stdout).toContain("DONE_BODY_RAN=true"); + expect(stdout).toContain("SUITE_BODY_RAN=true"); + expect(stdout).toContain("SKIP_BODY_RAN=false"); + expect(stdout).toContain("MARKED_BODY_RAN=true"); + expect(stderr).toContain("parent > late\n"); + expect(stderr).toContain("parent > late-done\n"); + expect(stderr).toContain("parent > late-suite\n"); + expect(stderr).toContain("test could not be started because its parent finished"); + // Only the plain late subtests fail; the late skip/todo ones do not. + expect(stderr).not.toContain("parent > late-skip"); + expect(stderr).not.toContain("parent > late-todo"); + expect(stderr).toContain("2 pass"); + expect({ exitCode, stderr }).toMatchObject({ + exitCode: 1, + stderr: expect.stringContaining("3 fail"), + }); + }); + test("should resolve the promise of a test that a name pattern filters out", async () => { const { exitCode, stderr } = await runTests(["23-filtered-test-promise.js"], {}, ["-t", "should resolve"]); expect(stderr).not.toContain("timed out");