Skip to content

Commit 615f2c8

Browse files
committed
test_runner: wip - replace execArgv with getOptionsAsFlagsFromBinding
1 parent 22466e7 commit 615f2c8

File tree

6 files changed

+181
-14
lines changed

6 files changed

+181
-14
lines changed

lib/internal/test_runner/runner.js

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ const { spawn } = require('child_process');
3535
const { finished } = require('internal/streams/end-of-stream');
3636
const { resolve, sep, isAbsolute } = require('path');
3737
const { DefaultDeserializer, DefaultSerializer } = require('v8');
38-
const { getOptionValue } = require('internal/options');
38+
const { getOptionValue, getOptionsAsFlagsFromBinding } = require('internal/options');
3939
const { Interface } = require('internal/readline/interface');
4040
const { deserializeError } = require('internal/error_serdes');
4141
const { Buffer } = require('buffer');
@@ -150,38 +150,41 @@ function getRunArgs(path, { forceExit,
150150
execArgv,
151151
root: { timeout },
152152
cwd }) {
153-
const argv = ArrayPrototypeFilter(process.execArgv, filterExecArgv);
153+
const processNodeOptions = getOptionsAsFlagsFromBinding();
154+
const runArgs = ArrayPrototypeFilter(processNodeOptions, filterExecArgv);
154155
if (forceExit === true) {
155-
ArrayPrototypePush(argv, '--test-force-exit');
156+
ArrayPrototypePush(runArgs, '--test-force-exit');
156157
}
157158
if (isUsingInspector()) {
158-
ArrayPrototypePush(argv, `--inspect-port=${getInspectPort(inspectPort)}`);
159+
ArrayPrototypePush(runArgs, `--inspect-port=${getInspectPort(inspectPort)}`);
159160
}
160161
if (testNamePatterns != null) {
161-
ArrayPrototypeForEach(testNamePatterns, (pattern) => ArrayPrototypePush(argv, `--test-name-pattern=${pattern}`));
162+
ArrayPrototypeForEach(testNamePatterns, (pattern) => ArrayPrototypePush(runArgs, `--test-name-pattern=${pattern}`));
162163
}
163164
if (testSkipPatterns != null) {
164-
ArrayPrototypeForEach(testSkipPatterns, (pattern) => ArrayPrototypePush(argv, `--test-skip-pattern=${pattern}`));
165+
ArrayPrototypeForEach(testSkipPatterns, (pattern) => ArrayPrototypePush(runArgs, `--test-skip-pattern=${pattern}`));
165166
}
166167
if (only === true) {
167-
ArrayPrototypePush(argv, '--test-only');
168+
ArrayPrototypePush(runArgs, '--test-only');
168169
}
169170
if (timeout != null) {
170-
ArrayPrototypePush(argv, `--test-timeout=${timeout}`);
171+
ArrayPrototypePush(runArgs, `--test-timeout=${timeout}`);
171172
}
172173

173-
ArrayPrototypePushApply(argv, execArgv);
174+
ArrayPrototypePushApply(runArgs, execArgv);
174175

175176
if (path === kIsolatedProcessName) {
176-
ArrayPrototypePush(argv, '--test');
177-
ArrayPrototypePushApply(argv, ArrayPrototypeSlice(process.argv, 1));
177+
ArrayPrototypePush(runArgs, '--test');
178+
ArrayPrototypePushApply(runArgs, ArrayPrototypeSlice(process.argv, 1));
178179
} else {
179-
ArrayPrototypePush(argv, path);
180+
ArrayPrototypePush(runArgs, path);
180181
}
181182

182-
ArrayPrototypePushApply(argv, suppliedArgs);
183+
ArrayPrototypePushApply(runArgs, suppliedArgs);
183184

184-
return argv;
185+
// TODO(pmarchini): we should be able to filter also argv provided by the user removing nodejs flags
186+
187+
return runArgs;
185188
}
186189

187190
const serializer = new DefaultSerializer();

test/fixtures/test-runner/flag-propagation/.env

Whitespace-only changes.
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"nodeOptions": {
3+
"max-http-header-size": 10
4+
}
5+
}
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
import { run } from 'node:test';
2+
import { tap } from 'node:test/reporters';
3+
import { parseArgs } from 'node:util';
4+
5+
// Parse arguments to get flag name, expected value, and propagation expectation
6+
const options = {
7+
flag: {
8+
type: 'string',
9+
default: '',
10+
},
11+
expected: {
12+
type: 'string',
13+
default: '',
14+
},
15+
shouldPropagate: {
16+
type: 'boolean',
17+
default: false,
18+
},
19+
description: {
20+
type: 'string',
21+
default: 'flag propagation test',
22+
},
23+
};
24+
25+
const { values } = parseArgs({ args: process.argv.slice(2), options });
26+
27+
const argv = [
28+
`--flag=${values.flag}`,
29+
`--expected=${values.expected}`,
30+
`--description="${values.description}"`,
31+
].filter(Boolean);
32+
33+
run({
34+
files: ['./test.mjs'],
35+
cwd: process.cwd(),
36+
argv,
37+
isolation: 'process',
38+
}).compose(tap).pipe(process.stdout);
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
import { test } from 'node:test';
2+
import { strictEqual } from 'node:assert';
3+
import internal from 'internal/options';
4+
import { parseArgs } from 'node:util';
5+
6+
// Parse arguments to get flag name, expected value, and propagation expectation
7+
const options = {
8+
flag: {
9+
type: 'string',
10+
default: '',
11+
},
12+
expected: {
13+
type: 'string',
14+
default: '',
15+
},
16+
description: {
17+
type: 'string',
18+
default: 'flag propagation test',
19+
},
20+
};
21+
22+
23+
// console.error('Running flag propagation test with args:', process.argv.slice(2));
24+
25+
const { values } = parseArgs({ args: process.argv.slice(2), options });
26+
27+
const { flag, expected, description } = values;
28+
29+
// Create a test that checks the flag propagation
30+
test(description, () => {
31+
const optionValue = internal.getOptionValue(flag);
32+
console.error(`testing flag: ${flag}, found value: ${optionValue}, expected: ${expected}`);
33+
34+
const isNumber = !isNaN(Number(expected));
35+
const booleanValue = expected === 'true' || expected === 'false';
36+
if (booleanValue) {
37+
strictEqual(optionValue, expected === 'true');
38+
return;
39+
} else if (isNumber) {
40+
strictEqual(Number(optionValue), Number(expected));
41+
} else{
42+
strictEqual(optionValue, expected);
43+
}
44+
});
Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
'use strict';
2+
3+
require('../common');
4+
const fixtures = require('../common/fixtures.js');
5+
const assert = require('assert');
6+
const { spawnSync } = require('child_process');
7+
const { describe, it } = require('node:test');
8+
const path = require('path');
9+
10+
// Test flag propagation to child test processes
11+
// This validates that certain flags are/aren't propagated to child test processes
12+
// based on the specification in the Node.js documentation
13+
describe('test runner flag propagation', () => {
14+
const flagPropagationTests = [
15+
['--experimental-config-file', 'node.config.json', ''],
16+
['--experimental-default-config-file', '', false],
17+
['--env-file', '.env', '.env'],
18+
['--env-file-if-exists', '.env', '.env'],
19+
['--test-concurrency', '2', '2'],
20+
// ['--test-force-exit', '', true], // <-- this test fails as is forces exit
21+
// ['--test-only', '', true], // <-- this needs to be investigated
22+
['--test-timeout', '5000', '5000'],
23+
['--test-coverage-branches', '100', '100'],
24+
['--test-coverage-functions', '100', '100'],
25+
['--test-coverage-lines', '100', '100'],
26+
// ['--test-coverage-exclude', 'test/**', 'test/**'],
27+
// ['--test-coverage-include', 'src/**', 'src/**'],
28+
['--test-update-snapshots', '', true],
29+
];
30+
31+
// Path to the static fixture
32+
const fixtureDir = fixtures.path('test-runner', 'flag-propagation');
33+
const runner = path.join(fixtureDir, 'runner.mjs');
34+
35+
for (const [flagName, testValue, expectedValue] of flagPropagationTests) {
36+
const testDescription = `should propagate ${flagName} to child tests as expected`;
37+
38+
it(testDescription, () => {
39+
const args = [
40+
'--test-reporter=tap',
41+
'--no-warnings',
42+
'--expose-internals',
43+
testValue ? `${flagName}=${testValue}` : flagName,
44+
// Use the runner fixture
45+
runner,
46+
// Pass parameters to the fixture
47+
`--flag=${flagName}`,
48+
`--expected=${expectedValue}`,
49+
`--description="${testDescription}"`,
50+
].filter(Boolean);
51+
52+
const child = spawnSync(
53+
process.execPath,
54+
args,
55+
{
56+
cwd: fixtureDir,
57+
},
58+
);
59+
const stdout = child.stdout.toString();
60+
const stderr = child.stderr.toString();
61+
62+
console.error(`Running test with args: ${args.join(' ')}`);
63+
console.error(`stdout: ${stdout}`);
64+
console.error(`stderr: ${stderr}`);
65+
66+
// Verify the test executed and passed
67+
assert.strictEqual(child.status, 0, `Flag propagation test failed for ${flagName}.`);
68+
assert.match(stdout, /tests 1/, `Test should execute for ${flagName}`);
69+
assert.match(stdout, /pass 1/, `Test should pass for ${flagName} propagation check`);
70+
});
71+
}
72+
});
73+
74+
// TODO: at the moment we can pass an argv down to the test children
75+
// this behaviour must be preserved for compatibility
76+
// add a test to ensure the non-regression of this behaviour
77+
// We could still pass downstream every argument that is not a flag

0 commit comments

Comments
 (0)