Skip to content

Commit acf2faa

Browse files
committed
test_runner: address review feedback
- Remove processExecArgv option: process.execArgv is runtime state that should not be a public API option. Keep reading it directly in getRunArgs() as before. - Fix env with isolation=none: only set options.env from CLI entry point when isolation is not 'none', preventing ERR_INVALID_ARG_VALUE when using --test-isolation=none. - Add documentation for the env option in doc/api/test.md. - Add tests for env option acceptance and env+isolation=none rejection.
1 parent e1746a9 commit acf2faa

4 files changed

Lines changed: 28 additions & 9 deletions

File tree

doc/api/test.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1546,6 +1546,9 @@ changes:
15461546
* `setup` {Function} A function that accepts the `TestsStream` instance
15471547
and can be used to setup listeners before any tests are run.
15481548
**Default:** `undefined`.
1549+
* `env` {Object} Environment variables to pass to test child processes.
1550+
This option has no effect when `isolation` is `'none'`.
1551+
**Default:** `process.env`.
15491552
* `execArgv` {Array} An array of CLI flags to pass to the `node` executable when
15501553
spawning the subprocesses. This option has no effect when `isolation` is `'none`'.
15511554
**Default:** `[]`

lib/internal/main/test_runner.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,9 @@ if (isUsingInspector() && options.isolation === 'process') {
3333
// Capture process state explicitly so run() does not need to access it.
3434
options.globPatterns = ArrayPrototypeSlice(process.argv, 1);
3535
options.cwd = process.cwd();
36-
options.env = process.env;
37-
options.processExecArgv = process.execArgv;
36+
if (options.isolation !== 'none') {
37+
options.env = process.env;
38+
}
3839

3940
debug('test runner configuration:', options);
4041
run(options).on('test:summary', (data) => {

lib/internal/test_runner/runner.js

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,6 @@ function getRunArgs(path, { forceExit,
167167
only,
168168
argv: suppliedArgs,
169169
execArgv,
170-
processExecArgv,
171170
globPatterns,
172171
rerunFailuresFilePath,
173172
root: { timeout },
@@ -185,7 +184,7 @@ function getRunArgs(path, { forceExit,
185184
*/
186185
const nodeOptionsSet = new SafeSet(processNodeOptions);
187186
const unknownProcessExecArgv = ArrayPrototypeFilter(
188-
processExecArgv,
187+
process.execArgv,
189188
(arg) => !nodeOptionsSet.has(arg),
190189
);
191190
ArrayPrototypePushApply(runArgs, unknownProcessExecArgv);
@@ -653,7 +652,6 @@ function run(options = kEmptyObject) {
653652
execArgv = [],
654653
argv = [],
655654
cwd,
656-
processExecArgv,
657655
rerunFailuresFilePath,
658656
env,
659657
} = options;
@@ -668,9 +666,6 @@ function run(options = kEmptyObject) {
668666
if (env === undefined) {
669667
env = process.env;
670668
}
671-
if (processExecArgv === undefined) {
672-
processExecArgv = process.execArgv;
673-
}
674669

675670
if (files != null) {
676671
validateArray(files, 'options.files');
@@ -849,7 +844,6 @@ function run(options = kEmptyObject) {
849844
isolation,
850845
argv,
851846
execArgv,
852-
processExecArgv,
853847
rerunFailuresFilePath,
854848
env,
855849
workerIdPool: isolation === 'process' ? workerIdPool : null,

test/parallel/test-runner-run.mjs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -687,6 +687,27 @@ describe('forceExit', () => {
687687
message: /The property 'options\.forceExit' is not supported with watch mode\./
688688
});
689689
});
690+
691+
it('should accept env option and pass it to child processes', async () => {
692+
const stream = run({
693+
files: [join(testFixtures, 'default-behavior/test/random.cjs')],
694+
env: { ...process.env, NODE_TEST_CUSTOM_ENV_VAR: '1' },
695+
});
696+
stream.on('test:fail', common.mustNotCall());
697+
stream.on('test:pass', common.mustCall(1));
698+
// eslint-disable-next-line no-unused-vars
699+
for await (const _ of stream);
700+
});
701+
702+
it('should reject env option with isolation none', () => {
703+
assert.throws(() => run({
704+
files: [join(testFixtures, 'default-behavior/test/random.cjs')],
705+
isolation: 'none',
706+
env: { FOO: 'bar' },
707+
}), {
708+
code: 'ERR_INVALID_ARG_VALUE',
709+
});
710+
});
690711
});
691712

692713

0 commit comments

Comments
 (0)