node:test: republish run() child events at the child's nesting

[robobun]

Problem

• Under run({ files }) from node:test, every republished test:pass / test:fail event carries a nesting one higher than node's: a file's top-level tests come out at nesting 1 and their subtests at nesting 2. Node v26.3.0 reports 0 and 1, exactly what the file reports when run on its own.
• Cause: republishChildEvent() in src/js/node/test.ts did data.nesting = (data.nesting ?? 0) + 1 on every event it forwarded from the child process. Node's FileTest#handleReportItem (lib/internal/test_runner/runner.js) forwards the child's nesting unchanged.
• The file node that run() reports for a file (its test:enqueue / test:dequeue, and its verdict when the file had nothing else to report, failed, or was cancelled) always carried testId: 1 and testNumber: 1, whatever its position in the run. Node gives the second file 2, and so on.

Fix

• republishChildEvent() forwards the child's nesting as-is.
• runFiles() passes each file's 1-based position to runOneFile() / reportCancelledFile(), which use it as the file node's testId and testNumber.
• Why this is correct: it matches node. Node's root creates one FileTest per file and numbers them in order; under process isolation it does not treat the file node as the parent of the file's tests, so the child's nesting values are the run's nesting values. Verified against node v26.3.0 on the test's files: the verdict sequence with its nesting column, and empty.js reporting as testNumber: 2, testId: 2, are node's output line for line (details below).
• Counting is unchanged: topLevel and the run-level test:plan still count one item per file. Node counts the file's nesting-0 verdicts instead, but bun's child does not yet report top-level describe() suites (node:test: report a verdict for each suite registered with bun:test under run() #39331 adds that), so counting nesting-0 verdicts here would report 0 top-level items for a describe-only file; that recount belongs with node:test: report a verdict for each suite registered with bun:test under run() #39331. Subtest testNumber (node forwards the child's value; bun's child sends 0) is also unchanged.
• Verified: test/js/node/test_runner/node-test.test.ts, new node:test run() block (two cases: a two-file run pinning the nesting column and the file node's number, and a run aborted before it starts, pinning both cancelled file nodes' numbers). Both fail on main (nesting 2/1/1/1, file node number 1) and pass with the fix. The other 45 cases in the file and the vendored run() tests (test-runner-todo-skip-tests.js, test-runner-expect-error.js, test-runner-expect-error-but-pass.js, test-runner-filetest-location.js, test-runner-tags-experimental-warning.mjs) still pass.
• node:test: --test CLI mode, node:test/reporters, standalone execution, and reporter-output parity (+18 tests, test_runner 32%→55%) #34515 rewrites this code path as part of a much larger change and also drops the +1; this is the standalone fix for the bug on main today.

Background

• run() is node:test's programmatic runner. With the default isolation: 'process' it runs each file in a child process; the child reports its tests as events and the parent republishes them on the TestsStream that run() returned, then appends the run-level test:plan, diagnostics and test:summary. Bun's run() does the same with a bun test child that prints one JSON event per line.
• nesting is a test's depth in the event stream: a file's top-level tests are 0, t.test() subtests inside them are 1, and so on. Reporters indent by it. Under process isolation node keeps the child's values; the file node that the parent synthesizes for each file also sits at nesting 0.
• The file node is node's FileTest: a test-shaped entry per file, enqueued and dequeued as the file starts, and given a verdict of its own only when the file had nothing else to report or the process itself failed (FileTest#skipReporting); a file whose tests merely failed is represented by those tests. Its testId and testNumber are its position among the run's files.

node v26.3.0 vs bun for the files used in the test

nested.js (top with subtest sub, a failing test, a skipped test) and empty.js (no tests), run in that order. Columns: event, name, nesting.

node v26.3.0                    bun main                        bun with this change
test:pass  sub        1         test:pass  sub        2         test:pass  sub        1
test:pass  top        0         test:pass  top        1         test:pass  top        0
test:fail  failing    0         test:fail  failing    1         test:fail  failing    0
test:pass  skipped    0         test:pass  skipped    1         test:pass  skipped    0
test:pass  empty.js   0         test:pass  empty.js   0         test:pass  empty.js   0
  (testNumber 2, testId 2)        (testNumber 1, testId 1)        (testNumber 2, testId 2)

Earlier revision of this PR

The first push also moved topLevel / testNumber onto node's model on the parent side: each nesting-0 verdict counted as a top-level item and was numbered across files, and the file node only counted when reported. That is right once the child reports every top-level item, but today a top-level describe() emits no verdict (#39331 adds it), so a describe-only file would have reported test:plan count 0 and topLevel: 0 where main and node both report 1. That part was dropped from this PR; the counting on main is unchanged.

[robobun]

^{Updated 2:00 AM PT - Aug 16th, 2026}

✅ @robobun, your commit cfbdcc79567c464150ebada0b3705fa24c2668f6 passed in Build #99350! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 39284

That installs a local version of the PR into your bun-39284 executable, so you can run:

bun-39284 --bun

[robobun]

Reproduced on main with a two-file repro (a file whose top-level test has one subtest, driven by run({ files })): bun reported sub 2 / top 1, node v26.3.0 reports sub 1 / top 0. With this branch, bun bd test test/js/node/test_runner/node-test.test.ts passes; the two new cases fail on main (nesting 2/1/1/1, file nodes always numbered 1).

Scope note: the first revision also re-counted topLevel / testNumber from the child's nesting-0 verdicts. That was dropped after a self-review: top-level describe() suites do not report a verdict yet (#39331 adds them), so a describe-only file would have counted as 0 top-level items. This PR now only fixes nesting and the file node's testId / testNumber; counting is unchanged from main.

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 19 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: 5317ade5-0bb4-4bbe-85ea-b2de3a35b6d7

📥 Commits

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

📒 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.}

[claude]

LGTM — focused node:test run() compat fix that stops adding +1 to child event nesting and threads correct testNumber/topLevel values through, matching Node v26.3.0.

What was reviewed:

• Traced the new testNumber = runCounts.topLevel + counts.topLevel arithmetic through the three-file fixture (nested/empty/second) — matches the expected 1/2/3, 2, 5 sequence and the run-level plan count of 5.
• Confirmed addRunCounts folds fileCounts.topLevel into the run accumulator between files, so runCounts.topLevel is the completed-files total when read in republishChildEvent.
• Checked that removing the unconditional fileCounts.topLevel++ and moving it under reportFileNode doesn't break the per-file test:summary (it's still counted before emission).
• Verified the dropped ?? 0 fallback on data.nesting is safe: the child's nestingOf() always emits a number.

Extended reasoning...

Overview

This PR fixes node:test's run({ files }) to republish child-process events with the child's own nesting value instead of nesting + 1, matching Node's FileTest#handleReportItem. It also threads a per-file testNumber (the file's 1-based position in the run) into runOneFile/reportCancelledFile, counts each nesting-0 verdict toward topLevel inside republishChildEvent, and only counts the synthetic file node in topLevel when it is actually reported (#skipReporting() semantics). A new ~100-line test in node-test.test.ts drives three files through run() and asserts nesting, testNumber, the run-level test:plan count, and per-file/run test:summary counts against Node v26.3.0's output.

Security risks

None. This is purely event-metadata bookkeeping in the node:test compat layer — no auth, crypto, filesystem, or network surface changes. The child process spawn path is unchanged.

Level of scrutiny

Medium-low. The change is confined to three functions in src/js/node/test.ts that already existed; the diff is mechanical (remove a +1, thread an index parameter, move one increment inside a conditional, add a nesting-0 branch that assigns testNumber). Each behavioral claim is cited against a specific Node source location (FileTest#handleReportItem, countCompletedTest, #skipReporting, report()) and verified against a comparison table of Node v26.3.0 output. Files run sequentially, so reading runCounts.topLevel mid-file while addRunCounts runs at file end has no ordering hazard.

Other factors

The test is comprehensive for a compat fix: it exercises a subtest (nesting 1), top-level pass/fail/skip, an empty file (file node reported), and a follow-on file (cross-file numbering), and asserts exact values via toEqual rather than shape checks. The PR states the rest of the file's 46 tests and the vendored run() tests still pass. The author also confirmed the new test fails on main (2/1/1/1 vs 1/0/0/0) and passes with the fix. The one acknowledged remaining gap — the subtest's own testNumber — is a separate child-side numbering issue that Node also forwards from the child, so it is correctly out of scope here.

[claude]

Additional findings (outside current diff — PR may have been updated during review):

• 🟡 src/js/node/test.ts:634-639 — The PR description's Fix bullets 2–3 and the "bun with this change" comparison column claim that nesting-0 verdicts get testNumber = run-position and are counted in topLevel, and that the file node only adds to topLevel when reported — but neither is in the diff: republishChildEvent() never assigns data.testNumber or touches counts.topLevel, and fileCounts.topLevel++ at line 566 is still unconditional. Republished top-level tests keep testNumber: 0 and run-level topLevel/test:plan count is still the number of files, not the table's 1/2/3/5 and 5. Either add the missing logic (and assert it — the new test collects testNumber for every verdict but only checks it on the file node), or trim the description/table to what actually shipped: nesting pass-through + file-node ordinal.

  Extended reasoning...

  What the description claims vs. what the diff does

  The PR description's Fix section states three things:

  1. republishChildEvent() forwards the child's nesting as-is — implemented (the + 1 on line 630 is removed).
  2. "Every nesting-0 verdict is counted in topLevel and given testNumber = its position among the run's top-level tests" — not implemented.
  3. "The file node only adds to topLevel when it is actually reported" — not implemented.

  And the comparison table's "bun with this change" column shows top → testNumber 1, failing → 2, skipped → 3, second-top → 5, nested.js topLevel 3, plan count 5, run topLevel 5.

  Code trace

  republishChildEvent() (src/js/node/test.ts:628-669) after this diff:

  • Sets data.file, builds data.details, deletes error/duration_ms/type.
  • Increments counts.suites or counts.tests + one of skipped/todo/passed/failed.
  • Never assigns data.testNumber. The child emits testNumber: 0 (lines 717, 740, 2475), and that value passes through unchanged.
  • Never touches counts.topLevel.

  fileCounts.topLevel++ (src/js/node/test.ts:566) is unconditional — it runs before reportFileNode is even computed on line 567. So every file contributes exactly 1 to topLevel regardless of whether its file node is reported.

  Concrete example (the description's own three-file scenario)

  Files: nested.js (top/sub/failing/skipped), empty.js, second.js (second-top). What the code actually produces:

  event	name	nesting	testNumber	table claims
  test:pass	sub	1	0	0 ✓
  test:pass	top	0	0	1 ✗
  test:fail	failing	0	0	2 ✗
  test:pass	skipped	0	0	3 ✗
  test:pass	empty.js	0	2	2 ✓ (this is the ordinal fix)
  test:pass	second-top	0	0	5 ✗

  Per-file nested.js topLevel: 1 (unconditional ++ at line 566), table claims 3. Run-level test:plan count / counts.topLevel: 3 (one per file via addRunCounts), table claims 5.

  Why the new test doesn't catch this

  The test at node-test.test.ts:395-403 collects { type, name, nesting, testNumber, testId } for every verdict, but the first assertion maps to only [type, name, nesting] — dropping testNumber — and the second assertion checks testNumber/testId only on verdicts.at(-1) (the empty.js file node, which is fixed via ordinal). Neither test:plan count nor summary.counts.topLevel is asserted. So the test exercises exactly the two things the diff does change and none of the unimplemented claims. Per REVIEW.md "assert the strongest invariant", the test should either assert testNumber on the republished verdicts or not collect it.

  Impact and severity

  The code that is in the diff is correct and nothing regresses: nesting is now forwarded unchanged (the headline fix), and file nodes get their run-position ordinal instead of a hardcoded 1. The unimplemented testNumber/topLevel behavior is unchanged from main — it's not a regression, just scope the description claims but the diff doesn't deliver. Per the PR-description-mismatch default this is a nit: merging as-is ships a correct partial fix with an overstated description (CLAUDE.md rule 11: "NEVER overstate what you got done"), not incorrect behavior.

  How to fix

  Either path resolves it:

  (a) Implement what the description says. In republishChildEvent(), for non-suite verdicts with data.nesting === 0, do data.testNumber = ++counts.topLevel (with counts being the run-level accumulator so numbering spans files, matching node's countCompletedTest), and move line 566's fileCounts.topLevel++ inside if (reportFileNode). Then extend the test to assert testNumber on top/failing/skipped and assert test:plan count / summary.counts.topLevel.

  (b) Trim the description. Drop Fix bullets 2–3 and correct the "bun with this change" column to show testNumber 0/0/0/0 for the republished verdicts and topLevel/plan = number of files. The diff then honestly describes nesting pass-through + file-node ordinal, which is what shipped and what the test covers.

  On the refutation

  One verifier refuted bug_002 as a duplicate of bug_001 — correct, and both were merged into this single finding. The refutation does not dispute the substance.

[robobun]

The description that review read was the first revision's; the diff had just been reduced (8d514e3) and the body was updated shortly after. The current description matches the diff: the child's nesting is forwarded unchanged and the file node carries its position as testId / testNumber; topLevel, the test:plan count and the republished tests' testNumber are unchanged from main, and the body says so (the dropped recount is described under "Earlier revision"). The test asserts the nesting of every verdict and the full record of the file node, which is what the diff changes.

[claude]

LGTM — focused node:test compat fix with tests pinned to node v26.3.0's output.

What was reviewed:

• republishChildEvent() no longer bumps nesting; confirmed the child side always sets nesting: nestingOf(node) so the removed ?? 0 fallback was dead.
• ordinal threading through runOneFile/reportCancelledFile — all six former testId: 1/testNumber: 1 sites updated consistently.
• Checked no other consumer in test.ts reads the republished nesting value (counting still keys off topLevel, unchanged).
• Tests: both new cases drain pipes concurrently, use tempDir/bunEnv, assert exact event sequences; the abort case exercises reportCancelledFile without spawning children.

Extended reasoning...

Overview

Two-file change to node:test's run() implementation. In src/js/node/test.ts, the parent-side republishChildEvent() stops adding +1 to the child's reported nesting (one line removed), and runFiles() now threads each file's 1-based position as ordinal into runOneFile() / reportCancelledFile(), which use it for the file node's testId and testNumber in place of six hardcoded 1s. A one-line comment on republishChildEvent() records why nesting is passed through unchanged. Two new subprocess tests in test/js/node/test_runner/node-test.test.ts pin the exact verdict/nesting sequence for a two-file run and the ordinals for an aborted-before-start run.

Security risks

None. This is event-metadata reshaping in the node:test compat layer — no parsing of untrusted input beyond what already existed, no auth/crypto/fs/network surface touched. The child JSON events are produced by bun's own spawned bun test process.

Level of scrutiny

Low-to-moderate. The change is mechanical (delete a +1, thread a loop index through as a parameter) and sits in the Node.js compat layer where the reference implementation is the spec. The PR description diffs the output against node v26.3.0 line-for-line and explains why the earlier, broader revision was scaled back (top-level describe() verdicts aren't emitted yet). I verified the removed ?? 0 fallback was not load-bearing: every child-side emit path (emitRunChildEvent callers at lines 712/735/2470) sets nesting: nestingOf(node), so data.nesting is always present on republished events.

Other factors

The comment-cop bot flagged paragraph-long comments on an earlier revision; the author trimmed them in cfbdcc7 and both threads are resolved. Tests follow repo conventions (test.concurrent, tempDir, bunEnv, concurrent pipe draining, exact-value assertions, 30s headroom for the debug+ASAN child matching neighbouring tests). The PR notes the existing 45 cases in the file plus five vendored run() tests still pass. No CODEOWNERS entry covers src/js/node/. Counting logic (topLevel, test:plan) is deliberately left unchanged with a clear rationale tied to #39331.