test_runner: add shards support

sercher · sercher · commit 20e3c935c0a6 · 2024-04-25T04:47:20.000+02:00
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>
diff --git a/graal-nodejs/doc/api/cli.md b/graal-nodejs/doc/api/cli.md
@@ -1289,6 +1289,27 @@ added: v18.0.0
 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
@@ -1955,6 +1976,7 @@ Node.js options that are allowed are:
 * `--test-only`
 * `--test-reporter-destination`
 * `--test-reporter`
+* `--test-shard`
 * `--throw-deprecation`
 * `--title`
 * `--tls-cipher-list`
diff --git a/graal-nodejs/doc/api/test.md b/graal-nodejs/doc/api/test.md
@@ -901,6 +901,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
diff --git a/graal-nodejs/doc/node.1 b/graal-nodejs/doc/node.1
@@ -409,6 +409,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.
 .
diff --git a/graal-nodejs/lib/internal/main/test_runner.js b/graal-nodejs/lib/internal/main/test_runner.js
@@ -7,6 +7,16 @@ const { getOptionValue } = require('internal/options');
 const { isUsingInspector } = require('internal/util/inspector');
 const { run } = require('internal/test_runner/runner');
 const { setupTestReporters } = require('internal/test_runner/utils');
+const {
+  codes: {
+    ERR_INVALID_ARG_VALUE,
+  },
+} = require('internal/errors');
+const {
+  NumberParseInt,
+  RegExpPrototypeExec,
+  StringPrototypeSplit,
+} = primordials;
 
 prepareMainThreadExecution(false);
 markBootstrapComplete();
@@ -21,7 +31,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 = 1;
+
+    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 = 1;
 });
diff --git a/graal-nodejs/lib/internal/test_runner/runner.js b/graal-nodejs/lib/internal/test_runner/runner.js
@@ -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');
@@ -459,7 +467,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) {
@@ -468,6 +476,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');
   }
@@ -489,7 +513,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;
diff --git a/graal-nodejs/src/node_options.cc b/graal-nodejs/src/node_options.cc
@@ -582,6 +582,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",
diff --git a/graal-nodejs/src/node_options.h b/graal-nodejs/src/node_options.h
@@ -161,6 +161,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;
diff --git a/graal-nodejs/test/fixtures/test-runner/shards/a.cjs b/graal-nodejs/test/fixtures/test-runner/shards/a.cjs
@@ -0,0 +1,4 @@
+'use strict';
+const test = require('node:test');
+
+test('a.cjs this should pass');
diff --git a/graal-nodejs/test/fixtures/test-runner/shards/b.cjs b/graal-nodejs/test/fixtures/test-runner/shards/b.cjs
@@ -0,0 +1,4 @@
+'use strict';
+const test = require('node:test');
+
+test('b.cjs this should pass');
diff --git a/graal-nodejs/test/fixtures/test-runner/shards/c.cjs b/graal-nodejs/test/fixtures/test-runner/shards/c.cjs
@@ -0,0 +1,4 @@
+'use strict';
+const test = require('node:test');
+
+test('c.cjs this should pass');
diff --git a/graal-nodejs/test/fixtures/test-runner/shards/d.cjs b/graal-nodejs/test/fixtures/test-runner/shards/d.cjs
@@ -0,0 +1,4 @@
+'use strict';
+const test = require('node:test');
+
+test('d.cjs this should pass');
diff --git a/graal-nodejs/test/fixtures/test-runner/shards/e.cjs b/graal-nodejs/test/fixtures/test-runner/shards/e.cjs
@@ -0,0 +1,4 @@
+'use strict';
+const test = require('node:test');
+
+test('e.cjs this should pass');
diff --git a/graal-nodejs/test/fixtures/test-runner/shards/f.cjs b/graal-nodejs/test/fixtures/test-runner/shards/f.cjs
@@ -0,0 +1,4 @@
+'use strict';
+const test = require('node:test');
+
+test('f.cjs this should pass');
diff --git a/graal-nodejs/test/fixtures/test-runner/shards/g.cjs b/graal-nodejs/test/fixtures/test-runner/shards/g.cjs
@@ -0,0 +1,4 @@
+'use strict';
+const test = require('node:test');
+
+test('g.cjs this should pass');
diff --git a/graal-nodejs/test/fixtures/test-runner/shards/h.cjs b/graal-nodejs/test/fixtures/test-runner/shards/h.cjs
@@ -0,0 +1,4 @@
+'use strict';
+const test = require('node:test');
+
+test('h.cjs this should pass');
diff --git a/graal-nodejs/test/fixtures/test-runner/shards/i.cjs b/graal-nodejs/test/fixtures/test-runner/shards/i.cjs
@@ -0,0 +1,4 @@
+'use strict';
+const test = require('node:test');
+
+test('i.cjs this should pass');
diff --git a/graal-nodejs/test/fixtures/test-runner/shards/j.cjs b/graal-nodejs/test/fixtures/test-runner/shards/j.cjs
@@ -0,0 +1,4 @@
+'use strict';
+const test = require('node:test');
+
+test('j.cjs this should pass');
diff --git a/graal-nodejs/test/parallel/test-runner-cli.js b/graal-nodejs/test/parallel/test-runner-cli.js
@@ -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');
 
@@ -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/);
+}
diff --git a/graal-nodejs/test/parallel/test-runner-run.mjs b/graal-nodejs/test/parallel/test-runner-run.mjs

Original file line number	Diff line number	Diff line change
`@@ -409,6 +409,9 @@ The destination for the corresponding test reporter.`
`409`	`409`	Configures the test runner to only execute top level tests that have the `only`
`410`	`410`	`option set.`
`411`	`411`	`.`
	`412`	`+.It Fl -test-shard`
	`413`	`+Test suite shard to execute in a format of <index>/<total>.`
	`414`	`+.`
`412`	`415`	`.It Fl -throw-deprecation`
`413`	`416`	`Throw errors for deprecations.`
`414`	`417`	`.`