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
test_runner: add initial CLI runner #42658
Conversation
Review requested: |
//cc @nodejs/test_runner |
How does --test
affect --require
, or loaders? What happens to the REPL with node --test
and no other arguments?
If NODE_OPTIONS='--test'
is set, and the user doesn't control how node is invoked (via a shebang, for example), does this mean the user can never "undo" test mode?
This really feels to me like it should be an entirely distinct binary, rather than just a "mode" of the main node binary.
As this PR currently is |
@richardlau thanks, if that's explicitly going to never be allowed there then that does mitigate that one concern, but all the others remain. |
|
f3263c3
to
805361e
Compare
be14d2f
to
a8c868e
Compare
} | ||
|
||
if (debug_options_.inspector_enabled) { | ||
errors->push_back("the inspector cannot be used with --test"); |
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.
Just a question: why is this? I can definitely see wanting to run the inspector for debugging while running tests.
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.
A couple reasons:
- First, it's trivial to run an individual file with the inspector flags. At this point, the CLI runner is just spawning Node child processes with no special flags.
- Debugging through the CLI test runner is an awkward experience similar to the cluster module. Each test file will need a different debugger port. We would need to manage that logic as well as relay the correct information to users (I want to debug test X, I need to debug file Y, and connect to debug port Z). It also doesn't make sense to set something like
--inspect-brk
on a bunch of child processes. - I looked at a couple test frameworks, and they didn't seem to have a great story around propagating inspector flags to test files.
- If someone is passionate about this, it can be added in the future. It just didn't seem worth it to me for an initial version given the previous points. I think it would make sense in the future to have configuration options for execArgv and argv of the child processes.
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.
Can we use chrome's debugger blackboxing stuff to "hide" the test runner code when inspecting?
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.
Possibly - I'm not sure though.
Commit Queue failed- Loading data for nodejs/node/pull/42658 FetchError: Invalid response body while trying to fetch https://api.github.com/graphql: Premature close at consumeBody (file:///opt/hostedtoolcache/node/16.14.2/x64/lib/node_modules/node-core-utils/node_modules/node-fetch/src/body.js:234:60) at processTicksAndRejections (node:internal/process/task_queues:96:5) at async Response.text (file:///opt/hostedtoolcache/node/16.14.2/x64/lib/node_modules/node-core-utils/node_modules/node-fetch/src/body.js:158:18) at async Request.json (file:///opt/hostedtoolcache/node/16.14.2/x64/lib/node_modules/node-core-utils/lib/request.js:49:18) at async Request.query (file:///opt/hostedtoolcache/node/16.14.2/x64/lib/node_modules/node-core-utils/lib/request.js:107:20) at async Request.queryAll (file:///opt/hostedtoolcache/node/16.14.2/x64/lib/node_modules/node-core-utils/lib/request.js:134:20) at async Request.gql (file:///opt/hostedtoolcache/node/16.14.2/x64/lib/node_modules/node-core-utils/lib/request.js:64:22) at async PRData.getComments (file:///opt/hostedtoolcache/node/16.14.2/x64/lib/node_modules/node-core-utils/lib/pr_data.js:97:21) at async Promise.all (index 2) at async Promise.all (index 1) { type: 'system', errno: 'ERR_STREAM_PREMATURE_CLOSE', code: 'ERR_STREAM_PREMATURE_CLOSE', erroredSysCall: undefined }https://github.com/nodejs/node/actions/runs/2173428186 |
Landed in adaf602 |
This commit introduces an initial version of a CLI-based test runner. PR-URL: nodejs#42658 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
This commit introduces an initial version of a CLI-based test runner. PR-URL: #42658 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Notable changes: doc: * add @kuriyosh to collaborators (Yoshiki Kurihara) #42824 lib,src: * (SEMVER-MINOR) implement WebAssembly Web API (Tobias Nießen) #42701 test_runner: * (SEMVER-MINOR) add initial CLI runner (Colin Ihrig) #42658 worker: * (SEMVER-MINOR) add hasRef() to MessagePort (Darshan Sen) #42849 PR-URL: #42943
Notable changes: doc: * add @kuriyosh to collaborators (Yoshiki Kurihara) #42824 lib,src: * (SEMVER-MINOR) implement WebAssembly Web API (Tobias Nießen) #42701 test_runner: * (SEMVER-MINOR) add initial CLI runner (Colin Ihrig) #42658 worker: * (SEMVER-MINOR) add hasRef() to MessagePort (Darshan Sen) #42849 PR-URL: #42943
Who will extend the |
As always, the maintainers of https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/node. |
This commit introduces an initial version of a CLI-based test runner. PR-URL: nodejs/node#42658 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Notable changes: doc: * add @kuriyosh to collaborators (Yoshiki Kurihara) nodejs/node#42824 lib,src: * (SEMVER-MINOR) implement WebAssembly Web API (Tobias Nießen) nodejs/node#42701 test_runner: * (SEMVER-MINOR) add initial CLI runner (Colin Ihrig) nodejs/node#42658 worker: * (SEMVER-MINOR) add hasRef() to MessagePort (Darshan Sen) nodejs/node#42849 PR-URL: nodejs/node#42943
This commit introduces an initial version of a CLI-based test runner.