Repository navigation
Add support for running all tests serially #49487
Description
Activity
- addedfeature requestIssues requesting new Node.js features.Issues requesting new Node.js features.
on Sep 4, 2023 - addedtest_runnerIssues and PRs related to the test runner subsystem.Issues and PRs related to the test runner subsystem.good first issueIssues that are suitable for first-time contributors.Issues that are suitable for first-time contributors.
on Sep 4, 2023 we have had some pushback on adding more cli flags for the test runner but besides that implementation would be trivial.
Reacted by Colin IhrigI think most applications would need this as it's a very common case. Environment variables are also good to configure this.
Alternatively, we are forcing users to call
runmanually, implementing a runner themselves. If this is the case, then we should add some docs in this regard.Reacted by Kris Kaczor and Nathan de MaestriI'm in favor of supporting this use case. I think the biggest questions to answer are:
- What exactly is the flag? We have a
concurrencyoption for tests. It supports numbers and booleans. I don't think we should support both in the CLI. - Does this flag impact tests within files at all? Currently, the CLI uses parallelism, but individual files do not by default. If I set this new flag, will it only impact the CLI? People will inevitably want to change the behavior of both.
Reacted by Moshe Atlow, Matteo Collina, Marco Ippolito, Chemi Atlow, rzdar, Armstrong Olusoji, paul-greenweb and Michael Dawson- What exactly is the flag? We have a
lib/internal/test_runner/test.js: 263 switch (typeof concurrency) { case 'number': validateUint32(concurrency, 'options.concurrency', 1); this.concurrency = concurrency; break; case 'boolean': if (concurrency) { this.concurrency = parent === null ? MathMax(availableParallelism() - 1, 1) : Infinity; } else { this.concurrency = 1; } break; default: if (concurrency != null) throw new ERR_INVALID_ARG_TYPE('options.concurrency', ['boolean', 'number'], concurrency); }So, if we pass the
concurrency = true, will it use the maximum possible threads?The proposal is to suppress this and use the integer value passed to
concurrencyoption?I can work on it, please guide.
I think if nothing is specified, it keeps the current behavior. Otherwise, a number is specified, setting concurrency to X.
I think if nothing is specified, it keeps the current behavior. Otherwise, a number is specified, setting concurrency to X.
Sure. Thanks
case 'number': validateUint32(concurrency, 'options.concurrency', 1); this.concurrency = concurrency; break;this is the way its working. So, if we are passing the concurrency option as a number, it will use it.
So, what changes we are proposing here?
Add a cli flag to set the concurrency of
node --test.Reacted by Moshe Atlow, Adrian Burlacu and awa-ximaadrian-burlacu-software commented
on Sep 28, 2023 More actionsUgly solution I have so far in
src/tests/utils/runnerwithout any command line flags:import { tap } from 'node:test/reporters'; import process from 'node:process'; import { run } from 'node:test'; import fs from 'fs'; import path from 'path'; let Files: string[] = []; function ThroughDirectory(Directory: string) { fs.readdirSync(Directory).forEach(File => { const Absolute = path.join(Directory, File); if (fs.statSync(Absolute).isDirectory()) return ThroughDirectory(Absolute); else if (Absolute.endsWith('.test.js')) return Files.push(Absolute); }); } ThroughDirectory('./dist/test'); // console.log('Files: ' + JSON.stringify(Files)); run({ concurrency: 1, files: Files }) .compose(tap) .pipe(process.stdout);
In
package.json:
"scripts": { "test": "node --env-file=./test/utils/.test.env --test-reporter=spec ./dist/test/utils/runner.js" },Would be good if I could output in spec format as a stream and get the files names/respect the
testNamePatternswithout the files argument somehow.Proposed fix in #49996.
I decided to address the concurrency of the CLI only. If we want to provide a mechanism for controlling the concurrency within a test file there are at least a few ways to extend what I've implemented:
- Add yet another flag for that.
- Support passing the new flag twice with the order having some special meaning.
- Extend the new flag's syntax to something like
--test-concurrency=1,2, where the first value controls the CLI and the second value controls the concurrency within files. - Something totally different?
The concurrency within individual files is not a priority for me at this time though.
Reacted by Giovanni Gaglione and Mateo NunezThe concurrency within individual files is not a priority for me at this time though.
- 1, a top-level
describeis usually sufficient for this
- 1, a top-level
- added a commit that references this issue
on Nov 11, 2023 - added a commit that references this issue
on Nov 27, 2023 - added a commit that references this issue
on Apr 15, 2024 - added 2 commits that reference this issue
on Apr 25, 2024
What is the problem this feature will solve?
Currently it's very hard to use
node:testto test against an external database or an external service with state as multiple files are automatically run in parallel.What is the feature you are proposing to solve the problem?
Adding a
node --test --test-concurrenty=1will do the trickWhat alternatives have you considered?
Calling
runmanually.