Skip to content

Commit

Permalink
test_runner: add shards support
Browse files Browse the repository at this point in the history
PR-URL: #48639
Backport-PR-URL: #48797
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
  • Loading branch information
rluvaton authored and juanarbol committed Jul 18, 2023
1 parent 07bfcc4 commit 0ef73ff
Show file tree
Hide file tree
Showing 19 changed files with 453 additions and 4 deletions.
22 changes: 22 additions & 0 deletions doc/api/cli.md
Expand Up @@ -1500,6 +1500,27 @@ changes:
Configures the test runner to only execute top level tests that have the `only`
option set.

### `--test-shard`

<!-- YAML
added: REPLACEME
-->

Test suite shard to execute in a format of `<index>/<total>`, where

`index` is a positive integer, index of divided parts
`total` is a positive integer, total of divided part
This command will divide all tests files into `total` equal parts,
and will run only those that happen to be in an `index` part.

For example, to split your tests suite into three parts, use this:

```bash
node --test --test-shard=1/3
node --test --test-shard=2/3
node --test --test-shard=3/3
```

### `--throw-deprecation`

<!-- YAML
Expand Down Expand Up @@ -2177,6 +2198,7 @@ Node.js options that are allowed are:
* `--test-only`
* `--test-reporter-destination`
* `--test-reporter`
* `--test-shard`
* `--throw-deprecation`
* `--title`
* `--tls-cipher-list`
Expand Down
5 changes: 5 additions & 0 deletions doc/api/test.md
Expand Up @@ -874,6 +874,11 @@ changes:
If unspecified, subtests inherit this value from their parent.
**Default:** `Infinity`.
* `watch` {boolean} Whether to run in watch mode or not. **Default:** `false`.
* `shard` {Object} Running tests in a specific shard. **Default:** `undefined`.
* `index` {number} is a positive integer between 1 and `<total>`
that specifies the index of the shard to run. This option is _required_.
* `total` {number} is a positive integer that specifies the total number
of shards to split the test files to. This option is _required_.
* Returns: {TestsStream}

```mjs
Expand Down
3 changes: 3 additions & 0 deletions doc/node.1
Expand Up @@ -424,6 +424,9 @@ The destination for the corresponding test reporter.
Configures the test runner to only execute top level tests that have the `only`
option set.
.
.It Fl -test-shard
Test suite shard to execute in a format of <index>/<total>.
.
.It Fl -throw-deprecation
Throw errors for deprecations.
.
Expand Down
37 changes: 36 additions & 1 deletion lib/internal/main/test_runner.js
Expand Up @@ -8,6 +8,16 @@ const { isUsingInspector } = require('internal/util/inspector');
const { run } = require('internal/test_runner/runner');
const { setupTestReporters } = require('internal/test_runner/utils');
const { exitCodes: { kGenericUserError } } = internalBinding('errors');
const {
codes: {
ERR_INVALID_ARG_VALUE,
},
} = require('internal/errors');
const {
NumberParseInt,
RegExpPrototypeExec,
StringPrototypeSplit,
} = primordials;

prepareMainThreadExecution(false);
markBootstrapComplete();
Expand All @@ -22,7 +32,32 @@ if (isUsingInspector()) {
inspectPort = process.debugPort;
}

run({ concurrency, inspectPort, watch: getOptionValue('--watch'), setup: setupTestReporters })
let shard;
const shardOption = getOptionValue('--test-shard');
if (shardOption) {
if (!RegExpPrototypeExec(/^\d+\/\d+$/, shardOption)) {
process.exitCode = kGenericUserError;

throw new ERR_INVALID_ARG_VALUE(
'--test-shard',
shardOption,
'must be in the form of <index>/<total>',
);
}

const { 0: indexStr, 1: totalStr } = StringPrototypeSplit(shardOption, '/');

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

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

run({ concurrency, inspectPort, watch: getOptionValue('--watch'), setup: setupTestReporters, shard })
.once('test:fail', () => {
process.exitCode = kGenericUserError;
});
34 changes: 31 additions & 3 deletions lib/internal/test_runner/runner.js
Expand Up @@ -39,10 +39,18 @@ const console = require('internal/console/global');
const {
codes: {
ERR_INVALID_ARG_TYPE,
ERR_INVALID_ARG_VALUE,
ERR_TEST_FAILURE,
ERR_OUT_OF_RANGE,
},
} = require('internal/errors');
const { validateArray, validateBoolean, validateFunction } = require('internal/validators');
const {
validateArray,
validateBoolean,
validateFunction,
validateObject,
validateInteger,
} = require('internal/validators');
const { getInspectPort, isUsingInspector, isInspectorMessage } = require('internal/util/inspector');
const { isRegExp } = require('internal/util/types');
const { kEmptyObject } = require('internal/util');
Expand Down Expand Up @@ -460,7 +468,7 @@ function run(options) {
if (options === null || typeof options !== 'object') {
options = kEmptyObject;
}
let { testNamePatterns } = options;
let { testNamePatterns, shard } = options;
const { concurrency, timeout, signal, files, inspectPort, watch, setup } = options;

if (files != null) {
Expand All @@ -469,6 +477,22 @@ function run(options) {
if (watch != null) {
validateBoolean(watch, 'options.watch');
}
if (shard != null) {
validateObject(shard, 'options.shard');
// Avoid re-evaluating the shard object in case it's a getter
shard = { __proto__: null, index: shard.index, total: shard.total };

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

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);
}

if (watch) {
throw new ERR_INVALID_ARG_VALUE('options.shard', watch, 'shards not supported with watch mode');
}
}
if (setup != null) {
validateFunction(setup, 'options.setup');
}
Expand All @@ -490,7 +514,11 @@ function run(options) {
}

const root = createTestTree({ concurrency, timeout, signal });
const testFiles = files ?? createTestFileList();
let testFiles = files ?? createTestFileList();

if (shard) {
testFiles = ArrayPrototypeFilter(testFiles, (_, index) => index % shard.total === shard.index - 1);
}

let postRun = () => root.postRun();
let filesWatcher;
Expand Down
4 changes: 4 additions & 0 deletions src/node_options.cc
Expand Up @@ -596,6 +596,10 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
"run tests with 'only' option set",
&EnvironmentOptions::test_only,
kAllowedInEnvvar);
AddOption("--test-shard",
"run test at specific shard",
&EnvironmentOptions::test_shard,
kAllowedInEnvvar);
AddOption("--test-udp-no-try-send", "", // For testing only.
&EnvironmentOptions::test_udp_no_try_send);
AddOption("--throw-deprecation",
Expand Down
1 change: 1 addition & 0 deletions src/node_options.h
Expand Up @@ -165,6 +165,7 @@ class EnvironmentOptions : public Options {
std::vector<std::string> test_reporter_destination;
bool test_only = false;
bool test_udp_no_try_send = false;
std::string test_shard;
bool throw_deprecation = false;
bool trace_atomics_wait = false;
bool trace_deprecation = false;
Expand Down
4 changes: 4 additions & 0 deletions test/fixtures/test-runner/shards/a.cjs
@@ -0,0 +1,4 @@
'use strict';
const test = require('node:test');

test('a.cjs this should pass');
4 changes: 4 additions & 0 deletions test/fixtures/test-runner/shards/b.cjs
@@ -0,0 +1,4 @@
'use strict';
const test = require('node:test');

test('b.cjs this should pass');
4 changes: 4 additions & 0 deletions test/fixtures/test-runner/shards/c.cjs
@@ -0,0 +1,4 @@
'use strict';
const test = require('node:test');

test('c.cjs this should pass');
4 changes: 4 additions & 0 deletions test/fixtures/test-runner/shards/d.cjs
@@ -0,0 +1,4 @@
'use strict';
const test = require('node:test');

test('d.cjs this should pass');
4 changes: 4 additions & 0 deletions test/fixtures/test-runner/shards/e.cjs
@@ -0,0 +1,4 @@
'use strict';
const test = require('node:test');

test('e.cjs this should pass');
4 changes: 4 additions & 0 deletions test/fixtures/test-runner/shards/f.cjs
@@ -0,0 +1,4 @@
'use strict';
const test = require('node:test');

test('f.cjs this should pass');
4 changes: 4 additions & 0 deletions test/fixtures/test-runner/shards/g.cjs
@@ -0,0 +1,4 @@
'use strict';
const test = require('node:test');

test('g.cjs this should pass');
4 changes: 4 additions & 0 deletions test/fixtures/test-runner/shards/h.cjs
@@ -0,0 +1,4 @@
'use strict';
const test = require('node:test');

test('h.cjs this should pass');
4 changes: 4 additions & 0 deletions test/fixtures/test-runner/shards/i.cjs
@@ -0,0 +1,4 @@
'use strict';
const test = require('node:test');

test('i.cjs this should pass');
4 changes: 4 additions & 0 deletions test/fixtures/test-runner/shards/j.cjs
@@ -0,0 +1,4 @@
'use strict';
const test = require('node:test');

test('j.cjs this should pass');
133 changes: 133 additions & 0 deletions test/parallel/test-runner-cli.js
Expand Up @@ -4,6 +4,7 @@ require('../common');
const assert = require('assert');
const { spawnSync } = require('child_process');
const { join } = require('path');
const { readdirSync } = require('fs');
const fixtures = require('../common/fixtures');
const testFixtures = fixtures.path('test-runner');

Expand Down Expand Up @@ -210,3 +211,135 @@ const testFixtures = fixtures.path('test-runner');
const stdout = child.stdout.toString();
assert.match(stdout, /ok 1 - this should pass/);
}

{
// --test-shard option validation
const args = ['--test', '--test-shard=1', join(testFixtures, 'index.js')];
const child = spawnSync(process.execPath, args, { cwd: testFixtures });

assert.strictEqual(child.status, 1);
assert.strictEqual(child.signal, null);
assert.match(child.stderr.toString(), /The argument '--test-shard' must be in the form of <index>\/<total>\. Received '1'/);
const stdout = child.stdout.toString();
assert.strictEqual(stdout, '');
}

{
// --test-shard option validation
const args = ['--test', '--test-shard=1/2/3', join(testFixtures, 'index.js')];
const child = spawnSync(process.execPath, args, { cwd: testFixtures });

assert.strictEqual(child.status, 1);
assert.strictEqual(child.signal, null);
assert.match(child.stderr.toString(), /The argument '--test-shard' must be in the form of <index>\/<total>\. Received '1\/2\/3'/);
const stdout = child.stdout.toString();
assert.strictEqual(stdout, '');
}

{
// --test-shard option validation
const args = ['--test', '--test-shard=0/3', join(testFixtures, 'index.js')];
const child = spawnSync(process.execPath, args, { cwd: testFixtures });

assert.strictEqual(child.status, 1);
assert.strictEqual(child.signal, null);
assert.match(child.stderr.toString(), /The value of "options\.shard\.index" is out of range\. It must be >= 1 && <= 3 \("options\.shard\.total"\)\. Received 0/);
const stdout = child.stdout.toString();
assert.strictEqual(stdout, '');
}

{
// --test-shard option validation
const args = ['--test', '--test-shard=0xf/20abcd', join(testFixtures, 'index.js')];
const child = spawnSync(process.execPath, args, { cwd: testFixtures });

assert.strictEqual(child.status, 1);
assert.strictEqual(child.signal, null);
assert.match(child.stderr.toString(), /The argument '--test-shard' must be in the form of <index>\/<total>\. Received '0xf\/20abcd'/);
const stdout = child.stdout.toString();
assert.strictEqual(stdout, '');
}

{
// --test-shard option validation
const args = ['--test', '--test-shard=hello', join(testFixtures, 'index.js')];
const child = spawnSync(process.execPath, args, { cwd: testFixtures });

assert.strictEqual(child.status, 1);
assert.strictEqual(child.signal, null);
assert.match(child.stderr.toString(), /The argument '--test-shard' must be in the form of <index>\/<total>\. Received 'hello'/);
const stdout = child.stdout.toString();
assert.strictEqual(stdout, '');
}

{
// --test-shard option, first shard
const shardsTestPath = join(testFixtures, 'shards');
const allShardsTestsFiles = readdirSync(shardsTestPath).map((file) => join(shardsTestPath, file));
const args = [
'--test',
'--test-shard=1/2',
...allShardsTestsFiles,
];
const child = spawnSync(process.execPath, args);

assert.strictEqual(child.status, 0);
assert.strictEqual(child.signal, null);
assert.strictEqual(child.stderr.toString(), '');
const stdout = child.stdout.toString();
assert.match(stdout, /# Subtest: a\.cjs this should pass/);
assert.match(stdout, /ok 1 - a\.cjs this should pass/);

assert.match(stdout, /# Subtest: c\.cjs this should pass/);
assert.match(stdout, /ok 2 - c\.cjs this should pass/);

assert.match(stdout, /# Subtest: e\.cjs this should pass/);
assert.match(stdout, /ok 3 - e\.cjs this should pass/);

assert.match(stdout, /# Subtest: g\.cjs this should pass/);
assert.match(stdout, /ok 4 - g\.cjs this should pass/);

assert.match(stdout, /# Subtest: i\.cjs this should pass/);
assert.match(stdout, /ok 5 - i\.cjs this should pass/);

assert.match(stdout, /# tests 5/);
assert.match(stdout, /# pass 5/);
assert.match(stdout, /# fail 0/);
assert.match(stdout, /# skipped 0/);
}

{
// --test-shard option, last shard
const shardsTestPath = join(testFixtures, 'shards');
const allShardsTestsFiles = readdirSync(shardsTestPath).map((file) => join(shardsTestPath, file));
const args = [
'--test',
'--test-shard=2/2',
...allShardsTestsFiles,
];
const child = spawnSync(process.execPath, args);

assert.strictEqual(child.status, 0);
assert.strictEqual(child.signal, null);
assert.strictEqual(child.stderr.toString(), '');
const stdout = child.stdout.toString();
assert.match(stdout, /# Subtest: b\.cjs this should pass/);
assert.match(stdout, /ok 1 - b\.cjs this should pass/);

assert.match(stdout, /# Subtest: d\.cjs this should pass/);
assert.match(stdout, /ok 2 - d\.cjs this should pass/);

assert.match(stdout, /# Subtest: f\.cjs this should pass/);
assert.match(stdout, /ok 3 - f\.cjs this should pass/);

assert.match(stdout, /# Subtest: h\.cjs this should pass/);
assert.match(stdout, /ok 4 - h\.cjs this should pass/);

assert.match(stdout, /# Subtest: j\.cjs this should pass/);
assert.match(stdout, /ok 5 - j\.cjs this should pass/);

assert.match(stdout, /# tests 5/);
assert.match(stdout, /# pass 5/);
assert.match(stdout, /# fail 0/);
assert.match(stdout, /# skipped 0/);
}

0 comments on commit 0ef73ff

Please sign in to comment.