[v16.x backport] `node:test` by aduh95 · Pull Request #43904 · nodejs/node · GitHub

No description provided.

Review requested:

@nodejs/modules
@nodejs/startup

This seems very risky; node typically does not backport new core modules.

This seems very risky; node typically does not backport new core modules.

@ljharb Could you elaborate on why it is risky? Thanks.

@F3n67u new core modules are always risky. In the typical case (every single one but node:test) it's adding a new non-prefixed core module, that could break someone who's requiring a folder inside a node_modules or a NODE_PREFIX folder.

In this case, the node: prefix does mean that nobody in node 16 could be relying on it, so it's less risky, but userland tooling doesn't have a way to discover this module exists (#42785), and userland tools like resolve/is-core-module don't assume that anything with a node: prefix is a valid core module, so they'd need to be updated - both directly and transitively.

@nodejs/lts what do you think?

This doesn’t strike me as all that risky; what could break in userland if there’s suddenly an additional prefixed core module? What relies on the list of builtins, and more precisely what would break if that list grew by one?

This feels like a semver-minor change; a new feature has been added. It’s not that different from a new API being added on any of the core modules. Sure, someone might have been depending somehow on the list of APIs on path being exactly 22 things and now there are 23, but that’s the definition of a semver-minor change.

If we don’t backport node:test, there will be lots of other PRs blocked from backporting, as many PRs since node:test landed have started to use that API in Node’s own tests. We could do some kind of special PR that makes node:test available internally but not publicly on 16, to allow these other PRs to be backported, but then we have a weird divergence to deal with in 16. Also node:test is generally useful and we should want our v16 users to enjoy it.

What relies on the list of builtins, and more precisely what would break if that list grew by one?

To clarify here, it would break if a userland module was using the same name as the core built-in one. But that doesn't apply here because node:test is only available when using the node: prefix, and no user package can use a name that starts with node: since Node.js 16.0.0 – specifically for this use case.

Also node:test is generally useful and we should want our v16 users to enjoy it.

FWIW they can already, via npm i test :)

CI: https://ci.nodejs.org/job/node-test-pull-request/45699/

CITGM: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/2972/

It could be backported without being exposed such that it has no risk and also unblocks additional backport PRs.

if this lands, will we need to remove https://github.com/nodejs/node/labels/dont-land-on-v16.x from all/some of these?
https://github.com/nodejs/node/issues?q=label%3Atest_runner+label%3Adont-land-on-v16.x+

@MoLow yes, but with v16.x instead of v14.x

CI: https://ci.nodejs.org/job/node-test-pull-request/45703/

CI: https://ci.nodejs.org/job/node-test-pull-request/45740/

CI: https://ci.nodejs.org/job/node-test-pull-request/45743/

CI: https://ci.nodejs.org/job/node-test-pull-request/45744/

CI: https://ci.nodejs.org/job/node-test-pull-request/45746/

CI: https://ci.nodejs.org/job/node-test-pull-request/45748/

CI: https://ci.nodejs.org/job/node-test-pull-request/45749/

I think CI was resumed enough times to confirm that the failing test is broken, not flaky.
It's probably unrelated to this PR so I'm starting a local bisect with the debug build.

This commit adds a new 'test' module that exposes an API for creating JavaScript tests. As the tests execute, TAP output is written to standard output. This commit only supports executing individual test files, and does not implement command line functionality for a full test runner. PR-URL: nodejs#42325 Refs: nodejs#40954 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

This commit makes it possible to add new core modules that can only be require()'ed and imported when the 'node:' scheme is used. The 'test' module is the first such module. These 'node:'-only modules are not included in the list returned by module.builtinModules. PR-URL: nodejs#42325 Refs: nodejs#40954 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

This commit introduces a CLI flag and test runner functionality to support running a subset of tests that are indicated by an 'only' option passed to the test. PR-URL: nodejs#42514 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

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

I removed #41818 from the staging branch and rebased this PR.

CI: https://ci.nodejs.org/job/node-test-pull-request/45750/

CI: https://ci.nodejs.org/job/node-test-pull-request/45752/ ✔️

This commit adds a new 'test' module that exposes an API for creating JavaScript tests. As the tests execute, TAP output is written to standard output. This commit only supports executing individual test files, and does not implement command line functionality for a full test runner. PR-URL: #42325 Backport-PR-URL: #43904 Refs: #40954 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

This commit makes it possible to add new core modules that can only be require()'ed and imported when the 'node:' scheme is used. The 'test' module is the first such module. These 'node:'-only modules are not included in the list returned by module.builtinModules. PR-URL: #42325 Backport-PR-URL: #43904 Refs: #40954 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

This commit introduces a CLI flag and test runner functionality to support running a subset of tests that are indicated by an 'only' option passed to the test. PR-URL: #42514 Backport-PR-URL: #43904 Reviewed-By: James M Snell <jasnell@gmail.com> 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>

Landed in 92051cb...cd6f24b

This commit adds a new 'test' module that exposes an API for creating JavaScript tests. As the tests execute, TAP output is written to standard output. This commit only supports executing individual test files, and does not implement command line functionality for a full test runner. PR-URL: nodejs/node#42325 Backport-PR-URL: nodejs/node#43904 Refs: nodejs/node#40954 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

This commit makes it possible to add new core modules that can only be require()'ed and imported when the 'node:' scheme is used. The 'test' module is the first such module. These 'node:'-only modules are not included in the list returned by module.builtinModules. PR-URL: nodejs/node#42325 Backport-PR-URL: nodejs/node#43904 Refs: nodejs/node#40954 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

This commit introduces a CLI flag and test runner functionality to support running a subset of tests that are indicated by an 'only' option passed to the test. PR-URL: nodejs/node#42514 Backport-PR-URL: nodejs/node#43904 Reviewed-By: James M Snell <jasnell@gmail.com> 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>

aduh95 added semver-minor PRs that contain new features and should be released in the next minor version. needs-citgm PRs that need a CITGM CI run. test_runner Issues and PRs related to the test runner subsystem. labels Jul 19, 2022

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. v16.x labels Jul 19, 2022

aduh95 requested a review from targos July 19, 2022 16:41

GeoffreyBooth approved these changes Jul 30, 2022

View reviewed changes

GeoffreyBooth added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Jul 30, 2022

juanarbol approved these changes Jul 31, 2022

View reviewed changes

targos force-pushed the v16.x-staging branch from eff0354 to 92051cb Compare July 31, 2022 07:45

cjihrig added 4 commits July 31, 2022 09:47

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 force-pushed the backport-node-test branch from e1af15c to c2100cc Compare July 31, 2022 07:48

targos approved these changes Jul 31, 2022

View reviewed changes

targos self-assigned this Jul 31, 2022

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 closed this Jul 31, 2022

GeoffreyBooth mentioned this pull request Jul 31, 2022

esm: improve performance & tidy tests #43784

Merged

aduh95 deleted the backport-node-test branch July 31, 2022 14:36

This was referenced Aug 1, 2022

CI Reliability 2022-08-01 nodejs/reliability#334

Open

CI Reliability 2022-08-02 nodejs/reliability#335

Open

CI Reliability 2022-08-03 nodejs/reliability#336

Open

CI Reliability 2022-08-05 nodejs/reliability#337

Open

[v16.x backport] `node:test` by aduh95 · Pull Request #43904 · nodejs/node · GitHub

Uh oh!

Uh oh!

[v16.x backport] node:test - #43904

[v16.x backport] node:test#43904aduh95 wants to merge 4 commits intonodejs:v16.x-stagingnodejs/node:v16.x-stagingfrom aduh95:backport-node-testaduh95/node:backport-node-testCopy head branch name to clipboard

Conversation

aduh95 commented Jul 19, 2022

Uh oh!

nodejs-github-bot commented Jul 19, 2022

Uh oh!

ljharb commented Jul 19, 2022

Uh oh!

F3n67u commented Jul 20, 2022

Uh oh!

ljharb commented Jul 20, 2022

Uh oh!

targos commented Jul 26, 2022

Uh oh!

GeoffreyBooth commented Jul 26, 2022

Uh oh!

aduh95 commented Jul 26, 2022

Uh oh!

nodejs-github-bot commented Jul 27, 2022

Uh oh!

targos commented Jul 27, 2022

Uh oh!

ljharb commented Jul 27, 2022

Uh oh!

MoLow commented Jul 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

targos commented Jul 27, 2022

Uh oh!

nodejs-github-bot commented Jul 27, 2022

Uh oh!

nodejs-github-bot commented Jul 30, 2022

Uh oh!

nodejs-github-bot commented Jul 30, 2022

Uh oh!

nodejs-github-bot commented Jul 31, 2022

Uh oh!

nodejs-github-bot commented Jul 31, 2022

Uh oh!

nodejs-github-bot commented Jul 31, 2022

Uh oh!

nodejs-github-bot commented Jul 31, 2022

Uh oh!

targos commented Jul 31, 2022

Uh oh!

targos commented Jul 31, 2022

Uh oh!

nodejs-github-bot commented Jul 31, 2022

Uh oh!

nodejs-github-bot commented Jul 31, 2022 • edited by targos Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

targos commented Jul 31, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

[v16.x backport] node:test#43904
aduh95 wants to merge 4 commits into
nodejs:v16.x-stagingfrom
aduh95:backport-node-test

MoLow commented Jul 27, 2022 •
edited
Loading

nodejs-github-bot commented Jul 31, 2022 •
edited by targos
Loading