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
62 changes: 58 additions & 4 deletions src/js/node/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<undefined> {
// 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),
);
Comment thread
robobun marked this conversation as resolved.
} 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);
}
Comment thread
robobun marked this conversation as resolved.

// Awaits a node's subtest chain, including links appended while waiting.
async function drainSubtestChain(node: TestNode) {
let chain;
Expand Down Expand Up @@ -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).
Expand Down Expand Up @@ -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);
Expand Down
59 changes: 59 additions & 0 deletions src/runtime/test_runner/jest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<JSValue> {
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::<u8>::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{}<r>\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::*;

Expand Down
6 changes: 4 additions & 2 deletions test/js/node/test_runner/fixtures/16-plan-and-late-subtest.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,17 @@ 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 => {
saved = pt;
});
let outcome;
await saved
.test("late", () => {})
.test("late", { skip: true }, () => {})
.then(
v => (outcome = { resolved: true, value: v }),
e => (outcome = { rejected: true, code: e?.code }),
Expand Down
54 changes: 54 additions & 0 deletions test/js/node/test_runner/fixtures/25-late-subtest-failure.js
Original file line number Diff line number Diff line change
@@ -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));
});
Comment thread
robobun marked this conversation as resolved.

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");
});
22 changes: 22 additions & 0 deletions test/js/node/test_runner/node-test.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
Loading