Repository navigation
Request: mark test runner stable in Node 20.0.0 #46642
Description
Activity
- addedfeature requestIssues requesting new Node.js features.Issues requesting new Node.js features.
on Feb 13, 2023 +1
the majority of the test runner is ready
I haven't been tracking closely but I get the impression people open feature requests for it quite a bit. How would moving it out of experimental interact with those?
Feature requests would have to follow semver. Of course we aren't obligated to implement all of the feature requests too - there are a few open feature requests that I personally don't think we should implement.
cc @nodejs/test_runner
I think this should be fine. We could just create experimental flags or warnings for any parts that are remaining experimental or that we think might get breaking changes. So really that’s my only question: are there any parts of the test runner, besides code coverage, that might get breaking changes? Reporters? How about we isolate those under experimental flags so that it’s clear to users when they’re deviating from the stable path, and call the rest stable.
Is there a way we could measure "ready" beyond just "i feel"?
It'd be great to try to collect feedback about who has migrated to node's test runner, and who hasn't and why not.
Reacted by Julian GruberI think the test api and runner are stable. The reporters and codecov are too new to be stable, however they have a very little (if at all) API surface.
I’d recommend we keep the codecov and reporters as doc-experimental until 20.0.0 reach LTS but otherwise drop all flags and warnings.
From a marketing perspective, announcing v20 with test runner is a great announcement.
Reacted by Denis Malinochkin, Avindra Goolcharan, root, Marvin Hagemeister, Abdellah, Richie McColl, Julian Gruber, Marco Ippolito, Wassim Chegham, Moshe Atlow and 4 more- addedtest_runnerIssues and PRs related to the test runner subsystem.Issues and PRs related to the test runner subsystem.
on Feb 15, 2023 I'd like to bring up #44023 again. I haven't cared about this topic much since opening that issue due to being busy with other things, but I consider that problem largely unsolved. The PR that closed that issue (#44241) is nice for sure but doesn't address command-line usage, and so IMHO doesn't really improve the TypeScript DX that much. Stabilizing could prevent such improvements from happening since they'd possibly be breaking changes. Please reconsider adding support for a string argument to
--test, or some other way of non-programmatically specifying the test file pattern, before marking as stable.To recap: The test runner has hardcoded globs for the test files. It doesn't support running test files written in TypeScript or any other language, or otherwise filtering the test files, without (1) listing all of them explicitly, or (2) writing a custom entry point. (1) is near impossible to maintain, and (2) requires significant amounts of boilerplate code and/or third-party packages, which defeats the point of a native test runner.
Reacted by Joshua J.Reacted by Joshua J.@meyfa could you open a new issue with the things that are missing? Note that marking it stable doesn't mean we cannot add things, but rather that we won't break users.
there is still an open issue on the subject #46292.
Although changing the behavior of--testcan be a breaking change, I think that adding glob support into core (which I am very much in favor of) will probably take time and should not delay the test runner itselfReacted by Fabian Meyer and Joshua J.- addedtsc-agendaIssues and PRs to discuss during Technical Steering Committee meetings.Issues and PRs to discuss during Technical Steering Committee meetings.
on Feb 17, 2023 Marking tsc-agenda for visibility.
Reacted by Moshe AtlowAlthough changing the behavior of
--testcan be a breaking change, I think that adding glob support into core (which I am very much in favor of) will probably take time and should not delay the test runner itselfWould adding glob support be a breaking change in some way? Other than “before it errored and now it doesn’t.”
As far as I’m concerned we should identify any parts that might still get breaking changes and wall those off as experimental, and mark the remainder as stable. I also think we should add warnings or experimental flags for the parts that might still change, because with the bulk of the test runner being stable it shouldn’t fall on users to pore through the docs to ensure that they’re only using the stable parts of the runner. I would imagine that there would likely be many users who only want to use the stable parts, waiting for the experimental parts to become stable, because they don’t want to deal with updating their testing code often as the experimental parts change.
Would adding glob support be a breaking change in some way? Other than “before it errored and now it doesn’t.”
It would be a breaking change if we use
*to mean "all the files" when before it meant a specific file. E.g.:$ rm -rf repro && mkdir repro && cd repro $ mkdir "**" $ echo 'console.log("pass")' > "**/*.js" $ echo 'throw new Error' > "**/should-fail.js" $ node --test **/*.js TAP version 13 # Subtest: …/repro/**/*.js # pass ok 1 - …/repro/**/*.js --- duration_ms: 112.994764 ... 1..1 # tests 1 # pass 1 # fail 0 # cancelled 0 # skipped 0 # todo 0 # duration_ms 115.977816 $ cd .. && rm -r repro
If we change the meaning of
**/*.jsto say "all the .js files in all the subdirectories", it would now execute**/should-fail.jsand the test suite would start failing.Reacted by Julian GruberReacted by Benjamin GruenbaumIf we change the meaning of
**/*.jsto say “all the .js files in all the subdirectories”, it would now execute**/should-fail.jsand the test suite would start failing.It’s a bit ridiculous to consider a file named
**/*.jsbut yes, I suppose that’s correct. Could we simply error on files with asterisks in their names? For now, until globbing is added. Then globbing could be added without being a breaking change. (I assume we only need to consider*?)Reacted by Julian Gruber, Moshe Atlow, Benjamin Gruenbaum, Fabian Meyer, Steven and Joshua J.Reacted by Matteo Collina and Joshua J.One thing to note is that changing the files to search would likely involve a semver minor change to add a new CLI flag, and not a breaking change to
--test. Since its inception,--testhas only been a boolean flag, and does not take any string values.I think the best path forward is to list all the concerns / missing features as issues and track them accordingly, so we can stabilize it in a short timeframe.
Somewhere, the issue of our increasingly bloated CLI was raised. The test mode and the watch mode alone have added about ten CLI options or so to the CLI.
node --helpprints more than 300 lines of text (and that's without V8 options).In a related discussion (which I also can't find right now), someone suggested a
python -mlike interface, i.e., some options would only be accepted following--testor--watch.I don't have a strong opinion on this at this point, but I think it should at least be noted if either mode is considered for stable.
@tniessen this thread is not related to
--watchat all. I have also pushed back in several other threads telling people that we won't be adding a ton of arbitrary CLI flags and environment variables to core just for the test runner - but expecting that number to be 0 is a bit unreasonable.Including the experimental code coverage flag, the test runner has added six CLI flags. For comparison, there are 7 flags for just the
reportfunctionality. There are also 4--cpu-profflags, the 4--heap-profflags, 6 TLS min and max flags, 12--traceflags, etc.some options would only be accepted following --test
That would not work because all of the other test runner flags can be used without
--test. Similarly,--testcan be used without any other flags or even thenode:testmodule.It is definitely possible for Node to scope CLI flags, and that would be a breaking change, but I think that would be a reasonable breaking change to ship separately (and as I mentioned above, not really apply to the test runner currently).
our increasingly bloated CLI was raised
The CLI will likely keep getting bigger, but I don't think it's fair to try to attribute all of that to the test runner. Last I checked, the Deno executable was bigger than the Node executable. If the CLI getting larger is a pressing matter, I think we either need to stop shipping new features in core or find a way to ship core outside of the CLI (a topic for another issue though).
Reacted by Moshe Atlow and Benjamin GruenbaumThanks for the detailed comment @cjihrig. I neither have much insight nor a strong opinion on this, but I am glad it has been considered :)
Reacted by Moshe Atlow and Benjamin Gruenbaum- removedtsc-agendaIssues and PRs to discuss during Technical Steering Committee meetings.Issues and PRs to discuss during Technical Steering Committee meetings.
on Mar 6, 2023 Would loader also be marked stable in v20? If yes, then this would mean we can run TypeScript tests directly with Node, right? This woud be so great!
What is the problem this feature will solve?
Having a stable test runner in core. People have been asking for it, and I feel the majority of the test runner is ready.
What is the feature you are proposing to solve the problem?
I would like to stabilize the test runner for the 20.0.0 release. This currently would not include code coverage, which is behind a separate flag.
What alternatives have you considered?
Not stabilizing the test runner.