From 4c8300afb0a1d69f435f041bd38263fdeca467eb Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 16 Aug 2026 05:45:20 +0000 Subject: [PATCH 1/5] node:test: enforce the timeout and signal options of suites A describe()/suite() validated its timeout and signal options and then ignored them, so a suite whose timeout fired or whose signal aborted ran all of its children and passed. Node stops the suite: the running child and the queued ones are cancelled, the suite's after hooks still run, and the suite fails with the timeout or the abort reason; a suite whose signal has already aborted when its turn comes runs nothing at all. TestNode now tracks its children and carries the verdict machinery (fail/cancel/cancelSubtests, the context signal, and a cancellation promise that executeTestNode races the body against). Inline suites arm the stop at the head of their own subtest chain, after their before hooks; top-level suites, which bun:test drives as describe blocks, arm it from a beforeAll registered after the describe callback and release it from an afterAll registered before it, and report the suite's own error by failing a hook of the scope. Suites without these options are untouched. --- src/js/node/test.ts | 396 +++++++++++++++--- .../test_runner/fixtures/32-suite-timeout.js | 68 +++ .../test_runner/fixtures/33-suite-signal.js | 72 ++++ .../fixtures/34-inline-suite-stop.js | 91 ++++ test/js/node/test_runner/node-test.test.ts | 102 +++++ 5 files changed, 671 insertions(+), 58 deletions(-) create mode 100644 test/js/node/test_runner/fixtures/32-suite-timeout.js create mode 100644 test/js/node/test_runner/fixtures/33-suite-signal.js create mode 100644 test/js/node/test_runner/fixtures/34-inline-suite-stop.js diff --git a/src/js/node/test.ts b/src/js/node/test.ts index 75abf5fb79e3..3323a70494b4 100644 --- a/src/js/node/test.ts +++ b/src/js/node/test.ts @@ -425,7 +425,7 @@ function reportCancelledFile( column: 1, file: absolute, }; - const error = makeTestFailure("test did not finish before its parent and was cancelled", "cancelledByParent"); + const error = makeCancelledByParentError(); const details = { __proto__: null, duration_ms: 0, type: "test", error }; reporter.enqueue({ __proto__: null, ...fileNode }); reporter.dequeue({ __proto__: null, ...fileNode }); @@ -1386,6 +1386,16 @@ function makeTestFailure(message: string, failureType?: string) { return error; } +function makeCancelledByParentError() { + return makeTestFailure("test did not finish before its parent and was cancelled", "cancelledByParent"); +} + +// Port of Node's Test#abortHandler (test.js:1059): the `signal` option's reason +// is the verdict, with Node's own error standing in for a falsy one. +function abortFailure(signal: AbortSignal): unknown { + return signal.reason || $makeAbortError("The test was aborted"); +} + class TestPlan { expected: number; actual = 0; @@ -1544,13 +1554,32 @@ class TestNode { // Inline subtests are serialized through this chain. `concurrency` is // validated for Node-compat error codes but subtests always run serially. subtestChain: Promise = Promise.resolve(); + // Every child declared under this node, bun:test-driven and inline alike, so + // a suite that stops can sweep the ones it will not wait for. + subtests: TestNode[] = []; failedSubtests = 0; firstSubtestError: unknown = undefined; // First failure from a before hook created while this test was running. hookFailure: unknown = undefined; + // Set by fail() and cancel(). Like Node's first-error-wins Test#fail, it is + // the verdict no matter how the body settles afterwards. + cancelError: unknown = undefined; + // An enclosing suite has a `timeout` or `signal` option, so its stop may + // cancel this node while it is queued or running. + inStoppableSuite: boolean; + // Suites only. Node's Suite.run() sets startTime once the suite has passed + // its [kShouldAbort] check; a suite cancelled before that point runs neither + // of its hook sets. suiteStop is the suite's armed stopTest(). + suiteStarted = false; + suiteStop: StopController | undefined = undefined; #ctx: TestContext | undefined; #suiteCtx: SuiteContext | undefined; #tags: string[] | undefined; + // `t.signal`, created on first use; #aborted remembers an abort() that + // happened before anything asked for it. + #abortController: AbortController | undefined; + #aborted = false; + #cancellation: PromiseWithResolvers | undefined; constructor( name: string, @@ -1574,6 +1603,9 @@ class TestNode { if (typeof skip === "string") this.message = skip; else if (typeof todo === "string") this.message = todo; this.expectFailure = parseExpectFailure(options.expectFailure) || parent?.expectFailure || false; + this.inStoppableSuite = + parent !== undefined && (parent.inStoppableSuite || (parent.isSuite && hasStopOptions(parent.options))); + parent?.subtests.push(this); } get tags(): string[] { @@ -1614,6 +1646,61 @@ class TestNode { return this.#suiteCtx; } + get signal(): AbortSignal { + if (this.#abortController === undefined) { + this.#abortController = new AbortController(); + if (this.#aborted) this.#abortController.abort(); + } + return this.#abortController.signal; + } + + abort() { + this.#aborted = true; + this.#abortController?.abort(); + } + + // Port of Node's Test#fail for a verdict that arrives from outside the node's + // own run (a suite's stop, or the suite around it stopping): the first one + // wins, and a node that already finished keeps its verdict. Returns whether + // it took. + fail(error: unknown): boolean { + if (this.finished || this.cancelError !== undefined) return false; + this.cancelError = error; + this.#cancellation?.reject(error); + return true; + } + + // Port of Node's Test#cancel (test.js:1064): fail() plus aborting t.signal. + // `error` is the `signal` option's reason of a suite aborting itself; a node + // swept up because the suite around it stopped gets cancelledByParent. + cancel(error: unknown) { + if (this.fail(error ?? makeCancelledByParentError())) this.abort(); + } + + // For executeTestNode to race the body against: rejects with the cancelError + // as soon as fail() reaches this node, or right away if it already has. + cancellation(): Promise { + let pending = this.#cancellation; + if (pending === undefined) { + pending = this.#cancellation = Promise.withResolvers(); + // Swallow the rejection when nothing is racing it anymore. + pending.promise.catch(() => {}); + const { cancelError } = this; + if (cancelError !== undefined) pending.reject(cancelError); + } + return pending.promise; + } + + // Port of Node's postRun() sweep (test.js:1476): a suite that stopped cancels + // the children it will not wait for, and those cancel theirs. + cancelSubtests() { + for (const subtest of this.subtests) { + if (subtest.finished) continue; + subtest.cancel(undefined); + subtest.cancelSubtests(); + } + } + // True while user code reached from this node should treat new tests as // inline subtests instead of bun:test registrations. isRunning(): boolean { @@ -1666,7 +1753,6 @@ function getRootNode(): TestNode { */ class TestContext { #node: TestNode; - #abortController?: AbortController; #assert: Record | undefined; constructor(node: TestNode) { @@ -1674,10 +1760,7 @@ class TestContext { } get signal(): AbortSignal { - if (this.#abortController === undefined) { - this.#abortController = new AbortController(); - } - return this.#abortController.signal; + return this.#node.signal; } get name(): string { @@ -1828,17 +1911,13 @@ class TestContext { */ class SuiteContext { #node: TestNode; - #abortController?: AbortController; constructor(node: TestNode) { this.#node = node; } get signal(): AbortSignal { - if (this.#abortController === undefined) { - this.#abortController = new AbortController(); - } - return this.#abortController.signal; + return this.#node.signal; } get name(): string { @@ -1948,6 +2027,13 @@ function validateTimeoutAndSignal(options: TestOptions | HookOptions) { } } +// Whether Node's Suite.run() would arm a stop for these (validated) options. +// Suites have no default or inherited timeout (test.js:1763), so only an +// explicit finite one or a signal counts. +function hasStopOptions({ timeout, signal }: TestOptions): boolean { + return (typeof timeout === "number" && Number.isFinite(timeout)) || signal !== undefined; +} + // Port of Node's parseExpectFailure (test.js:528). A string is a label, a // function or RegExp validates the error, an object may carry both, and any // other object is itself the validation. @@ -2018,8 +2104,11 @@ function applyExpectFailure(node: TestNode, failure: unknown): unknown { function validateTestOptions(options: TestOptions): { ownTags: string[] | undefined } { const { concurrency, tags, plan } = options; - // signal and concurrency are validated for Node's error contract but not yet - // enforced (t.signal never aborts; subtests always run serially). + // Suites enforce timeout and signal (see startInlineSuite and + // registerTopLevelSuiteStart); tests enforce timeout in executeTestNode. A + // test's signal and concurrency anywhere are only validated for Node's error + // contract so far (t.signal aborts only when a suite cancels the test; + // subtests always run serially). validateTimeoutAndSignal(options); if (concurrency != null && typeof concurrency !== "boolean") { if (typeof concurrency === "number") { @@ -2133,22 +2222,42 @@ function invokeTestFn(fn: Function, arg: unknown) { return fn(arg); } -// A single timeout armed once per test and raced against both the body and -// plan.check(), matching Node's stopTest()/stopPromise. `promise` never -// resolves; it only rejects with the timeout error. Callers must dispose(). -function createStopController(timeout: number | undefined) { - if (typeof timeout !== "number" || !Number.isFinite(timeout)) { +let addAbortListener; + +type StopController = { promise: Promise; dispose(): void }; + +// Port of Node's stopTest()/stopPromise (test.js:145): armed once per test (and +// raced against both the body and plan.check()) or once per suite (stopping +// the suite's children). `promise` never resolves; it rejects with the timeout +// failure or, for a suite, with its `signal` option's reason. Callers must +// dispose(). +function createStopController(timeout: number | undefined, signal?: AbortSignal): StopController | undefined { + const hasTimeout = typeof timeout === "number" && Number.isFinite(timeout); + if (!hasTimeout && signal === undefined) { return undefined; } - let timer: ReturnType; + let timer: ReturnType | undefined; + let abortListener: Disposable | undefined; const promise = new Promise((_, reject) => { - // Not unref'd: dispose() always clears it, and on Windows an unref'd timer - // alone under bun:test leaves the uws loop inactive so auto_tick busy-spins. - timer = realSetTimeout(() => reject(makeTestFailure(`test timed out after ${timeout}ms`)), timeout); + if (hasTimeout) { + // Not unref'd: dispose() always clears it, and on Windows an unref'd timer + // alone under bun:test leaves the uws loop inactive so auto_tick busy-spins. + timer = realSetTimeout(() => reject(makeTestFailure(`test timed out after ${timeout}ms`)), timeout); + } + if (signal !== undefined) { + addAbortListener ??= require("internal/abort_listener").addAbortListener; + abortListener = addAbortListener(signal, () => reject(abortFailure(signal))); + } }); // Swallow the rejection when nothing is racing it anymore. promise.catch(() => {}); - return { promise, dispose: () => realClearTimeout(timer) }; + return { + promise, + dispose() { + if (timer !== undefined) realClearTimeout(timer); + abortListener?.[Symbol.dispose](); + }, + }; } // Runs `run` racing Node's test timeout; the timer starts before the body so a @@ -2160,8 +2269,6 @@ function awaitWithTimeout(run: () => unknown, timeout: number | undefined) { return raceWithTimeoutAndSignal(run, timeout, undefined); } -let addAbortListener; - async function raceWithTimeoutAndSignal( run: () => unknown, timeout: number | undefined, @@ -2259,9 +2366,27 @@ async function executeTestNode(node: TestNode, fn: TestFn): Promise { // test's own after hooks. Returns the failure (if any) instead of throwing. node.started = true; const started = runChildReporterEnabled ? performance.now() : 0; + let failure: unknown; + + const cancelledBeforeStart = node.cancelError; + if (cancelledBeforeStart !== undefined) { + // An enclosing suite stopped while this test was still waiting its turn: + // Node's run() goes straight to postRun() ([kShouldAbort], test.js:1306), + // so no hook and no body runs, and fail() still puts the cancellation + // through expectFailure. + failure = applyExpectFailure(node, cancelledBeforeStart); + node.passed = failure === undefined; + node.error = failure ?? cancelledBeforeStart; + node.finished = true; + reportNodeToRunParent(node, started); + return failure; + } + const ctx = node.getCtx(); const ancestors = ancestorChain(node); - let failure: unknown; + // Rejected when an enclosing suite stops while this test runs; racing it is + // what ends the test's turn so the suite's remaining children can report. + const cancellation = node.inStoppableSuite ? node.cancellation() : undefined; // Node applies the plan option before the beforeEach hooks run, and only for a // truthy count, so `{ plan: 0 }` installs no plan at all (test.js:1313-1315). @@ -2281,11 +2406,18 @@ async function executeTestNode(node: TestNode, fn: TestFn): Promise { failure = err; } - if (failure === undefined) { + // A suite that stopped during the beforeEach hooks has already failed this + // test; there is nothing left for the body to decide, so it never starts. + if (failure === undefined && node.cancelError === undefined) { // Node arms one stopPromise (timeout + signal) and races both the body // AND the plan wait against it. Arm timeout once here so plan({wait:true}) // is bounded by the same test timeout, not left unbounded. const stop = createStopController(node.options.timeout); + const stops: Promise[] = []; + if (stop !== undefined) stops.push(stop.promise); + if (cancellation !== undefined) stops.push(cancellation); + const untilStopped = (pending: Promise) => + stops.length === 0 ? pending : Promise.race([...stops, pending]); try { const runBody = async () => { await runWithNode(node, () => invokeTestFn(fn, ctx)); @@ -2295,7 +2427,7 @@ async function executeTestNode(node: TestNode, fn: TestFn): Promise { }; try { - await (stop === undefined ? runBody() : Promise.race([stop.promise, runBody()])); + await untilStopped(runBody()); } catch (err) { // A body that throws or rejects with a nullish value must still fail. failure = err ?? makeTestFailure("test failed"); @@ -2310,15 +2442,14 @@ async function executeTestNode(node: TestNode, fn: TestFn): Promise { try { const pending = plan.check(); if (pending !== undefined) { - // Defuse: if stop wins the race, plan's own wait-timeout may still + // Defuse: if a stop wins the race, plan's own wait-timeout may still // reject `pending` afterward with no one listening. pending.catch(() => {}); - await (stop === undefined ? pending : Promise.race([stop.promise, pending])); + await untilStopped(pending); // A t.test() that fulfilled the plan from an async callback was // scheduled onto subtestChain during the wait; drain again so its // failure reaches failedSubtests below (Node fails the parent). - const drain = drainSubtestChain(node); - await (stop === undefined ? drain : Promise.race([stop.promise, drain])); + await untilStopped(drainSubtestChain(node)); } } catch (err) { failure = err; @@ -2342,6 +2473,11 @@ async function executeTestNode(node: TestNode, fn: TestFn): Promise { } } + // Cancelled while running (its turn ended through the race above, or the + // sweep reached it between two awaits): Node's fail() keeps the first error, + // so however the body or a hook settled, the cancellation is the verdict. + failure = node.cancelError ?? failure; + const bodyFailure = failure; failure = applyExpectFailure(node, failure); const acceptedXfail = bodyFailure !== undefined && failure === undefined; @@ -2395,7 +2531,10 @@ function scheduleSubtest(parent: TestNode, child: TestNode, fn: TestFn, ownTodo: } let failure: unknown; try { - await runOwnBeforeHooks(parent); + // A subtest swept while it was still queued is not what triggers the + // before hooks (its suite may never have started); executeTestNode just + // reports it. + if (child.cancelError === undefined) await runOwnBeforeHooks(parent); failure = await executeTestNode(child, fn); } catch (err) { failure = err; @@ -2430,10 +2569,68 @@ async function drainSubtestChain(node: TestNode) { } while (chain !== node.subtestChain); } +// Port of Node's [kShouldAbort] (test.js:1245) for a suite whose turn has come: +// it was swept by a suite around it that stopped, or its own `signal` option +// has aborted by now (Node's listener cancels it the moment the signal aborts; +// looking when the suite's turn comes gives the same verdict). Node then runs +// nothing of it, hooks included, and drops its children (test.js:1854). The +// children are cancelled here instead, so the ones the subtest chain or +// bun:test still reaches report without running. +function suiteShouldAbort(suite: TestNode): boolean { + if (suite.cancelError === undefined) { + const { signal } = suite.options; + if (!signal?.aborted) return false; + suite.cancel(abortFailure(signal)); + } + suite.cancelSubtests(); + return true; +} + +// Port of the stopTest() call in Node's Suite.run() (test.js:1865). Node runs +// the after hooks first and sweeps the children in postRun(); the sweep comes +// first here because it is what ends the running child's turn, after which the +// queued children report as cancelled, the chain (or bun:test's scope) drains +// on its own, and the after hooks follow. +function armSuiteStop(suite: TestNode) { + const { timeout, signal } = suite.options; + const stop = createStopController(timeout, signal); + if (stop === undefined) return; + suite.suiteStop = stop; + stop.promise.catch(error => { + stop.dispose(); + // A timeout merely fails the suite (test.js:1876); only its `signal` option + // cancels it, aborting the suite's own signal as well (the listener from + // test.js:724 runs Test#cancel). The children are swept either way. + if (signal?.aborted) suite.cancel(error); + else suite.fail(error); + suite.cancelSubtests(); + }); +} + +// First half of Node's Suite.run() for an inline suite: chained at the head of +// the suite's own subtestChain so the children queued behind it see its +// outcome. scheduleSuiteSubtest's run() is the second half. Must not reject: +// a rejected link would skip every child queued behind it. +async function startInlineSuite(suite: TestNode) { + if (suiteShouldAbort(suite)) return; + suite.suiteStarted = true; + try { + await runOwnBeforeHooks(suite); + } catch (err) { + // A failing suite-level before hook fails the suite, like Node. Its + // children fail through the same memoized rejection, so there is nothing + // left for a stop to cut short. + recordSuiteFailure(suite, err); + return; + } + armSuiteStop(suite); +} + function scheduleSuiteSubtest(parent: TestNode, suite: TestNode, build: unknown, ownTodo: boolean): Promise { // A describe()/suite() created while a test is running becomes a suite // subtest: its children were collected eagerly when the callback ran and are - // already chained on the suite's own subtestChain; failures roll up here. + // already chained on the suite's own subtestChain behind startInlineSuite; + // failures roll up here. const run = async () => { if (build !== undefined) { try { @@ -2444,23 +2641,27 @@ function scheduleSuiteSubtest(parent: TestNode, suite: TestNode, build: unknown, recordSuiteFailure(suite, err); } } - try { - await runOwnBeforeHooks(suite); - } catch (err) { - // A failing suite-level before hook fails the suite, like Node. - recordSuiteFailure(suite, err); - } - // Wait for children created during the callback and any they schedule. + // Wait for startInlineSuite, the children created during the callback, and + // any they schedule. A stop ends the running child's turn and the rest + // report as cancelled, so a stopped suite drains too. await drainSubtestChain(suite); - for (const hook of suite.hooks.after) { - try { - await runHook(hook, suite, suite.getSuiteCtx()); - } catch (err) { - recordSuiteFailure(suite, err); + suite.suiteStop?.dispose(); + // Like Node, a suite cancelled before it started skips its after hooks along + // with everything else, while one stopped midway still runs them. + if (suite.suiteStarted) { + for (const hook of suite.hooks.after) { + try { + await runHook(hook, suite, suite.getSuiteCtx()); + } catch (err) { + recordSuiteFailure(suite, err); + } } } suite.finished = true; - suite.passed = suite.failedSubtests === 0; + // Being stopped fails the suite even when no child failed (one aborted + // before it started ran none), and is what Node reports for it. + const { cancelError } = suite; + suite.passed = cancelError === undefined && suite.failedSubtests === 0; if (runChildReporterEnabled) { emitRunChildEvent(suite.passed ? "test:pass" : "test:fail", { __proto__: null, @@ -2474,17 +2675,18 @@ function scheduleSuiteSubtest(parent: TestNode, suite: TestNode, build: unknown, error: suite.passed ? undefined : serializeRunError( - makeTestFailure( - `${suite.failedSubtests} subtest${suite.failedSubtests > 1 ? "s" : ""} failed`, - "subtestsFailed", - ), + cancelError ?? + makeTestFailure( + `${suite.failedSubtests} subtest${suite.failedSubtests > 1 ? "s" : ""} failed`, + "subtestsFailed", + ), ), }); } // A todo suite's failures do not fail the owning test (Node). - if (suite.failedSubtests > 0 && !ownTodo) { + if (!suite.passed && !ownTodo) { parent.failedSubtests++; - parent.firstSubtestError ??= suite.firstSubtestError; + parent.firstSubtestError ??= cancelError ?? suite.firstSubtestError; } }; const result = (parent.subtestChain = parent.subtestChain.then(run)); @@ -2517,6 +2719,58 @@ function bunTestOptions(options: TestOptions) { return undefined; } +// A top-level suite is a bun:test describe block whose children bun:test runs +// itself, so the suite's half of Node's Suite.run() is spread over hooks of +// that scope. addSuite registers them around the describe callback so that they +// bracket the suite's own before()/after() hooks, which are bun:test hooks too +// and run in registration order. Only a suite with a stop of its own, or one +// inside such a suite, gets them; every other suite stays a plain describe. +// +// Like every test and hook callback this module hands to bun:test, the hooks +// take `done`: a nested describe callback runs inside its parent's async +// context (the shim's AsyncLocalStorage), and bun:test cannot read the arity +// of a callback registered under one, so it waits for done() either way. They +// fail by throwing, though: an error passed to done() is only printed, while a +// throw fails the hook itself. That is what reports the suite's own error +// against the suite (a describe block has no verdict of its own, and its +// cancelled children only say that their parent stopped) and, from a +// beforeAll, what makes bun:test skip the scope's tests. A todo suite's failure +// is not a failure in Node: its children report themselves as todo under a +// run() child, and sit in a describe.todo scope under plain bun:test. +function registerTopLevelSuiteFinish(suite: TestNode) { + // Registered before the callback, so it runs before the suite's after hooks: + // once the children are done the stop can no longer fail the suite (Node + // ignores one that fires during the after hooks), and its timer is released + // whatever those hooks do. + bunTest().afterAll((done: (error?: unknown) => void) => { + suite.suiteStop?.dispose(); + done(); + }); +} + +function registerTopLevelSuiteStart(suite: TestNode) { + const { beforeAll, afterAll } = bunTest(); + // Registered after the callback, so the suite's before hooks have already run + // by now; like in Node they do not count against the suite's timeout. + beforeAll((done: (error?: unknown) => void) => { + if (!suiteShouldAbort(suite)) { + suite.suiteStarted = true; + armSuiteStop(suite); + } else if (!suite.todoFlag) { + // Node drops the children of a suite that never starts. + throw suite.cancelError; + } + done(); + }); + // Runs after the suite's after hooks, where Node fails the suite as well. + afterAll((done: (error?: unknown) => void) => { + const stopped = suite.suiteStarted && suite.cancelError !== undefined; + suite.finished = true; + if (stopped && !suite.todoFlag) throw suite.cancelError; + done(); + }); +} + function currentCollectionParent(): TestNode { const node = currentNode(); if (node !== undefined && !node.isExecutionPhase && node.isSuite) { @@ -2682,11 +2936,12 @@ function addSuite( if (ownTodo) suite.todoFlag = true; // The suite's children must run after the parent's previously scheduled // subtests AND after the describe callback's own returned promise settles - // (Node's Suite.run awaits buildPromise before iterating subtests). The - // callback has not returned yet so its promise does not exist; seed the - // chain through a gate the callback's settlement opens. + // (Node's Suite.run awaits buildPromise before iterating subtests), and + // then behind the suite's own start (its abort check, before hooks, and + // stop). The callback has not returned yet so its promise does not exist; + // seed the chain through a gate the callback's settlement opens. const gate = Promise.withResolvers(); - suite.subtestChain = runningNode.subtestChain.then(() => gate.promise); + suite.subtestChain = runningNode.subtestChain.then(() => gate.promise).then(() => startInlineSuite(suite)); // Build the suite eagerly (Node also runs describe callbacks immediately), // collecting children onto the suite's own subtest chain. let build: unknown; @@ -2718,13 +2973,27 @@ function addSuite( // describe.todo(name, { skip: true }, fn) is a skip. const effectiveMode = mode === "skip" || options.skip ? "skip" : mode === "todo" || options.todo ? "todo" : undefined; + const stoppable = hasStopOptions(options) || suiteNode.inStoppableSuite; // Node never invokes a skipped suite's callback (it does run a todo one), so // the children are never declared and side effects in the body never happen. const wrapped = effectiveMode === "skip" ? kDefaultFunction : () => { - return runWithNode(suiteNode, () => fn(suiteNode.getSuiteCtx())); + if (stoppable) registerTopLevelSuiteFinish(suiteNode); + const built = runWithNode(suiteNode, () => fn(suiteNode.getSuiteCtx())); + if (!stoppable) return built; + if (built != null && typeof (built as PromiseLike).then === "function") { + // bun:test keeps the scope open until the callback's promise + // settles, so hooks registered from here still land in it, behind + // any the callback registered after awaiting something. + return (built as PromiseLike).then(value => { + registerTopLevelSuiteStart(suiteNode); + return value; + }); + } + registerTopLevelSuiteStart(suiteNode); + return built; }; const passOptions = bunTestOptions(options); @@ -2805,6 +3074,11 @@ function before(arg0: unknown, arg1: unknown) { } const { beforeAll } = bunTest(); beforeAll((done: (error?: unknown) => void) => { + // Node runs no hook of a suite that is cancelled by the time it would start. + if (suiteShouldAbort(owner)) { + done(); + return; + } Promise.resolve(runHook(hook, owner, hookArgFor(owner))).then( () => done(), err => done(err ?? new Error("before hook failed")), @@ -2821,6 +3095,12 @@ function after(arg0: unknown, arg1: unknown) { } const { afterAll } = bunTest(); afterAll((done: (error?: unknown) => void) => { + // Same as in before(): a suite that never started skips its after hooks + // too, while one that was stopped midway still runs them, like Node. + if (!owner.suiteStarted && suiteShouldAbort(owner)) { + done(); + return; + } Promise.resolve(runHook(hook, owner, hookArgFor(owner))).then( () => done(), err => done(err ?? new Error("after hook failed")), diff --git a/test/js/node/test_runner/fixtures/32-suite-timeout.js b/test/js/node/test_runner/fixtures/32-suite-timeout.js new file mode 100644 index 000000000000..384ff0d3f5c6 --- /dev/null +++ b/test/js/node/test_runner/fixtures/32-suite-timeout.js @@ -0,0 +1,68 @@ +// The `timeout` option of a top-level suite, as in Node v26.3.0's Suite.run(): +// when it fires, the child that is running is cancelled, the children and +// nested suites queued behind it never run, the suite's own after hooks still +// run (with the suite's own signal left alone: a timeout fails a suite, it does +// not abort it), and the suite itself fails with the timeout. The first suite +// here fails on purpose; node-test.test.ts asserts the exact counts and the +// OBS markers. Differences from `node --test` with this file: a suite's own +// failure shows up as a failed hook of its describe block, the children of a +// suite that never started are skipped rather than listed one by one, and the +// running child is cancelled before the after hooks run rather than after. +const { describe, it, before, after } = require("node:test"); + +const sleep = ms => new Promise(resolve => setTimeout(resolve, ms)); +// A cancelled test's t.signal aborts; the sleep only bounds a runner that never +// cancels, so that such a runner reports the wrong outcome instead of hanging. +const untilCancelled = t => + Promise.race([new Promise(resolve => t.signal.addEventListener("abort", resolve)), sleep(200)]); + +describe("suite that times out", { timeout: 20 }, () => { + before(() => console.log("OBS before ran")); + after(suite => console.log("OBS after ran, suite signal aborted: " + suite.signal.aborted)); + + it("running when the suite times out", async t => { + await untilCancelled(t); + console.log("OBS running child settled, signal aborted: " + t.signal.aborted); + }); + + it("queued when the suite times out", () => { + console.log("OBS queued child ran"); + }); + + describe("nested suite queued when the suite times out", () => { + before(() => console.log("OBS nested before ran")); + after(() => console.log("OBS nested after ran")); + it("nested child", () => console.log("OBS nested child ran")); + }); +}); + +// Also pins that the stop is released once the suite is done: a leaked 30s +// timer would keep the run() child of node-test.test.ts alive. +describe("suite within its timeout", { timeout: 30_000 }, () => { + it("passes", () => {}); + + // A test's own timeout still applies inside a suite that has one (this test + // fails on purpose), and it does not stop the suite around it. + it("has a shorter timeout of its own", { timeout: 20 }, async () => { + await sleep(200); + }); + + it("still runs after a sibling timed out on its own", () => {}); + + describe("nested suite without a timeout of its own", () => { + it("passes too", () => {}); + }); +}); + +// Node arms the suite's stop after its before hooks, so they do not count +// against the timeout. Had the stop been armed before the hook, its shorter +// timer would have fired during the hook and the child would have been +// cancelled without ever starting. The async callback also covers registering +// the suite's own bookkeeping after the callback's promise settles. +describe("suite whose before hook outlasts its timeout", { timeout: 1000 }, async () => { + await null; + before(() => sleep(1100)); + it("runs after the slow before hook", () => { + console.log("OBS child ran after the slow before hook"); + }); +}); diff --git a/test/js/node/test_runner/fixtures/33-suite-signal.js b/test/js/node/test_runner/fixtures/33-suite-signal.js new file mode 100644 index 000000000000..06cebbc255bd --- /dev/null +++ b/test/js/node/test_runner/fixtures/33-suite-signal.js @@ -0,0 +1,72 @@ +// The `signal` option of a top-level suite, as in Node v26.3.0. A suite whose +// signal has aborted by the time its turn comes (before it was declared, or by +// an earlier test) fails with the reason and runs nothing: no hooks, no +// children, although its describe callback does run. A signal that aborts +// while the suite runs cancels the running child and the queued one, still +// runs the after hooks, and fails the suite with the reason; a falsy reason is +// reported as Node's own "The test was aborted". Four suites here fail on +// purpose; node-test.test.ts asserts the exact counts and the OBS markers. +// Differences from `node --test` with this file: a suite's own failure shows up +// as a failed hook of its describe block, the children of a suite that never +// started are skipped rather than listed one by one, and the running child is +// cancelled before the after hooks run rather than after. +const { describe, it, before, after, test } = require("node:test"); + +const sleep = ms => new Promise(resolve => setTimeout(resolve, ms)); +// A cancelled test's t.signal aborts; the sleep only bounds a runner that never +// cancels, so that such a runner reports the wrong outcome instead of hanging. +const untilCancelled = t => + Promise.race([new Promise(resolve => t.signal.addEventListener("abort", resolve)), sleep(200)]); + +describe( + "suite aborted before it was declared", + { signal: AbortSignal.abort(new Error("aborted before declaration")) }, + () => { + console.log("OBS pre-aborted suite callback ran"); + before(() => console.log("OBS pre-aborted suite before ran")); + after(() => console.log("OBS pre-aborted suite after ran")); + it("child of the pre-aborted suite", () => console.log("OBS pre-aborted suite child ran")); + }, +); + +const abortedByEarlierTest = new AbortController(); + +test("aborts the next suite's signal before that suite runs", () => { + abortedByEarlierTest.abort(new Error("aborted by an earlier test")); +}); + +describe("suite aborted before it ran", { signal: abortedByEarlierTest.signal }, () => { + it("child of the suite aborted before it ran", () => console.log("OBS aborted-before-run suite child ran")); +}); + +const abortedWhileRunning = new AbortController(); + +describe("suite aborted while running", { signal: abortedWhileRunning.signal }, () => { + after(suite => console.log("OBS after ran, suite signal aborted: " + suite.signal.aborted)); + + it("finished before the abort", () => console.log("OBS first child ran")); + + it("running when the signal aborts", async t => { + abortedWhileRunning.abort(new Error("aborted while running")); + await untilCancelled(t); + console.log("OBS running child settled, signal aborted: " + t.signal.aborted); + }); + + it("queued when the signal aborts", () => console.log("OBS queued child ran")); +}); + +describe("suite aborted with a falsy reason", { signal: AbortSignal.abort(0) }, () => { + it("child of the suite aborted with a falsy reason", () => console.log("OBS falsy-reason suite child ran")); +}); + +// The listener is removed once the suite is done; aborting afterwards changes +// nothing (node-test.test.ts only checks that this suite passes). +const neverAbortedInTime = new AbortController(); + +describe("suite whose signal does not abort while it runs", { signal: neverAbortedInTime.signal }, () => { + it("passes", () => {}); +}); + +test("aborting a finished suite's signal is a no-op", () => { + neverAbortedInTime.abort(new Error("too late to matter")); +}); diff --git a/test/js/node/test_runner/fixtures/34-inline-suite-stop.js b/test/js/node/test_runner/fixtures/34-inline-suite-stop.js new file mode 100644 index 000000000000..8cb905306ed2 --- /dev/null +++ b/test/js/node/test_runner/fixtures/34-inline-suite-stop.js @@ -0,0 +1,91 @@ +// The `timeout` and `signal` options of inline suites (describe() inside a +// running test), as in Node v26.3.0: a suite that times out is failed by the +// timeout, its running child is cancelled, the children and nested suites +// queued behind it never run, and its after hooks still run (with the suite's +// own signal left alone: a timeout fails a suite, it does not abort it); a +// suite whose signal has already aborted is failed by the reason and runs +// nothing. Both fail the test that owns them, so the first test fails on +// purpose ("2 subtests failed"); the last test checks what the suites left +// behind, and passes verbatim under `node --test` too. node-test.test.ts also +// runs this file through run(), which reports each suite's own error. One +// difference from Node there: Node drops the children of a suite that was +// already aborted without reporting them, while here they report as cancelled. +const assert = require("node:assert"); +const { test, describe, it, before, after } = require("node:test"); + +const sleep = ms => new Promise(resolve => setTimeout(resolve, ms)); +// A cancelled test's t.signal aborts; the sleep only bounds a runner that never +// cancels, so that such a runner reports the wrong outcome instead of hanging. +const untilCancelled = t => + Promise.race([new Promise(resolve => t.signal.addEventListener("abort", resolve)), sleep(200)]); + +const seen = { + hooks: [], + runningChildSignalAborted: undefined, + queuedChildRan: false, + nestedChildRan: false, + abortedSuiteChildRan: false, + childWithinTimeoutRan: false, + childAfterSlowBeforeHookRan: false, +}; + +test("inline suites are stopped by their timeout or an aborted signal", () => { + describe("times out", { timeout: 20 }, () => { + before(() => seen.hooks.push("before")); + after(suite => seen.hooks.push("after, suite signal aborted: " + suite.signal.aborted)); + it("running when the suite times out", async t => { + await untilCancelled(t); + seen.runningChildSignalAborted = t.signal.aborted; + }); + it("queued when the suite times out", () => { + seen.queuedChildRan = true; + }); + describe("nested suite queued when the suite times out", () => { + before(() => seen.hooks.push("nested before")); + after(() => seen.hooks.push("nested after")); + it("nested child", () => { + seen.nestedChildRan = true; + }); + }); + }); + + describe("already aborted", { signal: AbortSignal.abort(new Error("inline abort reason")) }, () => { + before(() => seen.hooks.push("aborted suite before")); + after(() => seen.hooks.push("aborted suite after")); + it("child of the aborted suite", () => { + seen.abortedSuiteChildRan = true; + }); + }); + + describe("within its timeout", { timeout: 30_000 }, () => { + it("passes", () => { + seen.childWithinTimeoutRan = true; + }); + }); +}); + +// Node arms the suite's stop after its before hooks, so they do not count +// against the timeout. Had the stop been armed before the hook, its shorter +// timer would have fired during the hook and the child would have been +// cancelled without ever starting, failing this test as well. +test("an inline suite's before hooks do not count against its timeout", () => { + describe("before hook outlasts the timeout", { timeout: 1000 }, () => { + before(() => sleep(1100)); + it("runs after the slow before hook", () => { + seen.childAfterSlowBeforeHookRan = true; + }); + }); +}); + +test("what the stopped inline suites left behind", () => { + // Each test above waited for its suites before it finished. + assert.deepStrictEqual(seen, { + hooks: ["before", "after, suite signal aborted: false"], + runningChildSignalAborted: true, + queuedChildRan: false, + nestedChildRan: false, + abortedSuiteChildRan: false, + childWithinTimeoutRan: true, + childAfterSlowBeforeHookRan: true, + }); +}); diff --git a/test/js/node/test_runner/node-test.test.ts b/test/js/node/test_runner/node-test.test.ts index 2a27963203c0..46bd3ca1c2fc 100644 --- a/test/js/node/test_runner/node-test.test.ts +++ b/test/js/node/test_runner/node-test.test.ts @@ -2,6 +2,7 @@ import { spawn } from "bun"; import { describe, expect, test } from "bun:test"; import { bunEnv, bunExe } from "harness"; import { join } from "node:path"; +import { run } from "node:test"; describe("node:test", () => { // These three drive the largest fixtures (01-harness has 32 node:test cases); @@ -323,6 +324,107 @@ describe("node:test", () => { stderr: expect.stringContaining("0 fail"), }); }); + + test("should enforce a top-level suite's signal option", async () => { + const { exitCode, stdout, stderr } = await runTests(["33-suite-signal.js"]); + const markers = stdout.split("\n").filter(line => line.startsWith("OBS ")); + expect(markers).toEqual([ + "OBS pre-aborted suite callback ran", + "OBS first child ran", + "OBS running child settled, signal aborted: true", + "OBS after ran, suite signal aborted: true", + ]); + // Each aborted suite fails with its signal's reason. + expect(stderr).toContain("aborted before declaration"); + expect(stderr).toContain("aborted by an earlier test"); + expect(stderr).toContain("aborted while running"); + expect(stderr).toContain("The test was aborted"); + expect(stderr).toContain("test did not finish before its parent and was cancelled"); + expect(stderr).not.toContain("too late to matter"); + // The deliberate failures: four suites plus the two children cancelled by + // the abort while running. + expect(stderr).toContain("4 pass"); + expect({ exitCode, stderr }).toMatchObject({ + exitCode: 1, + stderr: expect.stringContaining("6 fail"), + }); + }); + + // Fixtures 32 and 34 each hold a before hook that deliberately outlasts a 1s + // suite timeout, on top of the debug+ASAN child's startup, so like the + // fixtures at the top they get headroom and overlap with each other. + test.concurrent( + "should enforce a top-level suite's timeout option", + async () => { + const { exitCode, stdout, stderr } = await runTests(["32-suite-timeout.js"]); + const markers = stdout.split("\n").filter(line => line.startsWith("OBS ")); + expect(markers).toEqual([ + "OBS before ran", + "OBS running child settled, signal aborted: true", + "OBS after ran, suite signal aborted: false", + "OBS child ran after the slow before hook", + ]); + // The suite's own verdict, and the verdict of the children it stopped. + expect(stderr).toContain("test timed out after 20ms"); + expect(stderr).toContain("test did not finish before its parent and was cancelled"); + // The deliberate failures: the running child, the queued child, the + // queued nested suite, the suite itself, and the test with a timeout of + // its own. + expect(stderr).toContain("4 pass"); + expect({ exitCode, stderr }).toMatchObject({ + exitCode: 1, + stderr: expect.stringContaining("5 fail"), + }); + }, + 30_000, + ); + + test.concurrent( + "should enforce an inline suite's timeout and signal options", + async () => { + const { exitCode, stderr } = await runTests(["34-inline-suite-stop.js"]); + // The owning test fails for the two stopped suites, with the timeout as + // the reported cause; the before-hook test and the one inspecting what + // the suites left behind pass. + expect(stderr).toContain("error: 2 subtests failed"); + expect(stderr).toContain("test timed out after 20ms"); + expect(stderr).toContain("2 pass"); + expect({ exitCode, stderr }).toMatchObject({ + exitCode: 1, + stderr: expect.stringContaining("1 fail"), + }); + }, + 30_000, + ); + + test.concurrent( + "should report a stopped inline suite's own error and its cancelled children through run()", + async () => { + const events = await run({ files: [join(import.meta.dirname, "fixtures", "34-inline-suite-stop.js")] }).toArray(); + const outcomes = events + .filter(({ type }) => type === "test:pass" || type === "test:fail") + .map(({ type, data }) => [data.name, type === "test:pass" ? "pass" : data.details.error.message]) + .sort(([a], [b]) => (a < b ? -1 : a > b ? 1 : 0)); + const cancelled = "test did not finish before its parent and was cancelled"; + expect(outcomes).toEqual([ + ["already aborted", "inline abort reason"], + ["an inline suite's before hooks do not count against its timeout", "pass"], + ["before hook outlasts the timeout", "pass"], + ["child of the aborted suite", cancelled], + ["inline suites are stopped by their timeout or an aborted signal", "2 subtests failed"], + ["nested child", cancelled], + ["nested suite queued when the suite times out", cancelled], + ["passes", "pass"], + ["queued when the suite times out", cancelled], + ["running when the suite times out", cancelled], + ["runs after the slow before hook", "pass"], + ["times out", "test timed out after 20ms"], + ["what the stopped inline suites left behind", "pass"], + ["within its timeout", "pass"], + ]); + }, + 30_000, + ); }); async function runTests(filenames: string[], env: Record = {}, args: string[] = []) { From 370a971d435676234de0b1be1af6709154c44ade Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 16 Aug 2026 06:26:33 +0000 Subject: [PATCH 2/5] node:test: keep a suite started by the time its before hooks run The abort check and the started flag of a top-level suite moved into a beforeAll registered ahead of the suite's own before hooks, leaving only the arming of the stop behind them, and armSuiteStop re-checks for a suite that was stopped while those hooks ran. A signal aborted by one of the suite's before hooks now stops the suite the way Node does: the remaining before hooks and the after hooks still run, the children are cancelled without starting, and the suite fails with the reason; previously such a suite was treated as never started and its after hooks were skipped. --- src/js/node/test.ts | 117 ++++++++++-------- .../test_runner/fixtures/33-suite-signal.js | 21 +++- .../fixtures/34-inline-suite-stop.js | 37 ++++-- test/js/node/test_runner/node-test.test.ts | 18 ++- 4 files changed, 129 insertions(+), 64 deletions(-) diff --git a/src/js/node/test.ts b/src/js/node/test.ts index 3323a70494b4..2a5c221e2d26 100644 --- a/src/js/node/test.ts +++ b/src/js/node/test.ts @@ -2104,11 +2104,11 @@ function applyExpectFailure(node: TestNode, failure: unknown): unknown { function validateTestOptions(options: TestOptions): { ownTags: string[] | undefined } { const { concurrency, tags, plan } = options; - // Suites enforce timeout and signal (see startInlineSuite and - // registerTopLevelSuiteStart); tests enforce timeout in executeTestNode. A - // test's signal and concurrency anywhere are only validated for Node's error - // contract so far (t.signal aborts only when a suite cancels the test; - // subtests always run serially). + // Suites enforce timeout and signal (see startInlineSuite and the top-level + // suite hooks registered by addSuite); tests enforce timeout in + // executeTestNode. A test's signal and concurrency anywhere are only + // validated for Node's error contract so far (t.signal aborts only when a + // suite cancels the test; subtests always run serially). validateTimeoutAndSignal(options); if (concurrency != null && typeof concurrency !== "boolean") { if (typeof concurrency === "number") { @@ -2569,13 +2569,15 @@ async function drainSubtestChain(node: TestNode) { } while (chain !== node.subtestChain); } -// Port of Node's [kShouldAbort] (test.js:1245) for a suite whose turn has come: -// it was swept by a suite around it that stopped, or its own `signal` option -// has aborted by now (Node's listener cancels it the moment the signal aborts; -// looking when the suite's turn comes gives the same verdict). Node then runs -// nothing of it, hooks included, and drops its children (test.js:1854). The -// children are cancelled here instead, so the ones the subtest chain or -// bun:test still reaches report without running. +// Whether the suite is already stopped at one of the two points where Node's +// Suite.run() would notice: when its turn comes ([kShouldAbort], test.js:1245; +// it then runs nothing, hooks included) and again after its before hooks +// (test.js:1866; it then starts no child, but still runs its after hooks). +// Either it was swept by a suite around it that stopped, or its own `signal` +// option has aborted by now (Node's listener cancels it the moment the signal +// aborts; looking at these two points gives the same verdicts). Node drops the +// children of such a suite; they are cancelled here instead, so the ones the +// subtest chain or bun:test still reaches report without running. function suiteShouldAbort(suite: TestNode): boolean { if (suite.cancelError === undefined) { const { signal } = suite.options; @@ -2586,12 +2588,14 @@ function suiteShouldAbort(suite: TestNode): boolean { return true; } -// Port of the stopTest() call in Node's Suite.run() (test.js:1865). Node runs -// the after hooks first and sweeps the children in postRun(); the sweep comes -// first here because it is what ends the running child's turn, after which the -// queued children report as cancelled, the chain (or bun:test's scope) drains -// on its own, and the after hooks follow. +// Port of the stopTest() call in Node's Suite.run() (test.js:1865), made once +// the suite's before hooks have run. Node runs the after hooks first and sweeps +// the children in postRun() when the stop fires; the sweep comes first here +// because it is what ends the running child's turn, after which the queued +// children report as cancelled, the chain (or bun:test's scope) drains on its +// own, and the after hooks follow. function armSuiteStop(suite: TestNode) { + if (suiteShouldAbort(suite)) return; const { timeout, signal } = suite.options; const stop = createStopController(timeout, signal); if (stop === undefined) return; @@ -2721,10 +2725,14 @@ function bunTestOptions(options: TestOptions) { // A top-level suite is a bun:test describe block whose children bun:test runs // itself, so the suite's half of Node's Suite.run() is spread over hooks of -// that scope. addSuite registers them around the describe callback so that they -// bracket the suite's own before()/after() hooks, which are bun:test hooks too -// and run in registration order. Only a suite with a stop of its own, or one -// inside such a suite, gets them; every other suite stays a plain describe. +// that scope. The suite's own before()/after() hooks are bun:test hooks of the +// same scope, and bun:test runs each kind in registration order, so addSuite +// registers one pair before the describe callback (they run ahead of the +// suite's hooks of the same kind) and one pair after it (they run behind +// them), reproducing Node's order: abort check, before hooks, stop armed, +// children, stop released, after hooks, verdict. Only a suite with a stop of +// its own, or one inside such a suite, gets them; every other suite stays a +// plain describe. // // Like every test and hook callback this module hands to bun:test, the hooks // take `done`: a nested describe callback runs inside its parent's async @@ -2734,35 +2742,40 @@ function bunTestOptions(options: TestOptions) { // throw fails the hook itself. That is what reports the suite's own error // against the suite (a describe block has no verdict of its own, and its // cancelled children only say that their parent stopped) and, from a -// beforeAll, what makes bun:test skip the scope's tests. A todo suite's failure -// is not a failure in Node: its children report themselves as todo under a -// run() child, and sit in a describe.todo scope under plain bun:test. -function registerTopLevelSuiteFinish(suite: TestNode) { - // Registered before the callback, so it runs before the suite's after hooks: - // once the children are done the stop can no longer fail the suite (Node - // ignores one that fires during the after hooks), and its timer is released - // whatever those hooks do. - bunTest().afterAll((done: (error?: unknown) => void) => { +// beforeAll, what makes bun:test skip the scope's remaining hooks and tests. A +// todo suite's failure is not a failure in Node: its children report +// themselves as todo under a run() child, and sit in a describe.todo scope +// under plain bun:test. +function registerTopLevelSuiteLeadingHooks(suite: TestNode) { + const { beforeAll, afterAll } = bunTest(); + // Node never starts a suite that is already stopped when its turn comes and + // drops its children; failing here makes bun:test skip the suite's before + // hooks and its tests the same way. A todo suite's before hooks are skipped + // by their own wrappers instead (suiteNeverStarted). + beforeAll((done: (error?: unknown) => void) => { + if (!suiteShouldAbort(suite)) suite.suiteStarted = true; + else if (!suite.todoFlag) throw suite.cancelError; + done(); + }); + // Runs before the suite's after hooks: once the children are done the stop + // can no longer fail the suite (Node ignores one that fires during the after + // hooks), and its timer is released whatever those hooks do. + afterAll((done: (error?: unknown) => void) => { suite.suiteStop?.dispose(); done(); }); } -function registerTopLevelSuiteStart(suite: TestNode) { +function registerTopLevelSuiteTrailingHooks(suite: TestNode) { const { beforeAll, afterAll } = bunTest(); - // Registered after the callback, so the suite's before hooks have already run - // by now; like in Node they do not count against the suite's timeout. + // Runs once the suite's before hooks have run; like in Node they do not count + // against the timeout, and a signal they aborted stops the suite right here. beforeAll((done: (error?: unknown) => void) => { - if (!suiteShouldAbort(suite)) { - suite.suiteStarted = true; - armSuiteStop(suite); - } else if (!suite.todoFlag) { - // Node drops the children of a suite that never starts. - throw suite.cancelError; - } + armSuiteStop(suite); done(); }); - // Runs after the suite's after hooks, where Node fails the suite as well. + // Runs after the suite's after hooks, where Node fails the suite as well. A + // suite that never started already failed the leading beforeAll. afterAll((done: (error?: unknown) => void) => { const stopped = suite.suiteStarted && suite.cancelError !== undefined; suite.finished = true; @@ -2771,6 +2784,15 @@ function registerTopLevelSuiteStart(suite: TestNode) { }); } +// Node runs neither hook set of a suite that was stopped before it started +// ([kShouldAbort] goes straight to postRun()), while a suite stopped later +// still runs its after hooks. The before()/after() wrappers of a top-level +// suite consult this: bun:test runs a scope's afterAll hooks even after a +// beforeAll failed, and a todo suite's leading beforeAll does not fail at all. +function suiteNeverStarted(suite: TestNode): boolean { + return suite.cancelError !== undefined && !suite.suiteStarted; +} + function currentCollectionParent(): TestNode { const node = currentNode(); if (node !== undefined && !node.isExecutionPhase && node.isSuite) { @@ -2980,7 +3002,7 @@ function addSuite( effectiveMode === "skip" ? kDefaultFunction : () => { - if (stoppable) registerTopLevelSuiteFinish(suiteNode); + if (stoppable) registerTopLevelSuiteLeadingHooks(suiteNode); const built = runWithNode(suiteNode, () => fn(suiteNode.getSuiteCtx())); if (!stoppable) return built; if (built != null && typeof (built as PromiseLike).then === "function") { @@ -2988,11 +3010,11 @@ function addSuite( // settles, so hooks registered from here still land in it, behind // any the callback registered after awaiting something. return (built as PromiseLike).then(value => { - registerTopLevelSuiteStart(suiteNode); + registerTopLevelSuiteTrailingHooks(suiteNode); return value; }); } - registerTopLevelSuiteStart(suiteNode); + registerTopLevelSuiteTrailingHooks(suiteNode); return built; }; @@ -3074,8 +3096,7 @@ function before(arg0: unknown, arg1: unknown) { } const { beforeAll } = bunTest(); beforeAll((done: (error?: unknown) => void) => { - // Node runs no hook of a suite that is cancelled by the time it would start. - if (suiteShouldAbort(owner)) { + if (suiteNeverStarted(owner)) { done(); return; } @@ -3095,9 +3116,7 @@ function after(arg0: unknown, arg1: unknown) { } const { afterAll } = bunTest(); afterAll((done: (error?: unknown) => void) => { - // Same as in before(): a suite that never started skips its after hooks - // too, while one that was stopped midway still runs them, like Node. - if (!owner.suiteStarted && suiteShouldAbort(owner)) { + if (suiteNeverStarted(owner)) { done(); return; } diff --git a/test/js/node/test_runner/fixtures/33-suite-signal.js b/test/js/node/test_runner/fixtures/33-suite-signal.js index 06cebbc255bd..6ff38b7fcfea 100644 --- a/test/js/node/test_runner/fixtures/33-suite-signal.js +++ b/test/js/node/test_runner/fixtures/33-suite-signal.js @@ -3,8 +3,10 @@ // an earlier test) fails with the reason and runs nothing: no hooks, no // children, although its describe callback does run. A signal that aborts // while the suite runs cancels the running child and the queued one, still -// runs the after hooks, and fails the suite with the reason; a falsy reason is -// reported as Node's own "The test was aborted". Four suites here fail on +// runs the after hooks, and fails the suite with the reason; one aborted by +// the suite's own before hook still runs the remaining before hooks and the +// after hooks, and cancels every child without starting it; a falsy reason is +// reported as Node's own "The test was aborted". Five suites here fail on // purpose; node-test.test.ts asserts the exact counts and the OBS markers. // Differences from `node --test` with this file: a suite's own failure shows up // as a failed hook of its describe block, the children of a suite that never @@ -55,6 +57,21 @@ describe("suite aborted while running", { signal: abortedWhileRunning.signal }, it("queued when the signal aborts", () => console.log("OBS queued child ran")); }); +const abortedByBeforeHook = new AbortController(); + +describe("suite aborted by its own before hook", { signal: abortedByBeforeHook.signal }, () => { + before(() => { + abortedByBeforeHook.abort(new Error("aborted by a before hook")); + console.log("OBS before hook aborted the suite"); + }); + before(() => console.log("OBS second before hook still ran")); + after(suite => + console.log("OBS after of the suite aborted by its hook ran, suite signal aborted: " + suite.signal.aborted), + ); + + it("child of the suite aborted by its hook", () => console.log("OBS child of the suite aborted by its hook ran")); +}); + describe("suite aborted with a falsy reason", { signal: AbortSignal.abort(0) }, () => { it("child of the suite aborted with a falsy reason", () => console.log("OBS falsy-reason suite child ran")); }); diff --git a/test/js/node/test_runner/fixtures/34-inline-suite-stop.js b/test/js/node/test_runner/fixtures/34-inline-suite-stop.js index 8cb905306ed2..c09273512e63 100644 --- a/test/js/node/test_runner/fixtures/34-inline-suite-stop.js +++ b/test/js/node/test_runner/fixtures/34-inline-suite-stop.js @@ -4,12 +4,14 @@ // queued behind it never run, and its after hooks still run (with the suite's // own signal left alone: a timeout fails a suite, it does not abort it); a // suite whose signal has already aborted is failed by the reason and runs -// nothing. Both fail the test that owns them, so the first test fails on -// purpose ("2 subtests failed"); the last test checks what the suites left -// behind, and passes verbatim under `node --test` too. node-test.test.ts also -// runs this file through run(), which reports each suite's own error. One -// difference from Node there: Node drops the children of a suite that was -// already aborted without reporting them, while here they report as cancelled. +// nothing; one whose own before hook aborts the signal still runs its other +// hooks but starts no child. All three fail the test that owns them, so the +// first test fails on purpose ("3 subtests failed"); the last test checks what +// the suites left behind, and passes verbatim under `node --test` too. +// node-test.test.ts also runs this file through run(), which reports each +// suite's own error. One difference from Node there: Node drops the children +// of an aborted suite without reporting them, while here they report as +// cancelled. const assert = require("node:assert"); const { test, describe, it, before, after } = require("node:test"); @@ -25,6 +27,7 @@ const seen = { queuedChildRan: false, nestedChildRan: false, abortedSuiteChildRan: false, + childOfSuiteAbortedByHookRan: false, childWithinTimeoutRan: false, childAfterSlowBeforeHookRan: false, }; @@ -57,6 +60,19 @@ test("inline suites are stopped by their timeout or an aborted signal", () => { }); }); + const abortedByBeforeHook = new AbortController(); + describe("aborted by its own before hook", { signal: abortedByBeforeHook.signal }, () => { + before(() => { + abortedByBeforeHook.abort(new Error("inline suite aborted by a before hook")); + seen.hooks.push("aborting before"); + }); + before(() => seen.hooks.push("second before after the abort")); + after(suite => seen.hooks.push("after the abort, suite signal aborted: " + suite.signal.aborted)); + it("child of the suite aborted by its hook", () => { + seen.childOfSuiteAbortedByHookRan = true; + }); + }); + describe("within its timeout", { timeout: 30_000 }, () => { it("passes", () => { seen.childWithinTimeoutRan = true; @@ -80,11 +96,18 @@ test("an inline suite's before hooks do not count against its timeout", () => { test("what the stopped inline suites left behind", () => { // Each test above waited for its suites before it finished. assert.deepStrictEqual(seen, { - hooks: ["before", "after, suite signal aborted: false"], + hooks: [ + "before", + "after, suite signal aborted: false", + "aborting before", + "second before after the abort", + "after the abort, suite signal aborted: true", + ], runningChildSignalAborted: true, queuedChildRan: false, nestedChildRan: false, abortedSuiteChildRan: false, + childOfSuiteAbortedByHookRan: false, childWithinTimeoutRan: true, childAfterSlowBeforeHookRan: true, }); diff --git a/test/js/node/test_runner/node-test.test.ts b/test/js/node/test_runner/node-test.test.ts index 46bd3ca1c2fc..698c4f849287 100644 --- a/test/js/node/test_runner/node-test.test.ts +++ b/test/js/node/test_runner/node-test.test.ts @@ -333,20 +333,24 @@ describe("node:test", () => { "OBS first child ran", "OBS running child settled, signal aborted: true", "OBS after ran, suite signal aborted: true", + "OBS before hook aborted the suite", + "OBS second before hook still ran", + "OBS after of the suite aborted by its hook ran, suite signal aborted: true", ]); // Each aborted suite fails with its signal's reason. expect(stderr).toContain("aborted before declaration"); expect(stderr).toContain("aborted by an earlier test"); expect(stderr).toContain("aborted while running"); + expect(stderr).toContain("aborted by a before hook"); expect(stderr).toContain("The test was aborted"); expect(stderr).toContain("test did not finish before its parent and was cancelled"); expect(stderr).not.toContain("too late to matter"); - // The deliberate failures: four suites plus the two children cancelled by - // the abort while running. + // The deliberate failures: five suites, the two children cancelled by the + // abort while running, and the child of the suite its before hook aborted. expect(stderr).toContain("4 pass"); expect({ exitCode, stderr }).toMatchObject({ exitCode: 1, - stderr: expect.stringContaining("6 fail"), + stderr: expect.stringContaining("8 fail"), }); }); @@ -383,10 +387,10 @@ describe("node:test", () => { "should enforce an inline suite's timeout and signal options", async () => { const { exitCode, stderr } = await runTests(["34-inline-suite-stop.js"]); - // The owning test fails for the two stopped suites, with the timeout as + // The owning test fails for the three stopped suites, with the timeout as // the reported cause; the before-hook test and the one inspecting what // the suites left behind pass. - expect(stderr).toContain("error: 2 subtests failed"); + expect(stderr).toContain("error: 3 subtests failed"); expect(stderr).toContain("test timed out after 20ms"); expect(stderr).toContain("2 pass"); expect({ exitCode, stderr }).toMatchObject({ @@ -407,11 +411,13 @@ describe("node:test", () => { .sort(([a], [b]) => (a < b ? -1 : a > b ? 1 : 0)); const cancelled = "test did not finish before its parent and was cancelled"; expect(outcomes).toEqual([ + ["aborted by its own before hook", "inline suite aborted by a before hook"], ["already aborted", "inline abort reason"], ["an inline suite's before hooks do not count against its timeout", "pass"], ["before hook outlasts the timeout", "pass"], ["child of the aborted suite", cancelled], - ["inline suites are stopped by their timeout or an aborted signal", "2 subtests failed"], + ["child of the suite aborted by its hook", cancelled], + ["inline suites are stopped by their timeout or an aborted signal", "3 subtests failed"], ["nested child", cancelled], ["nested suite queued when the suite times out", cancelled], ["passes", "pass"], From e33a93ba8b10035efb337a57311f81df4acbb892 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 16 Aug 2026 06:48:16 +0000 Subject: [PATCH 3/5] node:test: drop finished children from the sweep set, trim comments Children are tracked in a per-node unfinishedSubtests set (Node's name for it) and removed by finish(), so a finished node is collectable as before instead of staying reachable from the root for the rest of the file. --- src/js/node/test.ts | 206 +++++++++++++++----------------------------- 1 file changed, 68 insertions(+), 138 deletions(-) diff --git a/src/js/node/test.ts b/src/js/node/test.ts index 2a5c221e2d26..7dfa08405008 100644 --- a/src/js/node/test.ts +++ b/src/js/node/test.ts @@ -1390,8 +1390,7 @@ function makeCancelledByParentError() { return makeTestFailure("test did not finish before its parent and was cancelled", "cancelledByParent"); } -// Port of Node's Test#abortHandler (test.js:1059): the `signal` option's reason -// is the verdict, with Node's own error standing in for a falsy one. +// Node's abortHandler (test.js:1059) substitutes its own error for a falsy reason. function abortFailure(signal: AbortSignal): unknown { return signal.reason || $makeAbortError("The test was aborted"); } @@ -1554,29 +1553,23 @@ class TestNode { // Inline subtests are serialized through this chain. `concurrency` is // validated for Node-compat error codes but subtests always run serially. subtestChain: Promise = Promise.resolve(); - // Every child declared under this node, bun:test-driven and inline alike, so - // a suite that stops can sweep the ones it will not wait for. - subtests: TestNode[] = []; + // Node's unfinishedSubtests: every child, top-level and inline, until it finishes. + unfinishedSubtests = new Set(); failedSubtests = 0; firstSubtestError: unknown = undefined; // First failure from a before hook created while this test was running. hookFailure: unknown = undefined; - // Set by fail() and cancel(). Like Node's first-error-wins Test#fail, it is - // the verdict no matter how the body settles afterwards. + // The verdict fail()/cancel() set from outside the node's own run. cancelError: unknown = undefined; - // An enclosing suite has a `timeout` or `signal` option, so its stop may - // cancel this node while it is queued or running. + // An enclosing suite has a timeout or signal option and may cancel this node. inStoppableSuite: boolean; - // Suites only. Node's Suite.run() sets startTime once the suite has passed - // its [kShouldAbort] check; a suite cancelled before that point runs neither - // of its hook sets. suiteStop is the suite's armed stopTest(). + // Suites only: passed the abort check (Node's startTime), and the armed stop. suiteStarted = false; suiteStop: StopController | undefined = undefined; #ctx: TestContext | undefined; #suiteCtx: SuiteContext | undefined; #tags: string[] | undefined; - // `t.signal`, created on first use; #aborted remembers an abort() that - // happened before anything asked for it. + // t.signal, created on first use; #aborted records an abort() made before that. #abortController: AbortController | undefined; #aborted = false; #cancellation: PromiseWithResolvers | undefined; @@ -1605,7 +1598,7 @@ class TestNode { this.expectFailure = parseExpectFailure(options.expectFailure) || parent?.expectFailure || false; this.inStoppableSuite = parent !== undefined && (parent.inStoppableSuite || (parent.isSuite && hasStopOptions(parent.options))); - parent?.subtests.push(this); + parent?.unfinishedSubtests.add(this); } get tags(): string[] { @@ -1659,10 +1652,7 @@ class TestNode { this.#abortController?.abort(); } - // Port of Node's Test#fail for a verdict that arrives from outside the node's - // own run (a suite's stop, or the suite around it stopping): the first one - // wins, and a node that already finished keeps its verdict. Returns whether - // it took. + // Node's Test#fail: the first verdict wins, and a finished node keeps its own. fail(error: unknown): boolean { if (this.finished || this.cancelError !== undefined) return false; this.cancelError = error; @@ -1670,15 +1660,12 @@ class TestNode { return true; } - // Port of Node's Test#cancel (test.js:1064): fail() plus aborting t.signal. - // `error` is the `signal` option's reason of a suite aborting itself; a node - // swept up because the suite around it stopped gets cancelledByParent. + // Node's Test#cancel (test.js:1064): fail() plus aborting t.signal. cancel(error: unknown) { if (this.fail(error ?? makeCancelledByParentError())) this.abort(); } - // For executeTestNode to race the body against: rejects with the cancelError - // as soon as fail() reaches this node, or right away if it already has. + // Raced by executeTestNode; rejects once fail() reaches the node. cancellation(): Promise { let pending = this.#cancellation; if (pending === undefined) { @@ -1691,16 +1678,19 @@ class TestNode { return pending.promise; } - // Port of Node's postRun() sweep (test.js:1476): a suite that stopped cancels - // the children it will not wait for, and those cancel theirs. + // Node's postRun() sweep (test.js:1476). cancelSubtests() { - for (const subtest of this.subtests) { - if (subtest.finished) continue; + for (const subtest of this.unfinishedSubtests) { subtest.cancel(undefined); subtest.cancelSubtests(); } } + finish() { + this.finished = true; + this.parent?.unfinishedSubtests.delete(this); + } + // True while user code reached from this node should treat new tests as // inline subtests instead of bun:test registrations. isRunning(): boolean { @@ -2027,9 +2017,7 @@ function validateTimeoutAndSignal(options: TestOptions | HookOptions) { } } -// Whether Node's Suite.run() would arm a stop for these (validated) options. -// Suites have no default or inherited timeout (test.js:1763), so only an -// explicit finite one or a signal counts. +// Suites have no default or inherited timeout (test.js:1763). function hasStopOptions({ timeout, signal }: TestOptions): boolean { return (typeof timeout === "number" && Number.isFinite(timeout)) || signal !== undefined; } @@ -2104,11 +2092,8 @@ function applyExpectFailure(node: TestNode, failure: unknown): unknown { function validateTestOptions(options: TestOptions): { ownTags: string[] | undefined } { const { concurrency, tags, plan } = options; - // Suites enforce timeout and signal (see startInlineSuite and the top-level - // suite hooks registered by addSuite); tests enforce timeout in - // executeTestNode. A test's signal and concurrency anywhere are only - // validated for Node's error contract so far (t.signal aborts only when a - // suite cancels the test; subtests always run serially). + // A test's signal, and concurrency, are validated for Node's error contract + // but not yet enforced (subtests always run serially). validateTimeoutAndSignal(options); if (concurrency != null && typeof concurrency !== "boolean") { if (typeof concurrency === "number") { @@ -2226,11 +2211,8 @@ let addAbortListener; type StopController = { promise: Promise; dispose(): void }; -// Port of Node's stopTest()/stopPromise (test.js:145): armed once per test (and -// raced against both the body and plan.check()) or once per suite (stopping -// the suite's children). `promise` never resolves; it rejects with the timeout -// failure or, for a suite, with its `signal` option's reason. Callers must -// dispose(). +// Node's stopTest() (test.js:145): `promise` never resolves, it rejects on the +// timeout or (suites) the signal option. Callers must dispose(). function createStopController(timeout: number | undefined, signal?: AbortSignal): StopController | undefined { const hasTimeout = typeof timeout === "number" && Number.isFinite(timeout); if (!hasTimeout && signal === undefined) { @@ -2370,22 +2352,17 @@ async function executeTestNode(node: TestNode, fn: TestFn): Promise { const cancelledBeforeStart = node.cancelError; if (cancelledBeforeStart !== undefined) { - // An enclosing suite stopped while this test was still waiting its turn: - // Node's run() goes straight to postRun() ([kShouldAbort], test.js:1306), - // so no hook and no body runs, and fail() still puts the cancellation - // through expectFailure. + // Swept while queued: no hooks and no body ([kShouldAbort], test.js:1306). failure = applyExpectFailure(node, cancelledBeforeStart); node.passed = failure === undefined; node.error = failure ?? cancelledBeforeStart; - node.finished = true; + node.finish(); reportNodeToRunParent(node, started); return failure; } const ctx = node.getCtx(); const ancestors = ancestorChain(node); - // Rejected when an enclosing suite stops while this test runs; racing it is - // what ends the test's turn so the suite's remaining children can report. const cancellation = node.inStoppableSuite ? node.cancellation() : undefined; // Node applies the plan option before the beforeEach hooks run, and only for a @@ -2406,8 +2383,7 @@ async function executeTestNode(node: TestNode, fn: TestFn): Promise { failure = err; } - // A suite that stopped during the beforeEach hooks has already failed this - // test; there is nothing left for the body to decide, so it never starts. + // Swept during the beforeEach hooks: the body never starts. if (failure === undefined && node.cancelError === undefined) { // Node arms one stopPromise (timeout + signal) and races both the body // AND the plan wait against it. Arm timeout once here so plan({wait:true}) @@ -2473,9 +2449,7 @@ async function executeTestNode(node: TestNode, fn: TestFn): Promise { } } - // Cancelled while running (its turn ended through the race above, or the - // sweep reached it between two awaits): Node's fail() keeps the first error, - // so however the body or a hook settled, the cancellation is the verdict. + // A cancellation that arrived while running is the verdict (Node's fail() keeps the first error). failure = node.cancelError ?? failure; const bodyFailure = failure; @@ -2489,7 +2463,7 @@ async function executeTestNode(node: TestNode, fn: TestFn): Promise { node.error = failure ?? (acceptedXfail ? bodyFailure : null); // Mark finished before hooks so a late t.test() from an after/afterEach // hook hits addTest()'s parentAlreadyFinished path (Node cancels these). - node.finished = true; + node.finish(); for (let i = ancestors.length - 1; i >= 0; i--) { const ancestor = ancestors[i]; @@ -2525,15 +2499,13 @@ async function executeTestNode(node: TestNode, fn: TestFn): Promise { function scheduleSubtest(parent: TestNode, child: TestNode, fn: TestFn, ownTodo: boolean): Promise { const run = async () => { if (child.options.skip) { - child.finished = true; + child.finish(); child.passed = true; return; } let failure: unknown; try { - // A subtest swept while it was still queued is not what triggers the - // before hooks (its suite may never have started); executeTestNode just - // reports it. + // A swept child must not trigger the before hooks of a suite that never started. if (child.cancelError === undefined) await runOwnBeforeHooks(parent); failure = await executeTestNode(child, fn); } catch (err) { @@ -2569,15 +2541,10 @@ async function drainSubtestChain(node: TestNode) { } while (chain !== node.subtestChain); } -// Whether the suite is already stopped at one of the two points where Node's -// Suite.run() would notice: when its turn comes ([kShouldAbort], test.js:1245; -// it then runs nothing, hooks included) and again after its before hooks -// (test.js:1866; it then starts no child, but still runs its after hooks). -// Either it was swept by a suite around it that stopped, or its own `signal` -// option has aborted by now (Node's listener cancels it the moment the signal -// aborts; looking at these two points gives the same verdicts). Node drops the -// children of such a suite; they are cancelled here instead, so the ones the -// subtest chain or bun:test still reaches report without running. +// Node's [kShouldAbort] (test.js:1245), also consulted after the before hooks +// (test.js:1866): swept by an enclosing suite, or its own signal aborted by now. +// Node drops the children; cancelling them makes the ones bun:test or the chain +// still reaches report without running. function suiteShouldAbort(suite: TestNode): boolean { if (suite.cancelError === undefined) { const { signal } = suite.options; @@ -2588,12 +2555,10 @@ function suiteShouldAbort(suite: TestNode): boolean { return true; } -// Port of the stopTest() call in Node's Suite.run() (test.js:1865), made once -// the suite's before hooks have run. Node runs the after hooks first and sweeps -// the children in postRun() when the stop fires; the sweep comes first here -// because it is what ends the running child's turn, after which the queued -// children report as cancelled, the chain (or bun:test's scope) drains on its -// own, and the after hooks follow. +// stopTest() of Suite.run() (test.js:1865). Node sweeps the children in +// postRun(), after the after hooks; sweeping as soon as the stop fires is what +// ends the running child's turn here, so the scope or chain drains and the +// after hooks follow. function armSuiteStop(suite: TestNode) { if (suiteShouldAbort(suite)) return; const { timeout, signal } = suite.options; @@ -2602,28 +2567,24 @@ function armSuiteStop(suite: TestNode) { suite.suiteStop = stop; stop.promise.catch(error => { stop.dispose(); - // A timeout merely fails the suite (test.js:1876); only its `signal` option - // cancels it, aborting the suite's own signal as well (the listener from - // test.js:724 runs Test#cancel). The children are swept either way. + // A timeout only fails the suite (test.js:1876); an aborted signal cancels + // it, aborting the suite's own signal too (test.js:724). if (signal?.aborted) suite.cancel(error); else suite.fail(error); suite.cancelSubtests(); }); } -// First half of Node's Suite.run() for an inline suite: chained at the head of -// the suite's own subtestChain so the children queued behind it see its -// outcome. scheduleSuiteSubtest's run() is the second half. Must not reject: -// a rejected link would skip every child queued behind it. +// First half of Suite.run() for an inline suite, chained ahead of its children; +// scheduleSuiteSubtest's run() is the second half. Must not reject: the +// children are chained behind it. async function startInlineSuite(suite: TestNode) { if (suiteShouldAbort(suite)) return; suite.suiteStarted = true; try { await runOwnBeforeHooks(suite); } catch (err) { - // A failing suite-level before hook fails the suite, like Node. Its - // children fail through the same memoized rejection, so there is nothing - // left for a stop to cut short. + // Fails the suite like Node; the children fail through the memoized rejection. recordSuiteFailure(suite, err); return; } @@ -2645,13 +2606,10 @@ function scheduleSuiteSubtest(parent: TestNode, suite: TestNode, build: unknown, recordSuiteFailure(suite, err); } } - // Wait for startInlineSuite, the children created during the callback, and - // any they schedule. A stop ends the running child's turn and the rest - // report as cancelled, so a stopped suite drains too. + // Covers startInlineSuite too; a stopped suite drains as its children report. await drainSubtestChain(suite); suite.suiteStop?.dispose(); - // Like Node, a suite cancelled before it started skips its after hooks along - // with everything else, while one stopped midway still runs them. + // A suite that never started skips its after hooks too (Node). if (suite.suiteStarted) { for (const hook of suite.hooks.after) { try { @@ -2661,9 +2619,7 @@ function scheduleSuiteSubtest(parent: TestNode, suite: TestNode, build: unknown, } } } - suite.finished = true; - // Being stopped fails the suite even when no child failed (one aborted - // before it started ran none), and is what Node reports for it. + suite.finish(); const { cancelError } = suite; suite.passed = cancelError === undefined && suite.failedSubtests === 0; if (runChildReporterEnabled) { @@ -2723,43 +2679,24 @@ function bunTestOptions(options: TestOptions) { return undefined; } -// A top-level suite is a bun:test describe block whose children bun:test runs -// itself, so the suite's half of Node's Suite.run() is spread over hooks of -// that scope. The suite's own before()/after() hooks are bun:test hooks of the -// same scope, and bun:test runs each kind in registration order, so addSuite -// registers one pair before the describe callback (they run ahead of the -// suite's hooks of the same kind) and one pair after it (they run behind -// them), reproducing Node's order: abort check, before hooks, stop armed, -// children, stop released, after hooks, verdict. Only a suite with a stop of -// its own, or one inside such a suite, gets them; every other suite stays a -// plain describe. -// -// Like every test and hook callback this module hands to bun:test, the hooks -// take `done`: a nested describe callback runs inside its parent's async -// context (the shim's AsyncLocalStorage), and bun:test cannot read the arity -// of a callback registered under one, so it waits for done() either way. They -// fail by throwing, though: an error passed to done() is only printed, while a -// throw fails the hook itself. That is what reports the suite's own error -// against the suite (a describe block has no verdict of its own, and its -// cancelled children only say that their parent stopped) and, from a -// beforeAll, what makes bun:test skip the scope's remaining hooks and tests. A -// todo suite's failure is not a failure in Node: its children report -// themselves as todo under a run() child, and sit in a describe.todo scope -// under plain bun:test. +// bun:test runs the children of a top-level suite itself, so Suite.run() is +// spread over hooks of its describe block. Hooks of one kind run in +// registration order, so the pair registered before the describe callback runs +// ahead of the suite's own before()/after() hooks and the pair registered after +// it runs behind them. The hooks take `done` like every callback this module +// hands to bun:test (it cannot read the arity of a callback registered under +// an async context, which a nested describe callback is), and fail by throwing: +// an error passed to done() is only printed, while a throw fails the hook, +// which is how bun:test reports the suite's own error, and from a beforeAll +// also skips the block. A todo suite's failure is not a failure in Node. function registerTopLevelSuiteLeadingHooks(suite: TestNode) { const { beforeAll, afterAll } = bunTest(); - // Node never starts a suite that is already stopped when its turn comes and - // drops its children; failing here makes bun:test skip the suite's before - // hooks and its tests the same way. A todo suite's before hooks are skipped - // by their own wrappers instead (suiteNeverStarted). beforeAll((done: (error?: unknown) => void) => { if (!suiteShouldAbort(suite)) suite.suiteStarted = true; else if (!suite.todoFlag) throw suite.cancelError; done(); }); - // Runs before the suite's after hooks: once the children are done the stop - // can no longer fail the suite (Node ignores one that fires during the after - // hooks), and its timer is released whatever those hooks do. + // Before the suite's after hooks: the stop is over once the children are. afterAll((done: (error?: unknown) => void) => { suite.suiteStop?.dispose(); done(); @@ -2768,27 +2705,21 @@ function registerTopLevelSuiteLeadingHooks(suite: TestNode) { function registerTopLevelSuiteTrailingHooks(suite: TestNode) { const { beforeAll, afterAll } = bunTest(); - // Runs once the suite's before hooks have run; like in Node they do not count - // against the timeout, and a signal they aborted stops the suite right here. + // After the suite's before hooks, which do not count against the timeout. beforeAll((done: (error?: unknown) => void) => { armSuiteStop(suite); done(); }); - // Runs after the suite's after hooks, where Node fails the suite as well. A - // suite that never started already failed the leading beforeAll. afterAll((done: (error?: unknown) => void) => { const stopped = suite.suiteStarted && suite.cancelError !== undefined; - suite.finished = true; + suite.finish(); if (stopped && !suite.todoFlag) throw suite.cancelError; done(); }); } -// Node runs neither hook set of a suite that was stopped before it started -// ([kShouldAbort] goes straight to postRun()), while a suite stopped later -// still runs its after hooks. The before()/after() wrappers of a top-level -// suite consult this: bun:test runs a scope's afterAll hooks even after a -// beforeAll failed, and a todo suite's leading beforeAll does not fail at all. +// Checked by before()/after(): bun:test still runs a block's afterAll hooks +// after its beforeAll failed, and a todo suite's leading beforeAll never fails. function suiteNeverStarted(suite: TestNode): boolean { return suite.cancelError !== undefined && !suite.suiteStarted; } @@ -2958,10 +2889,10 @@ function addSuite( if (ownTodo) suite.todoFlag = true; // The suite's children must run after the parent's previously scheduled // subtests AND after the describe callback's own returned promise settles - // (Node's Suite.run awaits buildPromise before iterating subtests), and - // then behind the suite's own start (its abort check, before hooks, and - // stop). The callback has not returned yet so its promise does not exist; - // seed the chain through a gate the callback's settlement opens. + // (Node's Suite.run awaits buildPromise before iterating subtests), then + // behind startInlineSuite. The callback has not returned yet so its promise + // does not exist; seed the chain through a gate the callback's settlement + // opens. const gate = Promise.withResolvers(); suite.subtestChain = runningNode.subtestChain.then(() => gate.promise).then(() => startInlineSuite(suite)); // Build the suite eagerly (Node also runs describe callbacks immediately), @@ -3006,9 +2937,8 @@ function addSuite( const built = runWithNode(suiteNode, () => fn(suiteNode.getSuiteCtx())); if (!stoppable) return built; if (built != null && typeof (built as PromiseLike).then === "function") { - // bun:test keeps the scope open until the callback's promise - // settles, so hooks registered from here still land in it, behind - // any the callback registered after awaiting something. + // The scope stays open until the promise settles, so these land + // behind hooks the callback registered after an await. return (built as PromiseLike).then(value => { registerTopLevelSuiteTrailingHooks(suiteNode); return value; From d372da13ba388bbf6c15a81463328b9cb09b089c Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 16 Aug 2026 06:51:55 +0000 Subject: [PATCH 4/5] node:test: shorten the suite stop comments --- src/js/node/test.ts | 33 ++++++++++++--------------------- 1 file changed, 12 insertions(+), 21 deletions(-) diff --git a/src/js/node/test.ts b/src/js/node/test.ts index 7dfa08405008..9c7749075664 100644 --- a/src/js/node/test.ts +++ b/src/js/node/test.ts @@ -2541,10 +2541,8 @@ async function drainSubtestChain(node: TestNode) { } while (chain !== node.subtestChain); } -// Node's [kShouldAbort] (test.js:1245), also consulted after the before hooks -// (test.js:1866): swept by an enclosing suite, or its own signal aborted by now. -// Node drops the children; cancelling them makes the ones bun:test or the chain -// still reaches report without running. +// Node's [kShouldAbort] (test.js:1245 and, after the before hooks, 1866). Node +// drops the children; cancelled ones report without running when reached. function suiteShouldAbort(suite: TestNode): boolean { if (suite.cancelError === undefined) { const { signal } = suite.options; @@ -2555,10 +2553,8 @@ function suiteShouldAbort(suite: TestNode): boolean { return true; } -// stopTest() of Suite.run() (test.js:1865). Node sweeps the children in -// postRun(), after the after hooks; sweeping as soon as the stop fires is what -// ends the running child's turn here, so the scope or chain drains and the -// after hooks follow. +// stopTest() of Suite.run() (test.js:1865). Node sweeps the children after the +// after hooks; here the sweep is what lets the children drain before them. function armSuiteStop(suite: TestNode) { if (suiteShouldAbort(suite)) return; const { timeout, signal } = suite.options; @@ -2575,9 +2571,8 @@ function armSuiteStop(suite: TestNode) { }); } -// First half of Suite.run() for an inline suite, chained ahead of its children; -// scheduleSuiteSubtest's run() is the second half. Must not reject: the -// children are chained behind it. +// First half of Suite.run(); scheduleSuiteSubtest's run() is the second. Must +// not reject: the children are chained behind it. async function startInlineSuite(suite: TestNode) { if (suiteShouldAbort(suite)) return; suite.suiteStarted = true; @@ -2679,16 +2674,12 @@ function bunTestOptions(options: TestOptions) { return undefined; } -// bun:test runs the children of a top-level suite itself, so Suite.run() is -// spread over hooks of its describe block. Hooks of one kind run in -// registration order, so the pair registered before the describe callback runs -// ahead of the suite's own before()/after() hooks and the pair registered after -// it runs behind them. The hooks take `done` like every callback this module -// hands to bun:test (it cannot read the arity of a callback registered under -// an async context, which a nested describe callback is), and fail by throwing: -// an error passed to done() is only printed, while a throw fails the hook, -// which is how bun:test reports the suite's own error, and from a beforeAll -// also skips the block. A todo suite's failure is not a failure in Node. +// Suite.run() for a top-level suite, whose children bun:test runs itself: hooks +// of one kind run in registration order, so this pair (registered before the +// describe callback) runs ahead of the suite's own hooks and the trailing pair +// behind them. They take `done` like every callback handed to bun:test, and +// fail by throwing, which bun:test reports as the suite's own failure (from a +// beforeAll it also skips the block); a todo suite's failure is none in Node. function registerTopLevelSuiteLeadingHooks(suite: TestNode) { const { beforeAll, afterAll } = bunTest(); beforeAll((done: (error?: unknown) => void) => { From b9fb05b7dab7ad3d95a0f896168030dc19179703 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 16 Aug 2026 06:57:58 +0000 Subject: [PATCH 5/5] node:test: note that a stopped suite still waits for a hook in flight --- src/js/node/test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/js/node/test.ts b/src/js/node/test.ts index 9c7749075664..d833c28b22de 100644 --- a/src/js/node/test.ts +++ b/src/js/node/test.ts @@ -2601,7 +2601,9 @@ function scheduleSuiteSubtest(parent: TestNode, suite: TestNode, build: unknown, recordSuiteFailure(suite, err); } } - // Covers startInlineSuite too; a stopped suite drains as its children report. + // Covers startInlineSuite too. A stopped suite drains as its children + // report, once any hook they were in the middle of returns (Node moves on + // without waiting for such a hook). await drainSubtestChain(suite); suite.suiteStop?.dispose(); // A suite that never started skips its after hooks too (Node).