test_runner: add initial CLI runner by cjihrig · Pull Request #42658 · nodejs/node

cjihrig · 2022-04-08T15:34:51Z

This commit introduces an initial version of a CLI-based test runner.

nodejs-github-bot · 2022-04-08T15:34:55Z

Review requested:

@nodejs/startup

aduh95 · 2022-04-08T15:49:26Z

//cc @nodejs/test_runner

ljharb

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.

richardlau · 2022-04-08T16:15:21Z

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?

As this PR currently is --test hasn't been added to the allow list for NODE_OPTIONS so setting it there would throw an error.

ljharb · 2022-04-08T16:19:48Z

@richardlau thanks, if that's explicitly going to never be allowed there then that does mitigate that one concern, but all the others remain.

cjihrig · 2022-04-08T16:22:52Z

How does --test affect --require, or loaders? What happens to the REPL with node --test and no other arguments?

node --test doesn't launch the REPL. I haven't tried a loader, but --require seems to work fine.

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?

--test in NODE_OPTIONS explicitly errors out.

jasnell · 2022-04-13T15:22:23Z

Just a question: why is this? I can definitely see wanting to run the inspector for debugging while running tests.

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.

Can we use chrome's debugger blackboxing stuff to "hide" the test runner code when inspecting?

Possibly - I'm not sure though.

nodejs-github-bot · 2022-04-15T17:37:30Z

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>

@kuriyosh

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

@kuriyosh

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

sosoba · 2022-05-04T07:08:55Z

Who will extend the @types/node package with a new node:test module?

Trott · 2022-05-04T14:26:34Z

Who will extend the @types/node package with a new node:test module?

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#42658 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

This commit introduces an initial version of a CLI-based test runner. PR-URL: #42658 Backport-PR-URL: #43904 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

This commit introduces an initial version of a CLI-based test runner. PR-URL: nodejs/node#42658 Backport-PR-URL: nodejs/node#43904 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Apr 8, 2022

aduh95 reviewed Apr 8, 2022

View reviewed changes

richardlau added the semver-minor PRs that contain new features and should be released in the next minor version. label Apr 8, 2022

ljharb reviewed Apr 8, 2022

View reviewed changes

cjihrig added dont-land-on-v12.x labels Apr 8, 2022

bnb reviewed Apr 8, 2022

View reviewed changes

Comment thread doc/api/test.md Outdated

targos reviewed Apr 9, 2022

View reviewed changes

Comment thread doc/api/test.md Outdated

cjihrig force-pushed the test-cli branch 2 times, most recently from f3263c3 to 805361e Compare April 9, 2022 18:32

aduh95 reviewed Apr 9, 2022

View reviewed changes

Comment thread doc/api/test.md Outdated

cjihrig force-pushed the test-cli branch from 805361e to 907e8ef Compare April 9, 2022 18:45

aduh95 reviewed Apr 9, 2022

View reviewed changes

Comment thread doc/api/test.md Outdated

cjihrig force-pushed the test-cli branch from 907e8ef to f8a34dc Compare April 9, 2022 22:38

ljharb reviewed Apr 10, 2022

View reviewed changes

Comment thread doc/api/test.md Outdated

Comment thread lib/internal/test_runner/utils.js Outdated

targos reviewed Apr 10, 2022

View reviewed changes

Comment thread doc/api/cli.md Outdated

cjihrig force-pushed the test-cli branch 2 times, most recently from be14d2f to a8c868e Compare April 10, 2022 23:00

targos mentioned this pull request Apr 12, 2022

2022-04-19, Version 18.0.0 (Current) #42262

Merged

jasnell reviewed Apr 13, 2022

View reviewed changes

Comment thread doc/api/test.md Outdated

jasnell reviewed Apr 13, 2022

View reviewed changes

aduh95 approved these changes Apr 13, 2022

View reviewed changes

nodejs-github-bot merged commit adaf602 into nodejs:master Apr 15, 2022

cjihrig deleted the test-cli branch April 15, 2022 17:50

xtx1130 pushed a commit to xtx1130/node that referenced this pull request Apr 25, 2022

test_runner: add initial CLI runner

05d037d

This commit introduces an initial version of a CLI-based test runner. PR-URL: nodejs#42658 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

targos pushed a commit that referenced this pull request Apr 28, 2022

test_runner: add initial CLI runner

78a860a

This commit introduces an initial version of a CLI-based test runner. PR-URL: #42658 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

targos mentioned this pull request May 2, 2022

v18.1.0 release proposal #42943

Merged

juanarbol mentioned this pull request May 30, 2022

test: use assert.match() instead of assert(regex.test()) #42803

Merged

tniessen added the test_runner Issues and PRs related to the test runner subsystem. label Jun 8, 2022

This was referenced Jul 18, 2022

util: add parseArgs module #42675

Merged

doc: add code examples to node test runner #43359

Closed

test_runner: add Subtest to tap protocol output #43417

Merged

targos pushed a commit to aduh95/node that referenced this pull request Jul 31, 2022

test_runner: add initial CLI runner

c2100cc

This commit introduces an initial version of a CLI-based test runner. PR-URL: nodejs#42658 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

targos pushed a commit that referenced this pull request Jul 31, 2022

test_runner: add initial CLI runner

cd6f24b

This commit introduces an initial version of a CLI-based test runner. PR-URL: #42658 Backport-PR-URL: #43904 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

targos added backported-to-v16.x and removed dont-land-on-v16.x labels Jul 31, 2022

targos mentioned this pull request Aug 2, 2022

v16.17.0 release proposal #44098

Merged

cola119 mentioned this pull request Oct 1, 2022

test: check --test is disallowed in NODE_OPTIONS #44846

Merged

Uh oh!

Uh oh!

Conversation

cjihrig commented Apr 8, 2022

Uh oh!

nodejs-github-bot commented Apr 8, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aduh95 commented Apr 8, 2022

Uh oh!

ljharb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

richardlau commented Apr 8, 2022

Uh oh!

ljharb commented Apr 8, 2022

Uh oh!

cjihrig commented Apr 8, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jasnell Apr 13, 2022

Choose a reason for hiding this comment

Uh oh!

cjihrig Apr 13, 2022

Choose a reason for hiding this comment

Uh oh!

ljharb Apr 13, 2022

Choose a reason for hiding this comment

Uh oh!

cjihrig Apr 14, 2022

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Apr 15, 2022

Uh oh!

sosoba commented May 4, 2022

Uh oh!

Trott commented May 4, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

13 participants