test_runner: add shards support by rluvaton · Pull Request #48639 · nodejs/node

rluvaton · 2023-07-02T21:53:00Z

Fix #48619

TODOs:

nodejs-github-bot · 2023-07-02T21:53:04Z

Review requested:

@nodejs/test_runner

lib/internal/test_runner/runner.js

rluvaton · 2023-07-03T10:07:48Z

@MoLow this is ready for review, thanks

doc/api/cli.md

meyfa · 2023-07-03T10:31:27Z

How good is the compatibility with globbing? For example:

$ node --test --shards=1/3 "test/**/*.js"

Is globbing deterministic in a way that when sharded, each test file runs exactly once?

rluvaton · 2023-07-03T10:32:53Z

Yes, it works well with globbing as we sort the tests files that we are about to run

lib/internal/main/test_runner.js

lib/internal/test_runner/runner.js

cjihrig · 2023-07-03T13:35:19Z

lib/internal/test_runner/runner.js

+  if (shard != null) {
+    validateObject(shard, 'options.shard');
+    validateNumber(shard.total, 'options.shard.total', 1);
+    validateNumber(shard.index, 'options.shard.index');


Suggested change

validateNumber(shard.index, 'options.shard.index');

validateInteger(shard.index, 'options.shard.index', 1, shard.total);

Not used the max here as the error message is not really understandable because you don't understand what's the reason for the upper limit

cjihrig · 2023-07-03T13:35:34Z

lib/internal/test_runner/runner.js

+    if (shard.index <= 0 || shard.total < shard.index) {
+      throw new ERR_OUT_OF_RANGE('options.shard.index', `>= 1 && <= ${shard.total} ("options.shard.total")`, shard.index);
+    }


This can be removed based on my previous comment.

see that comment answer 😄

lib/internal/test_runner/runner.js

lib/internal/main/test_runner.js

ljharb · 2023-07-03T15:53:44Z

lib/internal/main/test_runner.js

+  const index = NumberParseInt(indexStr, 10);
+  const total = NumberParseInt(totalStr, 10);


fwiw if it's NumberParseInt, the , 10 isn't required - only on the global one.

on the global one it's not required as far as I know

It's required in the conceptual sense, because parseInt will try to guess the radix if you omit it.

ljharb · 2023-07-03T15:53:58Z

lib/internal/test_runner/runner.js

-    validateNumber(shard.total, 'options.shard.total', 1);
-    validateNumber(shard.index, 'options.shard.index');
+    // Avoid re-evaluating the shard object in case it's a getter
+    shard = { index: shard.index, total: shard.total };


Suggested change

shard = { index: shard.index, total: shard.total };

shard = { __proto__: null, index: shard.index, total: shard.total };

is there a reason there is no eslint rule for that or there is some limitation prevents from implementing it?

I don't think anybody's taken the time to make one. There will be a few places where objects shouldn't have a null prototype - mostly when they're returned to userland - but those could have override comments.

I'll make one

#48646 need to answer some questions before changing all files

Notable changes: doc: * add atlowChemi to collaborators (atlowChemi) nodejs#48757 events: * (SEMVER-MINOR) allow safely adding listener to abortSignal (Chemi Atlow) nodejs#48596 fs: * add a fast-path for readFileSync utf-8 (Yagiz Nizipli) nodejs#48658 test_runner: * (SEMVER-MINOR) add shards support (Raz Luvaton) nodejs#48639 PR-URL: nodejs#48761

PR-URL: nodejs#48639 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

Notable changes: doc: * add atlowChemi to collaborators (atlowChemi) nodejs#48757 events: * (SEMVER-MINOR) allow safely adding listener to abortSignal (Chemi Atlow) nodejs#48596 fs: * add a fast-path for readFileSync utf-8 (Yagiz Nizipli) nodejs#48658 test_runner: * (SEMVER-MINOR) add shards support (Raz Luvaton) nodejs#48639 PR-URL: nodejs#48761

PR-URL: nodejs#48639 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

Notable changes: doc: * add atlowChemi to collaborators (atlowChemi) nodejs#48757 events: * (SEMVER-MINOR) allow safely adding listener to abortSignal (Chemi Atlow) nodejs#48596 fs: * add a fast-path for readFileSync utf-8 (Yagiz Nizipli) nodejs#48658 test_runner: * (SEMVER-MINOR) add shards support (Raz Luvaton) nodejs#48639 PR-URL: nodejs#48761

ruyadorno · 2023-09-11T02:31:42Z

This commit does not land cleanly on v18.x-staging and will need manual backport in case we want it in v18.

rluvaton · 2023-09-22T07:54:12Z

Missed that, will create one now

PR-URL: nodejs#48639 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

rluvaton · 2023-09-22T08:27:54Z

@ruyadorno backported to v18 created in #49762

PR-URL: nodejs#48639 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

PR-URL: #48639 Backport-PR-URL: #49762 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

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

PR-URL: nodejs/node#48639 Backport-PR-URL: nodejs/node#49762 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

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

PR-URL: nodejs/node#48639 Backport-PR-URL: nodejs/node#49762 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

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

test_runner: add shards support

a93fa41

nodejs-github-bot added needs-ci PRs that need a full CI run. test_runner Issues and PRs related to the test runner subsystem. labels Jul 2, 2023

rluvaton added 4 commits July 3, 2023 01:19

test_runner: add validations

3bc36b8

test_runner: add tests for the shards

bf9d2e5

reset unwanted changes

3663fb2

do not mutate array

09c8734

MoLow reviewed Jul 3, 2023

View reviewed changes

lib/internal/test_runner/runner.js Outdated Show resolved Hide resolved

rluvaton added 4 commits July 3, 2023 11:40

update validation

ed19709

allow passing shards in CLI

5e39d75

test_runner: add cli support

5c6ab88

test_runner: added docs

d293da5

rluvaton marked this pull request as ready for review July 3, 2023 10:07

rluvaton added 3 commits July 3, 2023 13:11

remove comment

51f7697

format

0625b75

fix example

2e0040b

meyfa reviewed Jul 3, 2023

View reviewed changes

doc/api/cli.md Outdated Show resolved Hide resolved

test_runner: replace shards option with shard and --test-shard

602f767

targos reviewed Jul 3, 2023

View reviewed changes

lib/internal/main/test_runner.js Outdated Show resolved Hide resolved

test_runner: used stricter shard validation

4ce4d76

cjihrig reviewed Jul 3, 2023

View reviewed changes

MoLow reviewed Jul 3, 2023

View reviewed changes

lib/internal/main/test_runner.js Outdated Show resolved Hide resolved

rluvaton added 2 commits July 3, 2023 18:10

update node man page

b7009d7

test_runner: update based on CR

76cfcb6

ljharb reviewed Jul 3, 2023

View reviewed changes

MoLow approved these changes Jul 3, 2023

View reviewed changes

Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023

test_runner: add shards support

1d03c88

PR-URL: nodejs#48639 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023

test_runner: add shards support

99e738a

PR-URL: nodejs#48639 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

ruyadorno added the backport-requested-v18.x label Sep 11, 2023

rluvaton added a commit to rluvaton/node that referenced this pull request Sep 22, 2023

test_runner: add shards support

9be6419

PR-URL: nodejs#48639 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

This was referenced Sep 22, 2023

[v18.x backport] test_runner: add shards support #49761

Closed

[v18.x backport] test_runner: add shards support #49762

Closed

rluvaton added backport-open-v18.x backported-to-v20.x PRs backported to the v20.x-staging branch. and removed backport-requested-v18.x backport-open-v20.x Indicate that the PR has an open backport labels Sep 22, 2023

rluvaton added a commit to rluvaton/node that referenced this pull request Sep 22, 2023

test_runner: add shards support

cb8763f

PR-URL: nodejs#48639 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

rluvaton added a commit to rluvaton/node that referenced this pull request Sep 22, 2023

test_runner: add shards support

5e2af43

PR-URL: nodejs#48639 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

rluvaton added a commit to rluvaton/node that referenced this pull request Sep 22, 2023

test_runner: add shards support

4dcd96b

PR-URL: nodejs#48639 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

targos pushed a commit that referenced this pull request Oct 28, 2023

test_runner: add shards support

b55eb2a

PR-URL: #48639 Backport-PR-URL: #49762 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

targos mentioned this pull request Nov 28, 2023

v18.19.0 release proposal #50953

Merged

sercher added a commit to sercher/graaljs that referenced this pull request Apr 25, 2024

test_runner: add shards support

90b7c8d

PR-URL: nodejs/node#48639 Backport-PR-URL: nodejs/node#49762 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

sercher added a commit to sercher/graaljs that referenced this pull request Apr 25, 2024

test_runner: add shards support

20e3c93

PR-URL: nodejs/node#48639 Backport-PR-URL: nodejs/node#49762 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

	validateNumber(shard.index, 'options.shard.index');
	validateInteger(shard.index, 'options.shard.index', 1, shard.total);

		const index = NumberParseInt(indexStr, 10);
		const total = NumberParseInt(totalStr, 10);

	shard = { index: shard.index, total: shard.total };
	shard = { __proto__: null, index: shard.index, total: shard.total };

Uh oh!

Conversation

rluvaton commented Jul 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented Jul 2, 2023

Uh oh!

Uh oh!

rluvaton commented Jul 3, 2023

Uh oh!

Uh oh!

meyfa commented Jul 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rluvaton commented Jul 3, 2023

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rluvaton Jul 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ruyadorno commented Sep 11, 2023

Uh oh!

rluvaton commented Sep 22, 2023

Uh oh!

rluvaton commented Sep 22, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants

rluvaton commented Jul 2, 2023 •

edited

Loading

meyfa commented Jul 3, 2023 •

edited

Loading

rluvaton Jul 3, 2023 •

edited

Loading