From f1852bc3f0674822a41df92cfd4d2256cb055b82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christopher=20Mart=C3=ADnez?= Date: Sun, 28 Jun 2026 01:51:02 -0600 Subject: [PATCH] test_runner: stop reading process state in run() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit run(), getRunArgs(), and runTestFile() reached into ambient process state (process.execArgv, process.env, and process.env.NODE_TEST_CONTEXT) from their bodies, which made the result of run() depend on the host process. Capture references to process.execArgv and process.env once at the run() boundary and thread them through the internal opts object, and read the NODE_TEST_CONTEXT recursion guard once into a module-level constant. Behavior is unchanged for the CLI and the programmatic run() API, and no new public option is added. Also add a regression test for the previously untested default process.env inheritance path. Fixes: https://github.com/nodejs/node/issues/53867 Signed-off-by: Christopher Martínez --- lib/internal/test_runner/runner.js | 27 ++++++++++++++++--- .../test-runner/process-env-inherited.js | 6 +++++ test/parallel/test-runner-run.mjs | 16 +++++++++++ 3 files changed, 45 insertions(+), 4 deletions(-) create mode 100644 test/fixtures/test-runner/process-env-inherited.js diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js index f9442be8ed164b..574d496674350b 100644 --- a/lib/internal/test_runner/runner.js +++ b/lib/internal/test_runner/runner.js @@ -108,6 +108,11 @@ let debug = require('internal/util/debuglog').debuglog('test_runner', (fn) => { }); const kIsolatedProcessName = Symbol('kIsolatedProcessName'); +// NODE_TEST_CONTEXT is set on a child's environment in runTestFile(). It is +// inherited at process creation, before any user code runs, and is never +// mutated in-process, so capturing it once keeps run() from reading ambient +// process state on every call. +const isTestRunnerChildProcess = process.env.NODE_TEST_CONTEXT !== undefined; const kFilterArgs = [ '--test', '--experimental-test-coverage', @@ -193,7 +198,8 @@ function getRunArgs(path, { forceExit, randomize, randomSeed, root: { timeout }, - cwd }) { + cwd, + processExecArgv }) { const processNodeOptions = getOptionsAsFlagsFromBinding(); const runArgs = ArrayPrototypeFilter(processNodeOptions, filterExecArgv); @@ -207,7 +213,7 @@ function getRunArgs(path, { forceExit, */ const nodeOptionsSet = new SafeSet(processNodeOptions); const unknownProcessExecArgv = ArrayPrototypeFilter( - process.execArgv, + processExecArgv, (arg) => !nodeOptionsSet.has(arg), ); ArrayPrototypePushApply(runArgs, unknownProcessExecArgv); @@ -496,7 +502,7 @@ function runTestFile(path, filesWatcher, opts) { const subtest = opts.root.createSubtest(FileTest, testPath, testOpts, async (t) => { const args = getRunArgs(path, opts); const stdio = ['pipe', 'pipe', 'pipe']; - const env = { __proto__: null, NODE_TEST_CONTEXT: 'child-v8', ...(opts.env || process.env) }; + const env = { __proto__: null, NODE_TEST_CONTEXT: 'child-v8', ...(opts.env || opts.processEnv) }; // Acquire a worker ID from the pool for process isolation mode let workerId; @@ -967,6 +973,17 @@ function run(options = kEmptyObject) { testFiles.length, ); + // Capture references to ambient process state once, at the run() boundary, so + // the exec args and environment forwarded to child test processes are fixed + // here rather than re-read later inside the per-file subtest callback. + // processExecArgv carries the parent's V8/exec-only flags (e.g. + // --allow-natives-syntax) and is distinct from the public `execArgv` option; + // processEnv is what children inherit when the `env` option is omitted. These + // stay live references (not copies) so later process.env mutations remain + // visible when each child's env is built. + const processExecArgv = process.execArgv; + const processEnv = process.env; + const opts = { __proto__: null, root, @@ -990,10 +1007,12 @@ function run(options = kEmptyObject) { randomize, randomSeed, testFiles, + processExecArgv, + processEnv, }; if (isolation === 'process') { - if (process.env.NODE_TEST_CONTEXT !== undefined) { + if (isTestRunnerChildProcess) { process.emitWarning('node:test run() is being called recursively within a test file. skipping running files.'); root.postRun(); return root.reporter; diff --git a/test/fixtures/test-runner/process-env-inherited.js b/test/fixtures/test-runner/process-env-inherited.js new file mode 100644 index 00000000000000..485d4c3b4ab676 --- /dev/null +++ b/test/fixtures/test-runner/process-env-inherited.js @@ -0,0 +1,6 @@ +const { test } = require('node:test'); + +test('process.env is inherited when no env option is provided', (t) => { + t.assert.strictEqual(process.env.INHERITED_VAR, 'XYZ', 'parent env var should be inherited by default'); + t.assert.strictEqual(process.env.NODE_TEST_CONTEXT, 'child-v8', 'NODE_TEST_CONTEXT should be set by run()'); +}); diff --git a/test/parallel/test-runner-run.mjs b/test/parallel/test-runner-run.mjs index b6eb6b6af51877..43310261fdeb1b 100644 --- a/test/parallel/test-runner-run.mjs +++ b/test/parallel/test-runner-run.mjs @@ -836,6 +836,22 @@ describe('env', () => { delete process.env.ABC; }); + it('should inherit process.env when the env option is omitted', async () => { + // Set a variable on the main process env and confirm the spawned test + // inherits it when the env option is omitted (the default behavior). + process.env.INHERITED_VAR = 'XYZ'; + + try { + const stream = run({ files: [join(testFixtures, 'process-env-inherited.js')] }); + stream.on('test:fail', common.mustNotCall()); + stream.on('test:pass', common.mustCall(1)); + // eslint-disable-next-line no-unused-vars + for await (const _ of stream); + } finally { + delete process.env.INHERITED_VAR; + } + }); + it('should throw error when env is specified with isolation=none', async () => { assert.throws(() => run({ env: { foo: 'bar' }, isolation: 'none' }), { code: 'ERR_INVALID_ARG_VALUE',