test_runner: preserve user flags in argv#61858
Conversation
This commit fixes an issue where user-specified flags were being stripped from process.argv when running with the --test flag. It ensures that arguments following '--' or starting with '-' (that are not consumed by Node.js options) are correctly passed to the test process. Fixes: nodejs#61852
|
Review requested:
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #61858 +/- ##
==========================================
+ Coverage 89.72% 89.73% +0.01%
==========================================
Files 675 675
Lines 204806 204826 +20
Branches 39355 39358 +3
==========================================
+ Hits 183761 183803 +42
+ Misses 13330 13295 -35
- Partials 7715 7728 +13
🚀 New features to boost your workflow:
|
| 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); | ||
| } | ||
| } |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?
3 participants
Fixes: #61852
This PR fixes an issue where user-specified flags were being stripped from
process.argvwhen running with the--testflag.Previously, the test runner treated all arguments as potential file patterns, ignoring flags that weren't specific to the test runner. This made it impossible to pass custom flags (e.g.,
--hello) to test files.This change ensures that:
--are correctly treated as user arguments.-(that are not consumed by Node.js or the test runner) are preserved and passed to the test process.Added new tests in
test/parallel/test-runner-cli-args.jsto verify:node --test fixture.js --hellonode --test fixture.js -- --hellonode --test fixture.js --hello -- world