node:test: --test CLI mode, node:test/reporters, standalone execution, and reporter-output parity (+18 tests, test_runner 32%→55%) - #34515
node:test: --test CLI mode, node:test/reporters, standalone execution, and reporter-output parity (+18 tests, test_runner 32%→55%)#34515cirospaciari wants to merge 220 commits into
--test CLI mode, node:test/reporters, standalone execution, and reporter-output parity (+18 tests, test_runner 32%→55%)#34515Conversation
`bun run` calls vm.on_exit() before global_exit(), which dispatches the process 'exit' event and drains cleanup hooks. The test command only set exit_code and went straight to global_exit(), so exit handlers never ran under `bun test`. This silently weakened the vendored Node.js test suite. node's common.mustCall(fn, N) verifies its counts from a process 'exit' handler (runCallChecks), so every mustCall count was unchecked: a file calling a mustCall(3) callback once still reported success, where node exits 1 with "Mismatched function calls. Expected exactly 3, actual 1." Handlers run before the bun:test GC roots are released, since they are user JS and may touch still-live state.
Replaces the blanket throwNotImplemented with a real implementation: - node-exact option validation, in node's order (runner.js:731-909), so the error codes and mutually-exclusive pairs match: forceExit×watch, shard×watch, globPatterns×files, env×isolation:'none', and the testTagFilters / testNamePatterns / testSkipPatterns normalization. - TestsStream: a Readable in objectMode with node's buffering, emitting each message as both an event and a stream chunk. - Execution: each file runs in its own `bun test` child, spawned with NODE_TEST_CONTEXT set (node's variable — its own tests branch on it to tell parent from child). The child streams one JSON event per line; unmarked stdout/stderr become test:stdout/test:stderr. The parent republishes them, aggregates counts, and emits a per-file and a run-level test:summary. - node's recursion guard, so a file calling run() on itself doesn't fork forever. Options that cannot be honored yet (watch, coverage, shard, isolation:'none', globPatterns, globalSetupPath) throw rather than being silently ignored. Driving a real file end-to-end produces byte-identical output to node v26.3.0 for test:pass/test:fail (name, nesting, error message) and both summaries. Vendors test-runner-tags-validation.mjs (13/13).
…delity
Adds node v26.3.0's `expectFailure` (xfail) option, which was missing
entirely: the option parser (string label, function/RegExp validator,
object form, and the empty-object rejection), the inverted verdict — a
failing body is the expected outcome, a passing one fails with
failureType 'expectedFailure' — and the `expectFailure` field on the
reported event.
Two node divergences the upstream tests surfaced, both reachable from
plain `bun test`, not just run():
- A skipped suite ran its callback. Node never invokes it, so its
children are never declared and its side effects never happen.
- `{ skip: true, todo: true }` was treated as todo. Node checks skip
first, for both tests and suites.
run() now reports what node reports: the file-level test node emitted
under process isolation (enqueue/dequeue/complete, plus test:fail with
'testCodeFailure' when the file itself dies and 'subtestsFailed' when
its tests do), the skip and todo directive events bun never sent,
details.type, suites counted only in `suites`, and failureType
preserved across the child process boundary.
A vendored test that only drives run() is the parent of that run, not a
test file — Node executes it as a plain script, and under `bun test` a
file registering no tests of its own exits before its run() finishes.
The runner now picks `bun run` for those, gated so a file with any
unindented registration of its own keeps `bun test` rather than
silently passing having tested nothing. 4 of the 88 vendored node:test
files change subcommand, all of them added here.
Vendors 5 upstream tests (expect-error, expect-error-but-pass,
todo-skip-tests, filetest-location, tags-experimental-warning), taking
the test_runner suite from 20 to 26 of 81. Every behavior above was
diffed against the real node v26.3.0 binary.
oxlint's no-duplicate-conditional-property-access flags reading a property in both the condition and the body.
|
Updated 6:48 PM PT - Aug 7th, 2026
✅ @robobun, your commit 8045b1d64a9cdbd339178708cd6114122c03b7e3 passed in 🧪 To try this PR locally: bunx bun-pr 34515That installs a local version of the PR into your bun-34515 --bun |
|
Found 1 issue this PR may fix:
🤖 Generated with Claude Code |
- Child spawn puts execArgv before the test keyword, like node's getRunArgs, so runtime flags land in the child's process.execArgv. - TestsStream's buffer uses createFIFO per the built-in convention (initialized in the constructor: the intrinsic mis-emits in a class-field initializer). - toRegExpPatterns uses the tamper-proof RegExp check. - The exit-handler tests drain both spawned pipes.
|
This PR may be a duplicate of:
🤖 Generated with Claude Code |
d1f4873 to
466ce81
Compare
The class is created on the first run() call so requiring node:test no longer eagerly loads node:stream; applyExpectFailure reuses the existing module-scope require of node:assert.
49ad091 to
5e5768a
Compare
5e5768a to
a9f345f
Compare
…erver asyncDispose
Three fixes behind the five vendored-test failures that surfaced once
`bun test` started running process.on('exit') handlers:
- `bun test` can now drain the event loop after a file's tests finish,
like a node process would before exiting, so in-flight async work
(fs I/O, workers, sockets) completes before exit handlers verify
common.mustCall() counts. Opt-in via BUN_TEST_DRAIN_EVENT_LOOP=1 —
the vendored-node-test runner sets it; bun suites keep
exit-after-tests. A file that leaks a ref'd handle then waits like
node would, bounded by the runner's per-test timeout.
- events.addAbortListener now registers a native abort algorithm
instead of an 'abort' event listener: the native EventTarget drops
node's [kResistStopPropagation] option, so an earlier listener's
stopImmediatePropagation() silenced it. Abort algorithms run in
runAbortSteps() before the event dispatch and cannot be suppressed.
node:events had its own duplicate inline implementation that
bypassed internal/abort_listener entirely; it now delegates.
- Server[Symbol.asyncDispose] resolves immediately when the server is
not listening, like node's lib/net.js guard; a second dispose used
to reject with ERR_SERVER_NOT_RUNNING and re-emit 'close'.
Verified byte-identical to the node v26.3.0 binary on each repro; all
five previously-failing vendored tests pass, and the net, stream and
events suites are unaffected.
a9f345f to
58dbcad
Compare
58dbcad to
2cd35bd
Compare
- addAbortListener: pair the abort algorithm with a once 'abort' listener so events.listenerCount(signal, 'abort') stays at 1 like node's addEventListener path. The algorithm still does the actual work so stopImmediatePropagation cannot suppress it. Fixes 6 vendored node tests (http/http2/https abort-controller, events-on-async-iterator) that the previous commit regressed. - env_var: register BUN_TEST_DRAIN_EVENT_LOOP as a typed boolean and read it via bun_core::env_var (clippy disallows std::env::var_os). - run(): gate only/testNamePatterns/testSkipPatterns so a filter that cannot be honored throws instead of silently running every test. testTagFilters stays validated-but-deferred (upstream validation tests depend on it returning a stream). - TestOptions: add the expectFailure field the constructor now reads.
Matches node's lib/internal/events/abort_listener.js; the shape was dropped when node:events started delegating to the internal module.
The markers are string literals asserting which node:test suite callbacks ran, not action items; rename them so the added-line scan does not match.
2cd35bd to
c2025d1
Compare
Matches node's test.js classification for the case where the body failed but the error did not satisfy expectFailure.match.
No-Verification-Needed: test-only commit, no runtime surface
8c61ade to
385f7a5
Compare
7c6963c to
46e390b
Compare
… Promise.$resolve - republishChildEvent() now reads the name field serializeRunError() already writes, so a child TypeError/AssertionError surfaces with its real name instead of 'Error'. - Server[Symbol.asyncDispose] uses the tamper-proof Promise.$resolve() intrinsic like the rest of src/js.
46e390b to
af8aef4
Compare
… dispose eventListenersDidChange() checks m_abortAlgorithms.isEmpty() to decide whether an AbortSignal.timeout can cancel its timer early; removing the listener while the algorithm is still registered defeats that check.
…cution Three missing Node surfaces, verified against the node v26.3.0 binary: `bun --test` now enters Node's test-runner CLI mode. The flag family is declared hidden in the AUTO/RUN param tables (value-taking ones must be declared or their value parses as the entrypoint, and execArgv's re-parser derives its value-consuming set from AUTO_PARAMS), and boots an embedded driver through the eval path. The driver does node's file discovery (default globs, literal paths, directories, the "Could not find" error and shard filtering), reads the `--test-*` flags back out of process.execArgv, runs files through node:test's run(), and composes reporters over the event stream. Options that cannot be honored yet throw rather than silently dropping behavior. node:test/reporters is a real module now: dot, junit, spec, tap and lcov ported from Node v26.3.0, registered as a prefix-only builtin. Standalone mode: `bun file.js` on a file that uses node:test bootstraps the runner the way Node's harness does — registrations queue, a beforeExit pass executes them with the shim's own machinery, the spec reporter prints, and the exit code reflects failures. Its output is line-identical to node's for the same file. run() children now emit node's full event stream — enqueue/dequeue, complete-before-start flush order, per-scope plans, suite completion events with accurate failed counts, todo inheritance (a failing test in a todo suite reports todo and cannot fail the run) — and print nothing of their own: the banner, per-test lines, summaries, file headers and test-attributed error dumps are suppressed when NODE_TEST_CONTEXT is child-v8, keyed on the exact value so a foreign runner's env cannot silence an unrelated `bun test`. Errors between tests still print and count, else the child would exit 0 and the parent would report a pass. With that, a custom reporter over `bun --test` produces byte-identical event counts to node's own pinned expectation. Vendors 6 more upstream tests (cli-timeout, cli-concurrency, todo-suite-hook-failure, mock-timers-with-timeout, enable-source-maps-issue, root-duration) and their fixtures, taking the test_runner suite from 26 to 32 of 81. Known gaps, all loud or documented: --test-name-pattern / --test-skip-pattern / --test-only throw NotImplemented; multi-file tap keeps per-file numbering; synthesized events carry no declaration line/column; Bun.Glob mis-parses `test/**/*` inside a brace group, so the default pattern ships split in two.
The previous commit dropped the signal local but executeStandaloneQueue still received it, throwing ReferenceError for every isolation:'none' run. Remove the parameter and the per-entry/per-child abort checks it fed — all scheduling paths node's in-process runner does not have.
…ed behavior These asserted that aborting between in-process entries stops the run (second test skipped, test:interrupted, success:false). Real node v26.3.0, run with the identical fixtures and side-effect probes, does the opposite on every field: both tests pass, the second body executes, no interruption, and the summary succeeds — the in-process runner never consults the run signal for scheduling.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
…ck to currentImportFile in inProcessSinkImpl, and drop the stale abort-cancel comment [allow size]
There was a problem hiding this comment.
No issues found in this pass, but this is a large feature PR (~2.5k LOC across the node:test shim, a new node:test/reporters builtin, the --test eval driver, and VM-level uncaught-exception routing) with an open file-layout question vs #23366 — leaving for human review.
Checked: the IS_NODE_TEST_RUN_CHILD gate in VirtualMachine.rs only diverts uncaughts to process listeners after the shim registers in-process, so grandchildren inheriting NODE_TEST_CONTEXT don't get the diverted routing. The is_node_test_child() output-suppression gate is exact-value (== "child-v8") so a foreign runner's env can't silence an unrelated bun test, and between-tests errors still print/count under suppression. The _bunTag envelope in the run-child JSON pipe wraps every non-primitive so user data can't collide with the serializer's own tags, and republishChildEvent clamps forged nesting/duration_ms before they reach reporter .repeat(). The --test eval-driver hookup in Arguments.rs is gated to AutoCommand | RunAsNodeCommand so it can't collide with bun test's own -t.
Extended reasoning...
Overview
This PR implements Node.js's --test CLI mode, the node:test/reporters builtin module (dot/tap/spec/junit/lcov), standalone node:test execution (bun file.js on a file that uses node:test), and run({ isolation: 'none' }). It touches 68 files: ~1600 LOC of new/reworked logic in src/js/node/test.ts, a new 697-line src/js/node/test.reporters.ts, a new 406-line embedded eval driver src/js/eval/node_test.ts, Rust changes across VirtualMachine.rs (uncaught-exception routing), Arguments.rs (29 new hidden --test-* flags), run_command.rs/test_command.rs/bun_test.rs/jest.rs (output suppression + a new native registerRunChild hook), plus module registration in HardcodedModule.rs/isBuiltinModule.cpp. The remaining ~40 files are vendored upstream node fixtures/tests and ~950 lines of new regression tests in node-test.test.ts.
Security risks
Low but non-trivial. The NODE_TEST_CONTEXT=child-v8 env var now suppresses bun test's reporter output and diverts uncaught-exception handling to user-installed process listeners — both gates are exact-value and the routing change requires an in-process registerRunChild() call (not just the env var), so a foreign process setting the env var can't silently mute a bun test or swallow its uncaughts. Child stdout is treated as user-controlled: republishChildEvent clamps nesting and validates duration_ms before they reach reporter .repeat()/jsToYaml, and rebuildError/reviveSerializedValue guard hostile marker payloads (null/forged _bunTag) rather than throwing into the run stream — covered by the new "hostile marker" regression test. No auth/crypto/permission code paths.
Level of scrutiny
High. This is a substantial feature with several architectural surfaces: a process-global IS_NODE_TEST_RUN_CHILD atomic that changes how VirtualMachine::uncaught_exception/unhandled_rejection route errors under bun test, a new embedded eval driver reachable via CLI flags, output suppression across five sites in the native test reporter, process-level SIGINT/SIGTERM handling, and complex cross-process JSON event serialization with tag envelopes. The PR description explicitly flags an open design question (file layout vs #23366) and documents known gaps. This has already been through ~55 iterations with dozens of prior findings addressed, and cirospaciari has been actively shepherding it (including taking design calls on layering), but hasn't approved yet.
Other factors
Test coverage is strong — 18 vendored upstream node tests plus ~950 lines of new test.each × isolation-mode regression tests pinning verbatim node v26.3.0 output for hook failures, describe-body throws, verdict numbering, cause serialization, and the env-leak/marker-injection edge cases. All prior bot findings on the timeline are resolved. Given the scope, the VM-routing change, and the open #23366 layout question, this warrants a maintainer's sign-off rather than auto-approval.
|
CI on 42478f1 (build 80168): |
…mode # Conflicts: # src/runtime/cli/test_command.rs # src/runtime/test_runner/bun_test.rs
…e() instead of loading node:os at module top [allow size]
…Process twin (nodejs/node #54832) [allow size]
run_command.rs: keep exec_eval — a superset of main's inline synthetic-[eval] path (same cwd boot plus the positional merge --test needs). No-Verification-Needed: merge resolution keeping the branch's verified superset helper; CI verifies the merged tree
…ped flag so {skip: ''} is a directive everywhere [allow size]
There was a problem hiding this comment.
This automated review pass found no new issues. Given the size of the change — a substantial rework of src/js/node/test.ts (standalone/in-process/run-child state machines, AsyncLocalStorage-based uncaught attribution), a new node:test/reporters module, a new --test eval driver, and native changes to VirtualMachine.rs/test_command.rs/bun_test.rs for uncaught routing and output suppression — a human sign-off is still warranted before merge.
What was reviewed: the run-child event pipeline (numbering, plan/complete ordering, error serialization round-trip including NaN/BigInt/circular), hook-failure and describe-throw cancellation across both isolation modes, the is_node_test_child() gating in the Rust reporter paths, and the NODE_TEST_CONTEXT exact-value check on both the JS and Rust sides. The finder-flagged dead runOptions.only assignment in eval/node_test.ts was examined — the --test-only fatal below it makes the true case unreachable, so it's a harmless no-op.
Extended reasoning...
Overview
This PR implements node's --test CLI mode, the node:test/reporters builtin, and standalone node:test execution. It touches 68 files: ~1200 lines of state-machine changes in src/js/node/test.ts (standalone queue, in-process run({isolation:'none'}), run-child event emission with enqueue/dequeue/complete/plan/start ordering, AsyncLocalStorage-based uncaught/unhandled attribution, hook-failure cancellation), a new 665-line test.reporters.ts port, a new 349-line eval/node_test.ts CLI driver, native changes to VirtualMachine.rs (IS_NODE_TEST_RUN_CHILD gating uncaught-exception routing), test_command.rs/bun_test.rs/jest.rs (is_node_test_child() output suppression), Arguments.rs (26 hidden --test-* flags), and module resolution wiring. The rest is 18 vendored upstream tests, ~35 fixtures, and ~900 lines of local regression tests.
Security risks
None identified. The NODE_TEST_CONTEXT env var is checked for the exact value child-v8 on both the JS and Rust sides, so a stray env var from a foreign runner cannot silence a plain bun test (a regression test pins this). The jsNodeTestRegisterChild Rust binding is only reachable from the builtin module and only flips a process-local atomic. File discovery uses Bun.Glob under the caller's cwd; no path traversal beyond what run({files}) already permits.
Level of scrutiny
High. This is a large behavioral feature spanning JS builtins, an eval-driver bootstrap path in Arguments.rs, and native uncaught-exception routing in VirtualMachine.rs. The test.ts changes introduce three interleaved execution modes (run-child, standalone, in-process) sharing a single event-emission layer with subtle ordering invariants. This is well beyond the auto-approval threshold.
Other factors
This PR has been through many prior automated review rounds; every inline finding I raised is resolved, and each fix carries a targeted regression test in node-test.test.ts (74 tests, verified against real node v26.3.0 output). The maintainer (cirospaciari) has been actively iterating on the branch. CI was reported green on all lanes as of build 80168. The PR description documents known gaps (name/skip/only filters throw, multi-file TAP renumbering, missing line/column) and notes overlap with #23366 for maintainer arbitration. No outstanding unresolved review threads.
3 participants
Implements node's
--testCLI mode, thenode:test/reportersmodule, and standalone node:test execution. Stacked on #34444 (base:claude/node-test-run-api) — only the last commit is new here.Fixes #24444.
Note: #23366 (nektro) also adds
node:test/reporters, from before the Zig→Rust rewrite. This version wires the module into the current Rust loader and is driven end-to-end by the upstream tests below (a custom reporter overbun --testhits node's pinned event-count expectation byte-for-byte); happy to defer to that PR's file layout if preferred.Every behavior was diffed against the real
node v26.3.0binary; standalone output is line-identical to node for the same file, and a custom reporter overbun --testproduces byte-identical event counts to node's own pinned expectation:bun --test(CLI mode)--testflag family is declared (hidden) in the AUTO/RUN param tables and boots an embedded driver through the eval path —process.execPath --test ...now works, which is what ~25 upstream test_runner tests spawn.Could not find '...'+ exit 1,--test-shardwith node's exact error messages.--test-reporter/--test-reporter-destinationwith node's arity rules, custom reporters via module specifier (file URLs, relative paths, node_modules packages), reporter setup/stream errors exit 7 like node.NODE_DEBUG=test_runnerprints the run options (whattest-runner-cli-{timeout,concurrency}assert on).--test-name-pattern,--test-skip-pattern,--test-only,--test-randomize) throw rather than silently running everything.node:test/reportersdot,junit,spec,tap,lcov, ported from node v26.3.0 and registered as a prefix-only builtin (likenode:testitself). Colors come from the existinginternal/util/colors.Standalone mode
bun file.json a file that uses node:test now bootstraps the runner the way node's harness does (lazy on first registration, executed onbeforeExit, spec reporter by default,--test-reporterhonored, exit code from failures). Registrations run in declaration order, so output ordering matches node exactly.run() child event fidelity
Children now emit node's full stream:
test:enqueue/test:dequeue, node's complete-before-start flush order, per-scopetest:plans, suite completion events with accurate failed counts, and inherited todo (a failing test inside a todo suite reports todo and cannot fail the run — including a todo suite's failingbeforehook, which node treats as advisory).Children print nothing of their own: banner, per-test lines, summaries, file headers and test-attributed error dumps are suppressed when
NODE_TEST_CONTEXTis exactlychild-v8(so a foreign runner's env can't silence an unrelatedbun test). Errors between tests still print and count — otherwise the child exits 0 and the parent reports a false pass.Coverage
Vendors 6 more upstream tests —
test-runner-cli-timeout,test-runner-cli-concurrency,test-runner-todo-suite-hook-failure,test-runner-mock-timers-with-timeout,test-runner-enable-source-maps-issue,test-runner-root-duration— plus their fixtures. test_runner: 26 → 32 of 81 on top of #34444's 20 → 26.test-runner-xfail.jsandtest-runner-exit-code.jsare close (standalone mode is most of what they need) but pin TAP details not yet met (per-test declaration positions,Interrupted while running:on SIGINT).Known gaps (all loud or documented)
--testkeeps per-file TAP numbering (node renumbers cumulatively across files).line/column; the spec/tap ports omit the location instead of printingundefined:undefined.Bun.Globmis-parsestest/**/*nested inside a brace group (returns zero matches for the whole alternation), so node's default pattern ships split into two globs. Worth a separate Bun.Glob issue.run({ isolation: 'none' })still throwsGLOBAL_ORDERvs the node binary),only/tag-filter pruning,testId/parentIdon every per-test event, andthisbound to the context for tests/hooks/describe callbacks. +4 more upstream tests (tags-events,no-isolation,no-isolation-different-cwd,test-id): test_runner 32 → 36 of 81.no test proof · iteration 70 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/node/test_runner/node-test.test.ts
Compat impact (upstream node v26.3.0 test files vendored, in-tree = passing)