-
-
Notifications
You must be signed in to change notification settings - Fork 5
doc(proposal): un/break --test
#13
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.
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
base: main
Are you sure you want to change the base?
Conversation
| node | ||
| --test ./src/foo/*.test.js | ||
| --test ./src/bar/*.test.js | ||
| ./src/not-pjson-main.js |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What would the purpose of this be? The test runner runs for the two glob patterns and then does what with this entry point script? Executes it as a test too or just as a regular node script?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, yes, true. Let's say there is no such thing as an entry-point for test mode (I think that doesn't exist currently either). If there's any "entry-point" when --test is present, throw.
I think in order to run an arbitrary script, it should be done with --import (and friends); otherwise, you get into inception.
node
--import ./test/env.setup.js
--import ./test/fix-snapshot-naming.js
--test ./src/foo/*.test.js
--test ./src/bar/*.test.js| ./src/not-pjson-main.js | ||
| ``` | ||
|
|
||
| An explicit entrypoint, like all node commands, must come last and must be a relative or absolute path (not a glob/pattern). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this tracks for test. Many users will only want to pass a single glob pattern; at least I know that I do! What would the explicit entry point do for a test script?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, okay, fair. So when watch mode is combined with test mode, shall we say that entry-point is not applicable—it's only applicable for watch mode separately?
|
How will this work across versions? Please don't underestimate the pain that inconsistencies in the test runner CLI across versions cause for users (ie glob support). It takes years for these pain points to go away. |
|
@cjihrig we discussed in today's meeting. I'm about to update this with the 3rd option that was born from that. I'll ping you for review in a few minutes. |
|
Discussed with @JakobJingleheimer and apparently it will not break existing users, so 👍 |
|
I think "option 3" will actually JustWork? node
--test ./foo.test.js
--test ./bar.test.jscurrently results in: [
'./foo.test.js',
'--test',
'./bar.test.js',
]
We can backport support for |
Ethan-Arrowood
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm still seeing some parts of this talking about main entrypoints with the test runner which just doesn't make sense. I'd prefer we clean that up before merging this as that may only cause more confusion in further discussions.
| `--watch` currently does 2 things: | ||
|
|
||
| * enables `watch` mode | ||
| * optionally accepts 1 value to override the entrypoint |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| * optionally accepts 1 value to override the entrypoint | |
| * optionally accepts 1 value to override the entrypoint | |
| * and the `--watch-path` option is used for any additional paths to include besides the entrypoint | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: those bullet-points are about --watch, so i think --watch-path should be separate
| * optionally accepts 1 value to override the entrypoint | |
| * optionally accepts 1 value to override the entrypoint | |
| `--watch-path` sets additional paths (can be set multiple times) to include as well as the entrypoint. |
| `test` would then work the same way: | ||
|
|
||
| ```sh title='reads "main" etc from package.json' | ||
| node --test | ||
| ``` | ||
| ```sh title='package.json "main" + an additional path' | ||
| node --test ./src/**/*.test.js | ||
| ``` | ||
| ```sh title='explicit entrypoint + an additional path' | ||
| node | ||
| --test ./src/foo/*.test.js | ||
| --test ./src/bar/*.test.js | ||
| ./src/not-pjson-main.js |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this section still needs to be updated, no? Test runs don't use main entrypoint or any entrypoint for that matter. It uses file patterns matching test files.
| --watch | ||
| ./src/not-pjson-main.js |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't believe an newline character would change the CLI parsing here. This is the same as specifying a path for --watch. Or am I mistaken?
Are you talking about the other options besides the one added yesterday? If so, my plan was to wait for people to weigh in; once one is selected, update the proposal to move the discard ones to a new "Rejected" section, and outdent the selected option. If you're talking about the small bit mentioning |
A follow-up to the discussion in the 26 Jan 2026 team meeting