test_runner, cli: add --test-concurrency flag #49996

cjihrig · 2023-10-01T15:49:05Z

This commit adds a new --test-concurrency CLI flag that controls the parallelism of the test runner CLI.

Fixes: #49487

nodejs-github-bot · 2023-10-01T15:49:10Z

Review requested:

@nodejs/test_runner

mcollina

lgtm

nodejs-github-bot · 2023-10-01T16:54:51Z

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

nodejs-github-bot · 2023-10-01T17:48:01Z

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

MoLow · 2023-10-01T17:56:04Z

test/parallel/test-runner-cli-concurrency.js

+});
+
+test('concurrency of two', () => {
+  const cp = spawnSync(process.execPath, ['--test', '--test-concurrency=2'], {


how does this test if the files really run concurrently? We have fixtures that create a mutex, can we use them?

node/test/parallel/test-runner-concurrency.js

Lines 89 to 98 in 966e3d3

test('--test multiple files', { skip: os.availableParallelism() < 3 }, async () => {

await fs.writeFile(tmpdir.resolve('test-runner-concurrency'), '');

const { code, stderr } = await common.spawnPromisified(process.execPath, [

'--test',

fixtures.path('test-runner', 'concurrency', 'a.mjs'),

fixtures.path('test-runner', 'concurrency', 'b.mjs'),

]);

assert.strictEqual(stderr, '');

assert.strictEqual(code, 0);

});

also, better to test the --test-concurrency within https://github.com/nodejs/node/blob/966e3d34936b28bfb31f00a6f862226baa3bf9d0/test/parallel/test-runner-concurrency.js to avoid the two tests using this "mutex" in parallel (and it is related to this file)

I now relaized this assersion. deepStrictEqual(readdirSync(tmpdir.path), [])
anyway we can still move the test into that file but not a blocker.
also, probably skip second test if os.availableParallelism() < 3

how does this test if the files really run concurrently?

We know that they are not running sequentially because each file never finishes. Since we know that the tests aren't running sequentially, we just need to verify that the batch size is respected. Plus, this change is not introducing any new concurrency functionality, it's just assigning a different value using the existing functionality.

nodejs-github-bot · 2023-10-08T17:30:37Z

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

nodejs-github-bot · 2023-10-08T20:09:30Z

Commit Queue failed

- Loading data for nodejs/node/pull/49996
✔  Done loading data for nodejs/node/pull/49996
----------------------------------- PR info ------------------------------------
Title      test_runner, cli: add --test-concurrency flag (#49996)
Author     Colin Ihrig  (@cjihrig)
Branch     cjihrig:test-runner-concurrency -> nodejs:main
Labels     c++, semver-minor
Commits    1
 - test_runner, cli: add --test-concurrency flag
Committers 1
 - cjihrig 
PR-URL: https://github.com/nodejs/node/pull/49996
Fixes: https://github.com/nodejs/node/issues/49487
Reviewed-By: Matteo Collina 
Reviewed-By: Moshe Atlow 
Reviewed-By: Benjamin Gruenbaum 
Reviewed-By: Chemi Atlow 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/49996
Fixes: https://github.com/nodejs/node/issues/49487
Reviewed-By: Matteo Collina 
Reviewed-By: Moshe Atlow 
Reviewed-By: Benjamin Gruenbaum 
Reviewed-By: Chemi Atlow 
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last approving review:
   ⚠  - test_runner, cli: add --test-concurrency flag
   ℹ  This PR was created on Sun, 01 Oct 2023 15:49:05 GMT
   ✔  Approvals: 4
   ✔  - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/49996#pullrequestreview-1651929277
   ✔  - Moshe Atlow (@MoLow) (TSC): https://github.com/nodejs/node/pull/49996#pullrequestreview-1651933179
   ✔  - Benjamin Gruenbaum (@benjamingr) (TSC): https://github.com/nodejs/node/pull/49996#pullrequestreview-1651942294
   ✔  - Chemi Atlow (@atlowChemi): https://github.com/nodejs/node/pull/49996#pullrequestreview-1652215745
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2023-10-08T17:30:37Z: https://ci.nodejs.org/job/node-test-pull-request/54622/
- Querying data for job/node-test-pull-request/54622/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu

https://github.com/nodejs/node/actions/runs/6449712653

This commit adds a new --test-concurrency CLI flag that controls the parallelism of the test runner CLI. PR-URL: nodejs#49996 Fixes: nodejs#49487 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il>

targos · 2023-10-10T09:06:12Z

The test added by this PR failed 4 times in one of the CI runs: https://ci.nodejs.org/job/node-test-pull-request/54448/
Please don't introduce flaky tests when you know they are flaky!

This commit adds a new --test-concurrency CLI flag that controls the parallelism of the test runner CLI. PR-URL: nodejs#49996 Fixes: nodejs#49487 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il>

This commit adds a new --test-concurrency CLI flag that controls the parallelism of the test runner CLI. PR-URL: #49996 Fixes: #49487 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il>

Notable changes: deps: * (SEMVER-MINOR) update uvwasi to 0.0.19 (Node.js GitHub Bot) #49908 doc: * add H4ad to collaborators (Vinícius Lourenço) #50217 esm: * (SEMVER-MINOR) detect ESM syntax in ambiguous JavaScript (Geoffrey Booth) #50096 * use import attributes instead of import assertions (Antoine du Hamel) #50140 * --experimental-default-type flag to flip module defaults (Geoffrey Booth) #49869 fs: * (SEMVER-MINOR) add flush option to appendFile() functions (Colin Ihrig) #50095 * add flush option to writeFile() functions (Colin Ihrig) #50009 lib: * (SEMVER-MINOR) add WebSocket client (Matthew Aitken) #49830 stream: * (SEMVER-MINOR) allow pass stream class to `stream.compose` (Alex Yang) #50187 * call helper function from push and unshift (Raz Luvaton) #50173 * optimize Writable (Robert Nagy) #50012 test_runner, cli: * (SEMVER-MINOR) add --test-concurrency flag (Colin Ihrig) #49996 vm: * (SEMVER-MINOR) use import attributes instead of import assertions (Antoine du Hamel) #50141 * use default HDO when importModuleDynamically is not set (Joyee Cheung) #49950 wasi: PR-URL: #50682

Notable changes: deps: * (SEMVER-MINOR) update uvwasi to 0.0.19 (Node.js GitHub Bot) nodejs#49908 doc: * add H4ad to collaborators (Vinícius Lourenço) nodejs#50217 esm: * (SEMVER-MINOR) detect ESM syntax in ambiguous JavaScript (Geoffrey Booth) nodejs#50096 * use import attributes instead of import assertions (Antoine du Hamel) nodejs#50140 * --experimental-default-type flag to flip module defaults (Geoffrey Booth) nodejs#49869 fs: * (SEMVER-MINOR) add flush option to appendFile() functions (Colin Ihrig) nodejs#50095 * add flush option to writeFile() functions (Colin Ihrig) nodejs#50009 lib: * (SEMVER-MINOR) add WebSocket client (Matthew Aitken) nodejs#49830 stream: * (SEMVER-MINOR) allow pass stream class to `stream.compose` (Alex Yang) nodejs#50187 * call helper function from push and unshift (Raz Luvaton) nodejs#50173 * optimize Writable (Robert Nagy) nodejs#50012 test_runner, cli: * (SEMVER-MINOR) add --test-concurrency flag (Colin Ihrig) nodejs#49996 vm: * (SEMVER-MINOR) use import attributes instead of import assertions (Antoine du Hamel) nodejs#50141 * use default HDO when importModuleDynamically is not set (Joyee Cheung) nodejs#49950 wasi: PR-URL: nodejs#50682

This commit adds a new --test-concurrency CLI flag that controls the parallelism of the test runner CLI. PR-URL: #49996 Fixes: #49487 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il>

Notable changes: deps: * (SEMVER-MINOR) update uvwasi to 0.0.19 (Node.js GitHub Bot) #49908 * (SEMVER-MINOR) upgrade npm to 10.0.0 (npm team) #49423 doc: * add new TSC members (Michael Dawson) #48841 * move and rename loaders section (Geoffrey Booth) #49261 esm: * use import attributes instead of import assertions (Antoine du Hamel) #50140 * --experimental-default-type flag to flip module defaults (Geoffrey Booth) #49869 * unflag import.meta.resolve (Guy Bedford) #49028 * move hook execution to separate thread (Jacob Smith) #44710 * leverage loaders when resolving subsequent loaders (Maël Nison) #43772 lib: * (SEMVER-MINOR) add api to detect whether source-maps are enabled (翠 / green) #46391 * (SEMVER-MINOR) add tracing channel to diagnostics_channel (Stephen Belanger) #44943 src: * (SEMVER-MINOR) add cjs_module_lexer_version base64_version (Jithil P Ponnan) #45629 stream: * use bitmap in readable state (Benjamin Gruenbaum) #49745 test_runner: * (SEMVER-MINOR) accept `testOnly` in `run` (Moshe Atlow) #49753 * (SEMVER-MINOR) add junit reporter (Moshe Atlow) #49614 * (SEMVER-MINOR) expose location of tests (Colin Ihrig) #48975 * (SEMVER-MINOR) add shards support (Raz Luvaton) #48639 * (SEMVER-MINOR) add initial draft for fakeTimers (Erick Wendel) #47775 test_runner, cli: * (SEMVER-MINOR) add --test-concurrency flag (Colin Ihrig) #49996 tls: * (SEMVER-MINOR) add ALPNCallback server option for dynamic ALPN negotiation (Tim Perry) #45190 vm: * (SEMVER-MINOR) use import attributes instead of import assertions (Antoine du Hamel) #50141 wasi: * (SEMVER-MINOR) updates required for latest uvwasi version (Michael Dawson) #49908 PR-URL: TODO

Notable changes: deps: * (SEMVER-MINOR) update uvwasi to 0.0.19 (Node.js GitHub Bot) #49908 * (SEMVER-MINOR) upgrade npm to 10.2.3 (npm team) #50531 doc: * move and rename loaders section (Geoffrey Booth) #49261 esm: * use import attributes instead of import assertions (Antoine du Hamel) #50140 * --experimental-default-type flag to flip module defaults (Geoffrey Booth) #49869 * unflag import.meta.resolve (Guy Bedford) #49028 * move hook execution to separate thread (Jacob Smith) #44710 * leverage loaders when resolving subsequent loaders (Maël Nison) #43772 lib: * (SEMVER-MINOR) add api to detect whether source-maps are enabled (翠 / green) #46391 * (SEMVER-MINOR) add tracing channel to diagnostics_channel (Stephen Belanger) #44943 src: * (SEMVER-MINOR) add cjs_module_lexer_version base64_version (Jithil P Ponnan) #45629 stream: * use bitmap in readable state (Benjamin Gruenbaum) #49745 test_runner: * (SEMVER-MINOR) accept `testOnly` in `run` (Moshe Atlow) #49753 * (SEMVER-MINOR) add junit reporter (Moshe Atlow) #49614 * (SEMVER-MINOR) expose location of tests (Colin Ihrig) #48975 * (SEMVER-MINOR) add shards support (Raz Luvaton) #48639 * (SEMVER-MINOR) add initial draft for fakeTimers (Erick Wendel) #47775 test_runner, cli: * (SEMVER-MINOR) add --test-concurrency flag (Colin Ihrig) #49996 tls: * (SEMVER-MINOR) add ALPNCallback server option for dynamic ALPN negotiation (Tim Perry) #45190 vm: * (SEMVER-MINOR) use import attributes instead of import assertions (Antoine du Hamel) #50141 PR-URL: #50953

This commit adds a new --test-concurrency CLI flag that controls the parallelism of the test runner CLI. PR-URL: nodejs#49996 Fixes: nodejs#49487 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il>

This commit adds a new --test-concurrency CLI flag that controls the parallelism of the test runner CLI. PR-URL: nodejs/node#49996 Fixes: nodejs/node#49487 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il>

Notable changes: deps: * (SEMVER-MINOR) update uvwasi to 0.0.19 (Node.js GitHub Bot) nodejs/node#49908 * (SEMVER-MINOR) upgrade npm to 10.2.3 (npm team) nodejs/node#50531 doc: * move and rename loaders section (Geoffrey Booth) nodejs/node#49261 esm: * use import attributes instead of import assertions (Antoine du Hamel) nodejs/node#50140 * --experimental-default-type flag to flip module defaults (Geoffrey Booth) nodejs/node#49869 * unflag import.meta.resolve (Guy Bedford) nodejs/node#49028 * move hook execution to separate thread (Jacob Smith) nodejs/node#44710 * leverage loaders when resolving subsequent loaders (Maël Nison) nodejs/node#43772 lib: * (SEMVER-MINOR) add api to detect whether source-maps are enabled (翠 / green) nodejs/node#46391 * (SEMVER-MINOR) add tracing channel to diagnostics_channel (Stephen Belanger) nodejs/node#44943 src: * (SEMVER-MINOR) add cjs_module_lexer_version base64_version (Jithil P Ponnan) nodejs/node#45629 stream: * use bitmap in readable state (Benjamin Gruenbaum) nodejs/node#49745 test_runner: * (SEMVER-MINOR) accept `testOnly` in `run` (Moshe Atlow) nodejs/node#49753 * (SEMVER-MINOR) add junit reporter (Moshe Atlow) nodejs/node#49614 * (SEMVER-MINOR) expose location of tests (Colin Ihrig) nodejs/node#48975 * (SEMVER-MINOR) add shards support (Raz Luvaton) nodejs/node#48639 * (SEMVER-MINOR) add initial draft for fakeTimers (Erick Wendel) nodejs/node#47775 test_runner, cli: * (SEMVER-MINOR) add --test-concurrency flag (Colin Ihrig) nodejs/node#49996 tls: * (SEMVER-MINOR) add ALPNCallback server option for dynamic ALPN negotiation (Tim Perry) nodejs/node#45190 vm: * (SEMVER-MINOR) use import attributes instead of import assertions (Antoine du Hamel) nodejs/node#50141 PR-URL: nodejs/node#50953

This commit adds a new --test-concurrency CLI flag that controls the parallelism of the test runner CLI. PR-URL: nodejs/node#49996 Fixes: nodejs/node#49487 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il>

Notable changes: deps: * (SEMVER-MINOR) update uvwasi to 0.0.19 (Node.js GitHub Bot) nodejs/node#49908 * (SEMVER-MINOR) upgrade npm to 10.2.3 (npm team) nodejs/node#50531 doc: * move and rename loaders section (Geoffrey Booth) nodejs/node#49261 esm: * use import attributes instead of import assertions (Antoine du Hamel) nodejs/node#50140 * --experimental-default-type flag to flip module defaults (Geoffrey Booth) nodejs/node#49869 * unflag import.meta.resolve (Guy Bedford) nodejs/node#49028 * move hook execution to separate thread (Jacob Smith) nodejs/node#44710 * leverage loaders when resolving subsequent loaders (Maël Nison) nodejs/node#43772 lib: * (SEMVER-MINOR) add api to detect whether source-maps are enabled (翠 / green) nodejs/node#46391 * (SEMVER-MINOR) add tracing channel to diagnostics_channel (Stephen Belanger) nodejs/node#44943 src: * (SEMVER-MINOR) add cjs_module_lexer_version base64_version (Jithil P Ponnan) nodejs/node#45629 stream: * use bitmap in readable state (Benjamin Gruenbaum) nodejs/node#49745 test_runner: * (SEMVER-MINOR) accept `testOnly` in `run` (Moshe Atlow) nodejs/node#49753 * (SEMVER-MINOR) add junit reporter (Moshe Atlow) nodejs/node#49614 * (SEMVER-MINOR) expose location of tests (Colin Ihrig) nodejs/node#48975 * (SEMVER-MINOR) add shards support (Raz Luvaton) nodejs/node#48639 * (SEMVER-MINOR) add initial draft for fakeTimers (Erick Wendel) nodejs/node#47775 test_runner, cli: * (SEMVER-MINOR) add --test-concurrency flag (Colin Ihrig) nodejs/node#49996 tls: * (SEMVER-MINOR) add ALPNCallback server option for dynamic ALPN negotiation (Tim Perry) nodejs/node#45190 vm: * (SEMVER-MINOR) use import attributes instead of import assertions (Antoine du Hamel) nodejs/node#50141 PR-URL: nodejs/node#50953

cjihrig requested review from mcollina and MoLow October 1, 2023 15:49

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Oct 1, 2023

cjihrig mentioned this pull request Oct 1, 2023

Add support for running all tests serially #49487

Closed

cjihrig force-pushed the test-runner-concurrency branch from 7f629c0 to abdb63d Compare October 1, 2023 15:53

mcollina approved these changes Oct 1, 2023

View reviewed changes

cjihrig added the semver-minor PRs that contain new features and should be released in the next minor version. label Oct 1, 2023

MoLow approved these changes Oct 1, 2023

View reviewed changes

MoLow added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 1, 2023

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 1, 2023

MoLow reviewed Oct 1, 2023

View reviewed changes

benjamingr approved these changes Oct 1, 2023

View reviewed changes

github-actions bot mentioned this pull request Oct 2, 2023

CI Reliability 2023-10-02 nodejs/reliability#681

Open

12 tasks

atlowChemi approved these changes Oct 2, 2023

View reviewed changes

This was referenced Oct 3, 2023

CI Reliability 2023-10-03 nodejs/reliability#682

Open

CI Reliability 2023-10-04 nodejs/reliability#683

Open

CI Reliability 2023-10-05 nodejs/reliability#684

Open

cjihrig force-pushed the test-runner-concurrency branch from 5a36454 to 9e8c18b Compare October 8, 2023 17:29

cjihrig added commit-queue Add this label to land a pull request using GitHub Actions. and removed needs-ci PRs that need a full CI run. labels Oct 8, 2023

nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Oct 8, 2023

cjihrig force-pushed the test-runner-concurrency branch from 9e8c18b to 9f9c582 Compare October 8, 2023 20:11

cjihrig merged commit 9f9c582 into nodejs:main Oct 8, 2023
21 checks passed

cjihrig deleted the test-runner-concurrency branch October 8, 2023 20:11

richardlau mentioned this pull request Oct 9, 2023

parallel/test-runner-cli-concurrency fails with EBUSY: resource busy or locked #50101

Closed

RafaelGSS mentioned this pull request Oct 9, 2023

2023-10-17, Version 21.0.0 (Current) #49870

Merged

targos mentioned this pull request Nov 12, 2023

v20.10.0 release proposal #50682

Merged

mateonunez mentioned this pull request Nov 19, 2023

refactor(test): remove tap from db-authorization tests platformatic/platformatic#1814

Merged

16 tasks

targos mentioned this pull request Nov 28, 2023

v18.19.0 release proposal #50953

Merged

juanarbol mentioned this pull request Dec 6, 2023

2023-12-06, N|Solid v5.0.1 Iron Release nodesource/nsolid#41

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test_runner, cli: add --test-concurrency flag #49996

test_runner, cli: add --test-concurrency flag #49996

cjihrig commented Oct 1, 2023

nodejs-github-bot commented Oct 1, 2023

mcollina left a comment

nodejs-github-bot commented Oct 1, 2023

nodejs-github-bot commented Oct 1, 2023

MoLow Oct 1, 2023

MoLow Oct 1, 2023

MoLow Oct 1, 2023

cjihrig Oct 1, 2023

nodejs-github-bot commented Oct 8, 2023 •

edited by cjihrig

nodejs-github-bot commented Oct 8, 2023

targos commented Oct 10, 2023

	test('--test multiple files', { skip: os.availableParallelism() < 3 }, async () => {
	await fs.writeFile(tmpdir.resolve('test-runner-concurrency'), '');
	const { code, stderr } = await common.spawnPromisified(process.execPath, [
	'--test',
	fixtures.path('test-runner', 'concurrency', 'a.mjs'),
	fixtures.path('test-runner', 'concurrency', 'b.mjs'),
	]);
	assert.strictEqual(stderr, '');
	assert.strictEqual(code, 0);
	});

test_runner, cli: add --test-concurrency flag #49996

test_runner, cli: add --test-concurrency flag #49996

Conversation

cjihrig commented Oct 1, 2023

nodejs-github-bot commented Oct 1, 2023

mcollina left a comment

Choose a reason for hiding this comment

nodejs-github-bot commented Oct 1, 2023

nodejs-github-bot commented Oct 1, 2023

MoLow Oct 1, 2023

Choose a reason for hiding this comment

MoLow Oct 1, 2023

Choose a reason for hiding this comment

MoLow Oct 1, 2023

Choose a reason for hiding this comment

cjihrig Oct 1, 2023

Choose a reason for hiding this comment

nodejs-github-bot commented Oct 8, 2023 • edited by cjihrig

nodejs-github-bot commented Oct 8, 2023

targos commented Oct 10, 2023

nodejs-github-bot commented Oct 8, 2023 •

edited by cjihrig