Skip to content

test_runner: avoid reading process state directly in run() #62377

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
murataslan1 wants to merge 3 commits into
base: main
Choose a base branch
from
Open

test_runner: avoid reading process state directly in run() #62377

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e1746a9
test_runner: avoid reading process state directly in run()
murataslan1 Mar 21, 2026
acf2faa
test_runner: address review feedback
murataslan1 Mar 22, 2026
37fc25a
test_runner: validate globPatterns as string array
murataslan1 Apr 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
test_runner: avoid reading process state directly in run() 
Refs: #53867

The run() function is exposed as a public API via node:test module.
Previously, it accessed process.argv, process.execArgv, process.cwd(),
and process.env directly, which meant programmatic API users could not
fully control the test runner behavior.

This change:
- Captures process.argv, process.cwd(), process.env, and
  process.execArgv in the CLI entry point (main/test_runner.js) and
  passes them explicitly to run() as options
- Adds a processExecArgv option for V8 flag propagation
- Replaces process.env fallback in runTestFile() with opts.env
- Replaces process.argv usage in getRunArgs() with globPatterns
- Replaces process.env.NODE_TEST_CONTEXT check with env option
- Maintains backwards compatibility: when options are not provided,
  run() falls back to process state as before
  • Loading branch information
@murataslan1
murataslan1 committed Mar 21, 2026
commit e1746a9e19f5c02d6d40ddad5e290a9c4e2950cb
4 changes: 4 additions & 0 deletions lib/internal/main/test_runner.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,11 @@ if (isUsingInspector() && options.isolation === 'process') {
options.inspectPort = process.debugPort;
}

// Capture process state explicitly so run() does not need to access it.
options.globPatterns = ArrayPrototypeSlice(process.argv, 1);
options.cwd = process.cwd();
options.env = process.env;
Copy link

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

options.env is being set unconditionally, which makes run() think the caller explicitly provided an env option. With --test-isolation=none, run() currently rejects options.env, so this change will break node --test --test-isolation=none. Consider only setting/passing options.env in the CLI when isolation='process' (or update run() to treat this captured env as non-user-provided).

Suggested change
options.env = process.env;
if (options.isolation === 'process') {
options.env = process.env;
}

Copilot uses AI. Check for mistakes.
options.processExecArgv = process.execArgv;
Copy link
Copy Markdown
Member

@avivkeller avivkeller Mar 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How's this differ from execArgv?

Copy link
Copy Markdown
Author

@murataslan1 murataslan1 Mar 22, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah you're right, dropped it. The idea was to avoid the direct process.execArgv read, but it doesn't really make sense as a user-facing option — it's just the parent's V8 flags that get forwarded to child processes. Reverted to reading it directly.


debug('test runner configuration:', options);
run(options).on('test:summary', (data) => {
Expand Down
32 changes: 26 additions & 6 deletions lib/internal/test_runner/runner.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,8 @@ function getRunArgs(path, { forceExit,
only,
argv: suppliedArgs,
execArgv,
processExecArgv,
globPatterns,
rerunFailuresFilePath,
Copy link

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

globPatterns is forwarded into runArgs via ArrayPrototypePushApply() (so it must be an array of strings). Currently run() only ensures it's an array; consider validating its element types (e.g., string array) to avoid child-spawn argument errors caused by non-string entries.

Copilot uses AI. Check for mistakes.
root: { timeout },
cwd }) {
Expand All @@ -183,7 +185,7 @@ function getRunArgs(path, { forceExit,
*/
const nodeOptionsSet = new SafeSet(processNodeOptions);
const unknownProcessExecArgv = ArrayPrototypeFilter(
process.execArgv,
processExecArgv,
(arg) => !nodeOptionsSet.has(arg),
);
ArrayPrototypePushApply(runArgs, unknownProcessExecArgv);
Expand Down Expand Up @@ -214,7 +216,9 @@ function getRunArgs(path, { forceExit,

if (path === kIsolatedProcessName) {
ArrayPrototypePush(runArgs, '--test');
ArrayPrototypePushApply(runArgs, ArrayPrototypeSlice(process.argv, 1));
if (globPatterns != null) {
ArrayPrototypePushApply(runArgs, globPatterns);
}
} else {
ArrayPrototypePush(runArgs, path);
}
Expand Down Expand Up @@ -421,7 +425,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 };

// Acquire a worker ID from the pool for process isolation mode
let workerId;
Expand Down Expand Up @@ -648,11 +652,26 @@ function run(options = kEmptyObject) {
functionCoverage = 0,
execArgv = [],
argv = [],
cwd = process.cwd(),
cwd,
processExecArgv,
rerunFailuresFilePath,
Comment on lines 653 to 655
Copy link

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

processExecArgv is consumed by getRunArgs() as an array (it is filtered and iterated), but there is no validation for the options.processExecArgv type. If a public API caller passes a non-array value, this will fail at runtime in less obvious ways. Consider validating it (e.g., validateStringArray(processExecArgv, 'options.processExecArgv')) when it is explicitly provided.

Copilot uses AI. Check for mistakes.
env,
} = options;

// Default to process state when not explicitly provided, maintaining
// backwards compatibility for public API users while allowing the
// internal CLI entry point to pass these values explicitly.
const userProvidedEnv = env !== undefined;
if (cwd === undefined) {
cwd = process.cwd();
}
if (env === undefined) {
env = process.env;
}
if (processExecArgv === undefined) {
processExecArgv = process.execArgv;
}

if (files != null) {
validateArray(files, 'options.files');
}
Expand Down Expand Up @@ -759,7 +778,7 @@ function run(options = kEmptyObject) {
validatePath(globalSetupPath, 'options.globalSetupPath');
}

if (env != null) {
if (userProvidedEnv) {
validateObject(env);

if (isolation === 'none') {
Expand Down Expand Up @@ -830,13 +849,14 @@ function run(options = kEmptyObject) {
isolation,
argv,
execArgv,
processExecArgv,
rerunFailuresFilePath,
env,
workerIdPool: isolation === 'process' ? workerIdPool : null,
};

if (isolation === 'process') {
if (process.env.NODE_TEST_CONTEXT !== undefined) {
if (env.NODE_TEST_CONTEXT !== undefined) {
process.emitWarning('node:test run() is being called recursively within a test file. skipping running files.');
root.postRun();
return root.reporter;
Expand Down
Loading