Repository navigation
Conversation
…ment run() started each file as `bun test <file> ...argv`, and `bun test` reads every token after the keyword as its own flag or file filter. The values never reached process.argv of the test file. The child's command line no longer has argv. run() puts the file and the values in BUN_INTERNAL_NODE_TEST_ARGV, and `bun test` decodes them into process.argv.slice(2) before a preload runs. A `bun test` that inherits the variable and is started for another file ignores it.
|
No actionable comments were generated in the recent review.
|
| Layer / File(s) | Summary |
|---|---|
Encode and forward run() arguments docs/runtime/nodejs-compat.mdx, src/bun_core/env_var.rs, src/js/node/test.ts, test/js/node/test_runner/fixtures/41-run-argv.mjs, test/js/node/test_runner/node-test.test.ts |
run() encodes the file path and arguments in a length-prefixed value passed through the child environment. NUL bytes produce an invalid-argument error. The documentation and tests cover argv values, execArgv options, and child behavior. |
Decode arguments for the matching test VM src/runtime/cli/test_command.rs, test/js/node/test_runner/node-test.test.ts |
The test command parses the encoded value and applies arguments only when its file matches a test positional. It removes the internal variable from the VM environment. Tests cover malformed, empty, matching, and inherited values. |
Handle parallel test runs src/runtime/cli/test/parallel/runner.rs, test/js/node/test_runner/node-test.test.ts |
The parallel coordinator exits with status 1 when VM arguments are nonempty. Tests cover single-file and multi-file --parallel behavior. |
Suggested reviewers: cirospaciari, jarred-sumner, alii
Priority:
Merge Risk: 
Minimal · up to 709de
-
The reviewed argument-forwarding change has no established merge-blocking issue; it is ready for normal checks.
Pre-merge checks: 
4
-
Full details: Title check
-
Explanation
The title names the main change: node:test run({ argv }) now delivers argv to the test file through the child's environment. It matches the changeset, which adds BUN_INTERNAL_NODE_TEST_ARGV, decodes it in TestCommand::exec, and stops passing argv on the bun test command line. It is specific and concise.
Full details: Description check
-
Explanation
The description covers the problem, the fix, verification, and downsides. It also gives extra background, measurements, and open questions. The template has two sections, 'What does this PR do?' and 'How did you verify your code works?'. The description uses Problem, Fix, and Background headings instead. It does not use the template headings, but it contains the same information: what changes, and how it was verified. Verification is detailed with test counts, platforms, and Node.js comparison. Windows and macOS were not run, and the description says so.
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
|
Updated 8:16 AM PT - Oct 25th, 2026
✅ @robobun, your commit 709dea3d4143d3d005e482db4712e72228360150 passed in 🧪 To try this PR locally: bunx bun-pr 46847That installs a local version of the PR into your bun-46847 --bun |
|
Status Reproduction, with two files: // only.test.mjs
import { test, describe } from "node:test";
test.only("focused", () => {});
test("unfocused", () => {});
describe("d", { only: true }, () => { test("in-only-suite", () => {}); });
describe("e", () => { test("plain-in-suite", () => {}); });
// run-argv.mjs
import { run } from "node:test";
let n = 0;
for await (const { type, data } of run({ files: [process.argv[2]], argv: [process.argv[3]] })) {
if ((type === "test:pass" || type === "test:fail") && data.details.type === "test") n++;
}
console.log("test events:", n);
PR: #46847 |
There was a problem hiding this comment.
Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.
Beyond the inline findings, I also checked the BUN_INTERNAL_NODE_TEST_ARGV decoder in src/runtime/cli/test_command.rs for adversarial lengths — parse_unsigned(...).ok()? and split_at_checked(len)? turn an oversized or non-numeric prefix into the exit-1 error rather than a panic or out-of-bounds read, and the gate on BUN_TEST_DRAIN_EVENT_LOOP means a plain bun test never reads the variable. The coordinator-side strip in runner.rs only drops the variable when the coordinator itself rejected it, so workers cannot pick up a value the coordinator did not accept.
Extended reasoning...
The change moves node:test run({ argv }) off the child's command line into a length-prefixed env var decoded in Rust before preloads run, touching src/js/node/test.ts, test_command.rs, the parallel coordinator, and new tests. The decoder parses untrusted env bytes; its length arithmetic uses checked parsing and split_at_checked, so malformed input exits with a message rather than panicking. The inline findings (env-string size limit, grandchild re-spawn seeing argv, test convention nits) are behavioral trade-offs a human should weigh, so this is not approved.
…ment The `bun test` that run() starts removes BUN_INTERNAL_NODE_TEST_ARGV from its environment map when it has read the values. `process.env` of the test file does not have the variable, and neither does a process that its main thread starts. A test file that starts itself once more reads no argv there, as in Node.js. A Worker thread still has the variable, because a Worker loads the environment of the process again. A `--parallel` child of run() that has argv exits with an error before it starts workers: a worker is not the process that run() starts. With one test file there is no worker, and the file gets argv. run() takes the file name for the variable from the path that it resolved, not from the last argument of the command line. The file is compared with the paths that `bun test` was given, not with the command name. Docs: flags for the child `bun test` go in execArgv, each as one element. Tests: the ones whose child runs a test file are not concurrent, and use the default timeout.
Problem
run({ files, argv })ofnode:testmust giveargvto each test file. The childbun testparses it, and the file reads[].argv: ["--only"]runs 0 of 4 tests and passes.runOneFile(src/js/node/test.ts:456):[execPath, ...execArgv, "test", file, ...argv].Fix
runOneFileputs the file path and the values in the child'sBUN_INTERNAL_NODE_TEST_ARGV, not on its command line.TestCommand::exec(src/runtime/cli/test_command.rs) decodes it intoprocess.argv.slice(2)before a preload runs, then removes it fromprocess.env. Correct becausebun testparses no caller value.test/js/node/test_runner/node-test.test.ts(19 new tests, 15 fail on main), the vendoredtest-runner-*files andtest/cli/test/parallel.test.ts. Node.js v26.3.0 passes the shared fixture.Background
run()starts each file as abun testchild, which parses every token after the file. Node.js startsnode <file> ...argv.ERR_NOT_IMPLEMENTEDgate forargv: it ends the false pass and delivers nothing. Take it if you do not want the variable. A count flag andbun test -- <args>(bun test: forward args after -- to process.argv #35622) change the command line.Downsides
argv: ["--bail"]no longer reachesbun test. UseexecArgvwith--flagor--flag=value.bun teststart: +13 instructions, 0 newgetenvcalls..textgrows 1,536 B.argvshares one environment string: 128 KiB on Linux, where Node.js allows that for each value.argvwith--parallelworkers is an error.Notes
What the child is given
run({ files: ["a.test.mjs"], argv: ["--only", "x"] })startsbun test /abs/a.test.mjs, as main does for an emptyargv. The environment of the child hasBUN_INTERNAL_NODE_TEST_ARGV=15:/abs/a.test.mjs6:--only1:x. Each entry is its UTF-8 byte length,:and the bytes. The first entry is the file, from the path thatrun()resolved.bun testreads the variable after it has loaded its environment, before bunfig preloads,--preloadand the file. The values are not inprocess.execArgv. A value that does not decode prints an error and exits 1. An empty value is no value.bun testthen removes the variable from its environment map.process.envandBun.envof the test file do not have it. A process that the main thread starts does not have it, throughBun.spawnwithoutenv,node:child_processorBun.$. So a test file that startsbun teston its own path reads[]there, as in Node.js.argvis read when each file starts, as in Node.js. A value pushed afterrun()returns, or insetup, reaches the files.argv,runOneFilesets the variable toundefined. So a variable that the caller's own process has does not reach the child.bun testreads the variable only whenBUN_TEST_DRAIN_EVENT_LOOPis set, whichrun()sets in the same object. Everybun testalready reads that flag once, so a plain start does not look the new variable up. Both are read from the process environment, not from.envfiles.bun testtakes the values only when that file is one of the paths on its command line. This is the second check, for a process that still inherits the variable (see Limits).execArgv, the file orargvthrowsERR_INVALID_ARG_VALUEforargs[N], for the first argument of the command line that has one, as on main. The name of the variable is not in a message.Node.js and Bun on the same file
run()10 times. That starts 12 processes, one of them from a test file. The cases: noargv, flags ofbunandbun test, paths of other test files, values that a parser or a shell changes,--inexecArgv,--onlyand--test-onlyon a file with 4 tests, a preload and a CommonJS file, two files withargvchanged after the call, a file that starts itself once more, and the case of Node'stest-runner-run.mjs.run({ files: [only.test.mjs], argv: ["--only"] })gives 4 test events under Node.js, 1 on main, 4 on this branch. For["--test-only"]: 4, 4, 4.argvis an option of the childbun test.--preload xloads a module,--watchnever ends,.runs every test file of the directory,--onlyand--helpreport a pass with 0 tests run.run({ argv }),test/parallel/test-runner-run.mjs, importsnode:test/reporters, which Bun does not have. So this PR adds no passing Node.js test file. Its caseshould allow custom arguments via argvis in the fixture with its fileprint-arguments.js.test:failof the file and goes on. Bun ends the stream with it, on main too, as for every child that it cannot start.Who this is for
run({ argv }), and no user reported the false pass.run()has been in releases since v1.4.0.run()ofnode:testand none passesargv.FluffyLabs/typeberry(bin/test-runner/index.ts), passesargv: process.argv.slice(3). It also passesconcurrency: trueand importsnode:test/reporters. Both stop it in Bun today, before a child starts:ERR_NOT_IMPLEMENTEDandERR_UNKNOWN_BUILTIN_MODULE. node:test:--testCLI mode, node:test/reporters, standalone execution, and reporter-output parity (+18 tests, test_runner 32%→55%) #34515 removes both stops and keepsargvon the command line ofbun test.bun test <file> -- <args>(Unable to pass arguments to tests by using -- to separate args #9162) is a different request. This PR does not change it.Measurements (release builds, linux-x64, merge base 5e18294 against this branch)
getenvcalls onbun test ./one.test.ts: 61 to 61. The new variable is not read. Arun()child withoutargv: 61 to 62. A plain--parallelcoordinator: 60 to 60 (gdb).bun teststart: +13.TestCommand::execitself runs 5 more from its entry to its call ofrun_all_tests(1,030 to 1,035, calls stepped over), and one more cached read of the flag is 8 (gdb).--parallelrun: +2 instructions inrun_as_coordinator(113 to 115 up to the first worker environment).Command::start,which,create_context_data,arguments::parse,ComptimeClap::parse_with_table,StreamingClap::next,exec_auto_or_run,write_context_no_parse,exec_test)..text(82,164,617 to 82,166,153), +0 B.rodata, equal file size. The new cold function is 979 B,TestCommand::execgrows 273 B andrun_as_coordinator80 B. The accessor adds 16 B of statics (size, nm).run()child with 3 values: 4 allocator entries and 1 free in the decoder (gdb).argvis omitted. 5 to 21 with 8 values, of which 17 are strings (heapStats, N=2000, JIT off, measured on the old and the new expressions outsiderun()).argv(/proc/self/cmdlineread by the test file).argvshapes with the same events as Node.js v26.3.0: 47 of 48 (main: 0 of 48).Differences from Node.js v26.3.0
argv: ["--env-file=nope"]is the one shape of the 48 that differs.nodereads--env-fileat any position of its command line and fails the file. Bun gives the value to the file.[], 9 fail the file or the test, 4 also run a second test file, 2 pass with 0 tests run, 1 hangs, 1 runs the test twice.Limits left
Size. All values share one environment string. Linux allows 128 KiB for one string. Measured with Node.js v26.3.0: for two values of 70,000 bytes, Node.js gives both to the file, main runs the file with
[], and this branch ends the stream withE2BIG: argument list too long. For one value of 200,000 bytes all three fail withE2BIG. macOS has no limit for one string. On Windows one variable has the limit that the whole command line of Node.js has (32,767 characters). One variable for each value removes the limit. It needs a set of numbered names that the writer, the reader and the removal each walk. I kept one variable.--parallel. A child ofrun()that hasargvand starts workers exits 1 withrun() of node:test cannot give argv to the workers of --parallel. A worker is not the process thatrun()starts, andrun()gets no test events from a worker (the same on main). This needs a directory or a name filter infiles, and--parallelinexecArgv. Node.js fails a directory infiles. With one test file there is no worker, and the file getsargv.Workerthreads. The removal is from the map of the main thread's VM. AWorkerloads the environment of the process again (Loader::clone_for_worker,src/dotenv/env_loader.rs), so it still has the variable:process.envBun.spawnSync, noenvnode:child_processnew Worker(url)new Worker(url, {})node:worker_threadsOne result changes with it: a
bun testthat a Worker starts for the same path reads the values ofrun(). worker: resolve fetch() proxy env (and the rest of the env loader) from the worker's own environment #41932 changes that loader (a Worker keeps the map of its parent). It is open and has conflicts. Atest.failingcase marks the first Worker row.Of the three variables that
run()sets, this PR removes one from the child's environment.NODE_TEST_CONTEXTis inherited as in Node.js.BUN_TEST_DRAIN_EVENT_LOOPstill reaches the test file, its children and--parallelworkers, as on main.execArgv: ["-e", code]or-p: the child is notbun test. On main its script readstest, the file and the values as arguments. Now it readstestand the file, and its children inherit the variable.A flag of
bun testinexecArgvmust be one element:--timeout=60000.["--timeout", "60000"]fails withScript not found "60000", on main too. For--preloadand the other runtime flags that take a value, that is cli: take flag arity from the auto table when the subcommand is classified #41511.execArgv: ["--only"]on anode:testfile still reports a pass with 0 tests run. That is the--onlyrule ofbun test(node:test: filter ononlyunder --test-only #46797), notargv.The other effects of the child line
bun <flags> test <file>are not changed: the caller's runtime flags are not handed on (node:test: give each file of run() the runtime flags of its caller #45549),--eval=and--print=inexecArgvdo not evaluate, and a compiled executable starts itself again.Seen in CI with the first commit, not changed here
argvto the workers of a--parallelchild. Its three tests for that passed on Linux and macOS and timed out after 30 s on Windows 11 aarch64 and Windows 2019 x64 (build 132400). The other 60 tests of the file passed there.BUN_TEST_DRAIN_EVENT_LOOP=1. In one of them no code of this PR ran in the worker past one lookup that found nothing.vm.on_before_exit()after each file (test_command.rs), which loops while the event loop is alive. On Windows the worker's pipe to the coordinator is auv::Pipethat reads and is never unref'd (test/parallel/Channel.rs), so the loop stays alive and the worker does not report the file.BUN_TEST_DRAIN_EVENT_LOOP=1 bun test --parallel=2does not end while a file has a timer, which is what the flag asks for. By my reading, on Windows the worker's own pipe has the same effect.run()sets the flag for its child on main, and the coordinator hands its environment to the workers.argv, so it has no test on that path. No issue tracks the hang.Other designs
ERR_NOT_IMPLEMENTEDfor a non-emptyargv: one line beside the other gates ofrun()(src/js/node/test.ts:320-332) and one test. It ends every false pass fromargvwith no native code and no variable. It breaksargv: ["--bail"]in the same way as this PR, and the test file still gets nothing. If you do not want to own the variable yet, I will send that.runOneFileappendedargvand--node-test-argc=<n>, andexec_testcut that tail off the argv view before the parse, asboot_standalonedoes. I built and measured it: +16 to +26 instructions perbun teststart, 47 of 48 shapes. The self-review found two defects that no small change removes. The flag is a legal value of another flag in the last position, sobun test f -t --node-test-argc=1lost its name filter with norun()involved. A flag that is not last is skipped as unknown, and the values are flags ofbun testagain.--node-test-argv=<value>): the parser still reads the caller's bytes, and withexecArgv: ["--"]they are file filters.bun test <file> -- <args>(Unable to pass arguments to tests by using -- to separate args #9162, bun test: forward args after -- to process.argv #35622): a change of the command line for every user. This PR does not touch the command line, so it neither needs nor blocks that decision. If it lands,run()can use it and the variable can go.bun <file> ...argv): node:test:--testCLI mode, node:test/reporters, standalone execution, and reporter-output parity (+18 tests, test_runner 32%→55%) #34515 letsbun <file>run anode:testfile, and keepsbun testas the child ofrun(). Abun <file>child is a separate decision: it fails files that usebun:test, and it ignoresbun testflags inexecArgvand[test]of bunfig. The variable stays for as long as the child isbun test.bun run <file> <args>has: it changesbun test a.test.ts --timeout 5000unless the stop depends on the environment.--preload=node:testand the variable): a preload from bunfig.toml loads before it and reads[].Loader::load_process, as forNODE_CHANNEL_FD: it also hides the variable from Workers. It adds one comparison for each environment entry to every start of every command.Questions for a maintainer
argvdelivered now, or theERR_NOT_IMPLEMENTEDgate untilbun testhas a place for arguments of the file?run()? Three open PRs touch the same object: node:test: enforce run({ timeout }) as the inherited per-test timeout #36449 (BUN_NODE_TEST_RUN_TIMEOUT), node:test run(): report child events in byte frames that printed text cannot produce #45003 (BUN_NODE_TEST_RUN_FRAMES) and node:test: filter ononlyunder --test-only #46797 (a--test-onlyflag on the command line). The names can follow whichever you pick.Other open PRs
onlyunder --test-only #46797 and node:test: give each file of run() the runtime flags of its caller #45549 rewrite the sameargsstatement. Each must drop...(opts.argv as string[])from it. The encoder here takes the file fromabsolute, not from the end ofargs, so the order of a merge does not change what the test file gets. node:test: filter ononlyunder --test-only #46797 can then delete its statedargvlimit. The PR that merges second owes a test forrun({ only: true, argv }).--testCLI mode, node:test/reporters, standalone execution, and reporter-output parity (+18 tests, test_runner 32%→55%) #34515 has a test that passesargv: ['--bail']to reachbun test. After this PR it must useexecArgv: ['--bail'], which still stops the child after the first failure.onlyunder --test-only #46797 has fixtures 30 to 40. The fixture here is 41.Tests
41-run-argv.mjs(2node:testtests, for both runtimes) and 19 tests indescribe("node:test run({ argv })"). On a release build of main 15 of the 19 fail. The four that pass on both are the Node.js run of the fixture, the marker for the Worker limit, and two cases where abun testmust not take the variable.bun testchildren and takes 7 s to 10 s on a debug build. The tests whose child runs a test file are not concurrent, so each has the default timeout for its own child (1.2 s to 2.0 s on a debug build). As concurrent tests they took 2.2 s to 4.8 s, and 1 run of 5 went past 5 s.test/js/node/test_runner/node-test.test.ts64 pass. The vendoredtest-runner-*files, run as CI runs them: 25 of 25.test/cli/test/parallel.test.tsandparallel-startup-failure.test.ts: 49 pass, 1 skip.each worker has a unique JEST_WORKER_ID and BUN_TEST_WORKER_ID, depends on timing in a debug build. In 16 runs each it passed 7 times on the first commit and 9 times on main.test/cli/test/{bun-test,isolation,rerun-each,pass-with-no-tests,test-shard}.test.ts: 197 pass, 6 todo, 2 fail.test/internal/source-lints/: 181 pass, 2 fail. The four failures are tests that need more than the default 5 s on a debug build (twonested too deeply to printcases, and two ofsema-parser-recursion.test.ts). They pass on a release build of this branch.bun run rust:check-all: 12 of 12 targets.Self-review
script_or_command.rs. Those two stop at the first entry that does not decode, and this one must fail there.--parallelworker path is gone, the file comes from the resolved path, the two-file case is back, the docs say where flags ofbun testgo, a test reads the environment thatBun.spawngives, and this body says who asked.Loader::clone_for_workerthat closes the Worker limit. It changes what every Worker loads, and worker: resolve fetch() proxy env (and the rest of the env loader) from the worker's own environment #41932 has it.ERR_NOT_IMPLEMENTEDgate in place of this change. It is the first question above.