node:test: report a verdict for each suite registered with bun:test under run()

[robobun]

Problem

• Under node:test's run(), a describe() / suite() block declared at the top level of a file (or nested in another one) never gets a test:pass / test:fail event of its own. Node v26.3.0 reports one after the suite's children. Only suites declared inside a running test (inline suites) were reported. Same on 1.4.0 and main; never worked.
• Consequences for run() consumers: the suites N diagnostic and counts.suites undercount (the file in the test below reports suites 3 on main, node says suites 7), and a failure of the suite itself is invisible: a failing suite-level after() hook, or a describe() callback that throws, only surfaced as a synthesized test:fail for the whole file carrying the child's stderr, where node reports the suite as failed (failed running after hook, hookFailed) and nothing for the file.
• Cause, in src/js/node/test.ts: addSuite() registers such a suite as a bun:test describe block and nothing reports it when the block is done; reportNodeToRunParent() only handles tests, and the suite event was emitted from scheduleSuiteSubtest() alone, the inline path. On the parent side, runOneFile() derived the per-file success and the "did the file itself die" check from counts.failed, which counts tests only, so a failed suite could not make a file unsuccessful and would not have stopped the file-level verdict either.

Fix

• Child side, only when the file runs as a run() child (NODE_TEST_CONTEXT set): the describe callback wrapper (buildReportedSuite()) registers a bun:test beforeAll / afterAll pair in the block before running the callback. bun:test runs a block's beforeAll hooks before and its afterAll hooks after everything declared in the block, nested blocks included, so the beforeAll starts the suite's clock and the afterAll (settleReportedSuite()) runs the suite's after() hooks, stopping at the first failure like node's runHook(), and reports the verdict. Nested suites therefore report before their parent, and children before their suite, which is node's order.
• The verdict is node's Suite.run() + Test.postRun(), in concludeSuite(): a failure recorded against the suite itself wins (a before() / after() hook failure, recorded as failed running <kind> hook / hookFailed with the hook's error as cause, or a throwing / rejecting describe callback), otherwise children that finished without passing and are not todo fail it with N subtests failed / subtestsFailed. rollUpIntoEnclosingSuite() is the roll-up for tests and suites registered with bun:test; inline subtests keep theirs. Under run(), after() hooks of such a suite are collected on the node for the closing hook to run (their failure still reaches bun:test through the hook's done(err), exactly what after() handed it before, so the child's exit status is unchanged); before() hooks keep registering with bun:test and additionally record their failure on the suite.
• A todo suite's own failures are advisory in node (countCompletedTest leaves success alone, the process exits 0): they are recorded so the suite reports test:fail with the todo directive, and not handed to bun:test. The same path now reports todo suites in general, which were previously reported as passed at declaration time, before their children; skipped suites are unchanged (reportSkippedNode(), formerly reportDirectiveOnlyNode(), is now only ever called for skips).
• A describe callback that throws or rejects is reported at declaration time and rethrown: bun:test drops the whole block in that case, hooks included, so this is the one verdict that cannot be emitted in execution order. The inline suite path (scheduleSuiteSubtest()) now goes through the same concludeSuite() / reportSuiteToRunParent(), so both kinds of suite produce the same event shape; its only visible changes are a real duration_ms instead of 0 and the first failure attached as the cause of subtestsFailed, which executeTestNode() already did for tests (makeSubtestsFailedError() is the shared helper).
• Parent side: runFiles() / runOneFile() carry node's harness.success next to the counters (RunTally). republishChildEvent() clears it for a failed or cancelled test and for a failed non-todo suite, as node's countCompletedTest() does; the per-file and run-level test:summary report it, and the file-level verdict is now synthesized only for a nonzero exit that no reported failure accounts for (previously: no failed test), so a suite-only failure yields success: false, fail 0, and no file-level event, as in node.
• Why this is correct: it matches node. For every fixture in the new tests, node v26.3.0 and this branch agree on the sequence of verdicts (names, details.type, directives, error messages and failure types), on the suites diagnostic and on every test:summary field the change touches; the comparison is in the details block. The known remaining differences are listed there too; none of them changes a verdict or a count.
• Both new hooks declare and call done, like every other callback this module hands to bun:test: bun:test derives "waits for done()" from the callback's arity, and cannot read it through the async-context wrapper a callback registered under an active AsyncLocalStorage context gets, which is the case for every nested describe callback (it runs inside its parent's runWithNode()). A zero-arity hook there waits 5s for a done() nobody calls and, for a beforeAll, skips the block's tests. jsc: get_length returns 0 for objects without a length property; bun:test reads callback arity before wrapping it #38910 fixes that in bun:test; the code here is right either way.
• Plain bun test on a node:test file is untouched: every new code path is behind runChildReporterEnabled, and rollUpIntoEnclosingSuite() returns immediately outside a run() child.
• Verified: test/js/node/test_runner/node-test.test.ts, new node:test run() block (5 cases: the suite sequence above, suite-only after-hook failure, todo suites' own failures, before-hook failure, throwing and rejecting describe callbacks). All 5 fail on the unfixed build (no suite verdicts, suites 3 instead of 7, file-level verdicts instead of suite ones) and pass with it; the 45 existing cases in the file still pass. The ported test/js/node/test/parallel/test-runner-* files behave as on main (test-runner-todo-skip-tests.js, which pins a skipped suite's single event, included; test-runner-mock-timers-scheduler.js fails its 100ms wall-clock bound on the unmodified debug build too). test/regression/issue/{19111,19412,24147,24338,24339,24374,26915,28431}.test.ts pass.

Background

• run() is node:test's programmatic runner. With the default process isolation it runs each file in a child process; the child reports its tests as events, and the parent republishes them on the stream run() returned and appends the run-level plan, diagnostics and test:summary. Bun's run() spawns bun test on each file with NODE_TEST_CONTEXT set; in that mode node:test prints one JSON event per verdict, which the parent rebuilds.
• In node every suite is itself reported: after its children, as a test:pass or test:fail whose details.type is "suite". countCompletedTest() (node's test_runner/utils.js) counts such an event only in suites, never in tests / passed / failed, and clears the harness's success flag when the suite failed and is not todo. That flag is what test:summary reports as success; it is the only place a suite failure shows up in the totals.
• Node's FileTest synthesizes a verdict for the file itself only when the child exited nonzero without any of its own top-level verdicts explaining it (a file that failed to load, a root-level hook failure); a file whose tests or suites failed is represented by those verdicts.
• Bun's node:test registers top-level tests and suites with bun:test (Bun.jest), which then drives execution: tests through a runner callback, suites as describe blocks. Suites declared while a test is running are executed by the module itself ("inline" suites). bun:test orders a block's hooks around its contents, so a beforeAll / afterAll pair registered from the describe callback observes the start and the end of everything in the block.

node v26.3.0 vs. this branch on the test fixtures, and the remaining differences

Verdicts as name (details.type, directive), in stream order, for suites.js from the first test. The left column is node v26.3.0 and is also, line for line, what this branch emits:

node v26.3.0 = this branch                     bun main
skipped suite (suite, skip)                    skipped suite (suite, skip)
ok                                             todo suite (suite, todo)
bad FAIL                                       ok
inner (suite) FAIL "1 subtest failed"          bad FAIL
ok2                                            ok2
outer (suite) FAIL "1 subtest failed"          fails inside a todo suite FAIL (todo)
empty suite (suite)                            todo child FAIL (todo)
fails inside a todo suite FAIL (todo)          skipped child (skip)
todo suite (suite, todo)                       inline child
todo child FAIL (todo)                         inline suite (suite)
skipped child (skip)                           parent test
suite with directives (suite)
inline child
inline suite (suite)
parent test
suites 7                                       suites 3

after-hook.js (one suite whose after() throws, one passing child):

node: ok; suite (suite) FAIL "failed running after hook" hookFailed, cause "after boom"; summary success false, fail 0, suites 1
main: ok; after-hook.js FAIL (file-level, error = the child's stderr); summary success false, fail 1, suites 0
this: same as node

todo-suites.js (todo suites whose before(), after(), and callback fail): node and this branch both report the three suites as test:fail with the todo directive and the hook / callback error, success: true, todo 3, suites 3, no file-level verdict. Main reported the suites as passed at declaration time plus a file-level failure.

Remaining differences, all pre-existing or noted in the code:

• Children of a suite whose before() hook or callback failed: node cancels them (cancelledByParent); here bun:test decides what happens to them. After a failed before() they currently run and pass (bun:test only prints an error passed to a hook's done(); once it fails the hook instead, bun test: fail the test or hook whose done() received an error #39112, it will skip them), so the before-hook test only pins the suite's verdict and the summary fields that hold either way. After a throwing callback bun:test drops them (no events), except in a todo suite, where the error is not rethrown and they run; there the counts agree with node, which counts the cancelled children as todo as well.
• A suite whose callback threw is reported when it is declared (bun:test drops the block), so it appears earlier in the stream than in node. Skipped suites were already reported at declaration time; unchanged.
• Shape: the child does not label test code failures testCodeFailure (pre-existing for tests, same for describe callbacks here), and subtestsFailed errors carry the first failure as cause where node repeats the message. nesting / testNumber / topLevel are node:test: republish run() child events at the child's nesting #39284's.
• Inline suites still fold their own hook or callback failure into N subtests failed (pre-existing); the shared concludeSuite() would take a recorded error there too.

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 15 minutes

Limit details: You’ve used all 5 included reviews currently available under your plan.

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: cd25b0b4-52f3-4227-bd69-cb8d3db62990

📥 Commits

Reviewing files that changed from the base of the PR and between aec33f5 and ad3bc28.

📒 Files selected for processing (2)

• src/js/node/test.ts
• test/js/node/test_runner/node-test.test.ts

---

_{Comment @coderabbitai help to get the list of available commands.}

[robobun]

^{Updated 12:42 AM PT - Aug 16th, 2026}

@robobun, your commit ad3bc28 is building: #99305

[robobun]

Status

Reproduced on bun 1.4.0 and main with the two-file repro from the report (a top-level describe() with one child, plus a test declaring an inline suite), driven through run(): bun emitted 4 verdicts and suites 1; node v26.3.0 emits 5 (the top-level suite included) and suites 2. With this branch bun emits node's 5 and suites 2.

Fix and tests are in this PR; the five new cases in test/js/node/test_runner/node-test.test.ts (node:test run() block) fail on the unfixed build and pass with it. Waiting for CI.

[claude]

I reviewed this PR and didn't find any bugs. It's a substantial addition to the node:test compat layer (~200 new lines in src/js/node/test.ts implementing suite-verdict reporting via injected beforeAll/afterAll bracketing, plus a parent-side RunTally refactor), so a human look would still be worthwhile.

What was reviewed:

• Checked that every new code path (buildReportedSuite, rollUpIntoEnclosingSuite, the after() diversion, before() failure recording) is gated on runChildReporterEnabled / isReportedSuite, so plain bun test on a node:test file is unaffected.
• Verified concludeSuite reads suite.error === null against TestNode.error's null initializer, and that failSuiteBuild's rethrow preserves bun:test's scope-drop behavior for non-todo suites.
• Confirmed the RunTally refactor preserves the previous counts.cancelled contribution to run failure via reportCancelledFile setting run.success = false, and that republishChildEvent matches node's countCompletedTest (failed non-todo suite → success = false, no test-counter bump).

Extended reasoning...

Overview

This PR makes node:test's run() report a test:pass/test:fail verdict for each describe()/suite() block registered with bun:test, matching Node v26.3.0. Previously only inline suites (declared inside a running test) were reported; top-level and nested-in-suite blocks were silent, so counts.suites undercounted and suite-level hook failures surfaced only as a synthesized file-level failure.

The child-side fix (buildReportedSuite) wraps each describe callback so that, in a run() child, it registers a beforeAll (starts the clock) and an afterAll (settleReportedSuite: runs the suite's collected after() hooks, records the first failure as hookFailed, then concludeSuite → reportSuiteToRunParent). rollUpIntoEnclosingSuite propagates child failures into the parent suite's failedSubtests. before() failures are additionally recorded via failSuiteFromHook; after() hooks in reported suites are collected on the node instead of registered directly with bun:test. Todo suites' own failures are recorded but not handed to bun:test (advisory in Node). A throwing/rejecting describe callback (failSuiteBuild) reports at declaration time and rethrows so bun:test still drops the scope.

The parent side replaces the counts-only accounting with RunTally = { counts, success }, so a failed non-todo suite (which never bumps failed) can still make test:summary.success false and suppress the file-level synthesized verdict. republishChildEvent, runOneFile, runFiles, and reportCancelledFile are updated to thread success through.

Five new test.concurrent cases in node-test.test.ts pin exact verdict sequences, error messages/failureTypes, the suites N diagnostic, and per-file/run-level summary fields against Node v26.3.0.

Security risks

None. This is test-runner event reporting; no auth, crypto, filesystem-path handling, or untrusted-input parsing beyond what already existed. The child→parent JSON channel and rebuildError are unchanged.

Level of scrutiny

Medium-high. The change is entirely inside the Node.js compat layer for node:test's programmatic run(), but it's ~200 lines of new control flow with several subtle interactions: the done-arity workaround for async-context-wrapped hooks, todo-suite advisory-failure semantics, the failSuiteBuild rethrow (non-todo) vs. record-only (todo) split, diverting after() to run inside the closing afterAll, and using beforeAll/afterAll ordering as the bracket. The PR description is exceptionally thorough and cites Node source for each decision, and the tests pin exact verdict sequences, but the design (hook injection to observe bun:test's block boundaries) is the kind of approach a maintainer should sign off on.

Other factors

• All new paths are gated on runChildReporterEnabled (set only when NODE_TEST_CONTEXT is in the env), so bun test <node-test-file> without run() is untouched — I traced each new call site and confirmed the guards.
• reportDirectiveOnlyNode → reportSkippedNode narrowing is correct: the todo path now goes through buildReportedSuite's closing hook (or stays with describe.todo outside a run() child), and every remaining call site passes a skip.
• The inline-suite path (scheduleSuiteSubtest) now shares concludeSuite/reportSuiteToRunParent with the new path; the only observable changes there are a real duration_ms and a cause on subtestsFailed, both improvements toward Node parity.
• Tests follow harness conventions (tempDir, await using, concurrent pipe drain, test.concurrent, exact assertions) and each pins a Node-verified expectation with documented remaining differences.
• No prior human or bot review on this PR; CI build was still in progress at review time.

[robobun]

Note from #39284 (the nesting fix this PR's description defers to): that PR has been reduced to forwarding the child's nesting unchanged plus numbering the file node by its position (testId / testNumber). It deliberately leaves topLevel and the top-level testNumber as they are on main (one top-level item per file, child verdicts numbered 0), because node's model for those depends on the suite verdicts this PR adds: node's FileTest#handleReportItem counts every nesting-0 verdict as a top-level item (countCompletedTest), numbers it counters.topLevel + 1 across files, and counts the file node itself only when #skipReporting() is false. Without suite verdicts, a describe-only file would count as 0 top-level items, so that recount only becomes correct together with (or after) this PR. On the parent side it is about ten lines in republishChildEvent() / runOneFile():

// republishChildEvent, for test:pass / test:fail
if (data.nesting === 0) {
  fileCounts.topLevel++;
  data.testNumber = runCounts.topLevel + fileCounts.topLevel; // runCounts: files already finished
}
// runOneFile: move the unconditional fileCounts.topLevel++ under reportFileNode

Reference values from node v26.3.0 for a file with one describe() holding two it()s followed by a file with one test(): the suite is nesting 0 with testNumber: 1, the second file's test gets testNumber: 2, the run-level test:plan count and counts.topLevel are 2; a file with no tests at all is counted as one top-level item itself. With both PRs applied as they stand, the counting stays as on main (one item per file), which is wrong in general but at least not a regression; the recount would be the follow-up here.