Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
24 changes: 22 additions & 2 deletions lib/internal/main/test_runner.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
'use strict';

const {
ArrayPrototypeSlice,
ArrayPrototypePush,
StringPrototypeStartsWith,
} = primordials;

const {
Expand Down Expand Up @@ -30,7 +31,26 @@ if (isUsingInspector() && options.isolation === 'process') {
options.inspectPort = process.debugPort;
}

options.globPatterns = ArrayPrototypeSlice(process.argv, 1);
const userArgs = [];
const globPatterns = [];
let isArg = false;

for (let i = 1; i < process.argv.length; i++) {
const arg = process.argv[i];
if (isArg) {
ArrayPrototypePush(userArgs, arg);
} else if (arg === '--') {
isArg = true;
ArrayPrototypePush(userArgs, arg);
} else if (StringPrototypeStartsWith(arg, '-')) {
ArrayPrototypePush(userArgs, arg);
} else {
ArrayPrototypePush(globPatterns, arg);
}
}
Comment on lines +34 to +50
Copy link
Member

Choose a reason for hiding this comment

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

I'm concerned about the possible edge cases like node --test a-file.js --mode ci b.file.js .
What do you think about it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That’s a very valid concern, @pmarchini. In your example, the current logic would indeed incorrectly classify ci as a glob pattern.

To resolve this for built-in flags, I can leverage getCLIOptionsInfo() from internal/options to determine if a flag is value-taking or boolean. This allows the parser to correctly group values into userArgs instead of globPatterns for all Node.js and Test Runner flags, without needing a hardcoded whitelist.

For unknown user flags like --mode, how do you think we should handle the ambiguity? Should we assume they are booleans unless the user uses the --mode=ci or -- syntax, or is there a preferred heuristic in Node.js core for this?


options.globPatterns = globPatterns;
options.argv = userArgs;

debug('test runner configuration:', options);
run(options).on('test:summary', (data) => {
Expand Down
1 change: 1 addition & 0 deletions test/fixtures/test-runner/argv.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
console.log(JSON.stringify(process.argv));
47 changes: 47 additions & 0 deletions test/parallel/test-runner-cli-args.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
'use strict';
require('../common');
const assert = require('assert');
const { spawnSync } = require('child_process');
const fixtures = require('../common/fixtures');

const testFixture = fixtures.path('test-runner/argv.js');

{
const args = ['--test', testFixture, '--hello'];
const child = spawnSync(process.execPath, args);

assert.strictEqual(child.stderr.toString(), '');
const stdout = child.stdout.toString();
assert.match(stdout, /"--hello"\]/);
assert.strictEqual(child.status, 0);
}

{
const args = ['--test', testFixture, '--', '--hello', '--world'];
const child = spawnSync(process.execPath, args);

assert.strictEqual(child.stderr.toString(), '');
const stdout = child.stdout.toString();
assert.match(stdout, /"--","--hello","--world"\]/);
assert.strictEqual(child.status, 0);
}

{
const args = ['--test', testFixture, '--', 'foo.js'];
const child = spawnSync(process.execPath, args);

assert.strictEqual(child.stderr.toString(), '');
const stdout = child.stdout.toString();
assert.match(stdout, /"--","foo\.js"\]/);
assert.strictEqual(child.status, 0);
}

{
const args = ['--test', testFixture, '--hello', '--', 'world'];
const child = spawnSync(process.execPath, args);

assert.strictEqual(child.stderr.toString(), '');
const stdout = child.stdout.toString();
assert.match(stdout, /"--hello","--","world"\]/);
assert.strictEqual(child.status, 0);
}
Loading