From a93fa41108e8e78b019876726d7b75d8611ce737 Mon Sep 17 00:00:00 2001 From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Mon, 3 Jul 2023 00:51:48 +0300 Subject: [PATCH 01/18] test_runner: add shards support --- lib/internal/test_runner/runner.js | 30 +++++++++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js index af121273c9e638..6fffa61941fea6 100644 --- a/lib/internal/test_runner/runner.js +++ b/lib/internal/test_runner/runner.js @@ -39,10 +39,11 @@ const console = require('internal/console/global'); const { codes: { ERR_INVALID_ARG_TYPE, + ERR_INVALID_ARG_VALUE, ERR_TEST_FAILURE, }, } = require('internal/errors'); -const { validateArray, validateBoolean, validateFunction } = require('internal/validators'); +const { validateArray, validateBoolean, validateFunction, validateObject, validateNumber} = require('internal/validators'); const { getInspectPort, isUsingInspector, isInspectorMessage } = require('internal/util/inspector'); const { isRegExp } = require('internal/util/types'); const { kEmptyObject } = require('internal/util'); @@ -417,7 +418,7 @@ function run(options) { options = kEmptyObject; } let { testNamePatterns } = options; - const { concurrency, timeout, signal, files, inspectPort, watch, setup } = options; + const { concurrency, timeout, signal, files, inspectPort, watch, setup, shards } = options; if (files != null) { validateArray(files, 'options.files'); @@ -425,6 +426,25 @@ function run(options) { if (watch != null) { validateBoolean(watch, 'options.watch'); } + if (shards != null) { + validateObject(shards, 'options.shards'); + validateNumber(shards.total, 'options.shards.total'); + validateNumber(shards.index, 'options.shards.index'); + + if(shards.total <= 0) { + throw new ERR_INVALID_ARG_VALUE('options.shards.total', shards.total, 'total shards must be greater than 0'); + } + if(shards.index <= 0) { + throw new ERR_INVALID_ARG_VALUE('options.shards.index', shards.index, 'shard index must be greater than 0'); + } + if(shards.total < shards.index) { + throw new ERR_INVALID_ARG_VALUE('options.shards.index', shards.index, 'shard index must be less than total shards'); + } + + if(watch) { + throw new ERR_INVALID_ARG_VALUE('options.shards', watch, 'shards not supported with watch mode'); + } + } if (setup != null) { validateFunction(setup, 'options.setup'); } @@ -446,7 +466,11 @@ function run(options) { } const root = createTestTree({ concurrency, timeout, signal }); - const testFiles = files ?? createTestFileList(); + let testFiles = files ?? createTestFileList(); + + if(shards) { + testFiles = testFiles.filter((file, index) => (index % shards.total) === (shards.index - 1)); + } let postRun = () => root.postRun(); let filesWatcher; From 3bc36b8f044b12bc7c8b04b7f9da7f4e12453c64 Mon Sep 17 00:00:00 2001 From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Mon, 3 Jul 2023 01:19:43 +0300 Subject: [PATCH 02/18] test_runner: add validations --- lib/internal/test_runner/runner.js | 22 ++++-- test/parallel/test-runner-run.mjs | 115 ++++++++++++++++++++++++++--- 2 files changed, 119 insertions(+), 18 deletions(-) diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js index 6fffa61941fea6..d5eb9fcd6cfd56 100644 --- a/lib/internal/test_runner/runner.js +++ b/lib/internal/test_runner/runner.js @@ -43,7 +43,13 @@ const { ERR_TEST_FAILURE, }, } = require('internal/errors'); -const { validateArray, validateBoolean, validateFunction, validateObject, validateNumber} = require('internal/validators'); +const { + validateArray, + validateBoolean, + validateFunction, + validateObject, + validateNumber, +} = require('internal/validators'); const { getInspectPort, isUsingInspector, isInspectorMessage } = require('internal/util/inspector'); const { isRegExp } = require('internal/util/types'); const { kEmptyObject } = require('internal/util'); @@ -431,17 +437,19 @@ function run(options) { validateNumber(shards.total, 'options.shards.total'); validateNumber(shards.index, 'options.shards.index'); - if(shards.total <= 0) { + if (shards.total <= 0) { throw new ERR_INVALID_ARG_VALUE('options.shards.total', shards.total, 'total shards must be greater than 0'); } - if(shards.index <= 0) { + if (shards.index <= 0) { throw new ERR_INVALID_ARG_VALUE('options.shards.index', shards.index, 'shard index must be greater than 0'); } - if(shards.total < shards.index) { - throw new ERR_INVALID_ARG_VALUE('options.shards.index', shards.index, 'shard index must be less than total shards'); + if (shards.total < shards.index) { + throw new ERR_INVALID_ARG_VALUE( + 'options.shards.index', shards.index, 'shard index must be less than total shards', + ); } - if(watch) { + if (watch) { throw new ERR_INVALID_ARG_VALUE('options.shards', watch, 'shards not supported with watch mode'); } } @@ -468,7 +476,7 @@ function run(options) { const root = createTestTree({ concurrency, timeout, signal }); let testFiles = files ?? createTestFileList(); - if(shards) { + if (shards) { testFiles = testFiles.filter((file, index) => (index % shards.total) === (shards.index - 1)); } diff --git a/test/parallel/test-runner-run.mjs b/test/parallel/test-runner-run.mjs index 0e92abd92bb5a2..6aa8d37d283f35 100644 --- a/test/parallel/test-runner-run.mjs +++ b/test/parallel/test-runner-run.mjs @@ -14,7 +14,7 @@ describe('require(\'node:test\').run', { concurrency: true }, () => { stream.on('test:fail', common.mustNotCall()); stream.on('test:pass', common.mustNotCall()); // eslint-disable-next-line no-unused-vars - for await (const _ of stream); + for await (const _ of stream) ; }); it('should fail with non existing file', async () => { @@ -22,7 +22,7 @@ describe('require(\'node:test\').run', { concurrency: true }, () => { stream.on('test:fail', common.mustCall(1)); stream.on('test:pass', common.mustNotCall()); // eslint-disable-next-line no-unused-vars - for await (const _ of stream); + for await (const _ of stream) ; }); it('should succeed with a file', async () => { @@ -30,7 +30,7 @@ describe('require(\'node:test\').run', { concurrency: true }, () => { stream.on('test:fail', common.mustNotCall()); stream.on('test:pass', common.mustCall(1)); // eslint-disable-next-line no-unused-vars - for await (const _ of stream); + for await (const _ of stream) ; }); it('should run same file twice', async () => { @@ -38,7 +38,7 @@ describe('require(\'node:test\').run', { concurrency: true }, () => { stream.on('test:fail', common.mustNotCall()); stream.on('test:pass', common.mustCall(2)); // eslint-disable-next-line no-unused-vars - for await (const _ of stream); + for await (const _ of stream) ; }); it('should run a failed test', async () => { @@ -46,22 +46,25 @@ describe('require(\'node:test\').run', { concurrency: true }, () => { stream.on('test:fail', common.mustCall(1)); stream.on('test:pass', common.mustNotCall()); // eslint-disable-next-line no-unused-vars - for await (const _ of stream); + for await (const _ of stream) ; }); it('should support timeout', async () => { - const stream = run({ timeout: 50, files: [ - fixtures.path('test-runner', 'never_ending_sync.js'), - fixtures.path('test-runner', 'never_ending_async.js'), - ] }); + const stream = run({ + timeout: 50, files: [ + fixtures.path('test-runner', 'never_ending_sync.js'), + fixtures.path('test-runner', 'never_ending_async.js'), + ] + }); stream.on('test:fail', common.mustCall(2)); stream.on('test:pass', common.mustNotCall()); // eslint-disable-next-line no-unused-vars - for await (const _ of stream); + for await (const _ of stream) ; }); it('should validate files', async () => { - [Symbol(), {}, () => {}, 0, 1, 0n, 1n, '', '1', Promise.resolve([]), true, false] + [Symbol(), {}, () => { + }, 0, 1, 0n, 1n, '', '1', Promise.resolve([]), true, false] .forEach((files) => assert.throws(() => run({ files }), { code: 'ERR_INVALID_ARG_TYPE' })); @@ -142,4 +145,94 @@ describe('require(\'node:test\').run', { concurrency: true }, () => { } }); }); + + describe('sharding', () => { + describe('validation', () => { + it('should require shards.total when having shards option', () => { + assert.throws(() => run({ files: [join(testFixtures, 'test/random.cjs')], shards: {} }), { + name: 'TypeError', + code: 'ERR_INVALID_ARG_TYPE', + message: 'The "options.shards.total" property must be of type number. Received undefined' + }); + }); + + it('should require shards.index when having shards option', () => { + assert.throws(() => run({ + files: [join(testFixtures, 'test/random.cjs')], + shards: { + total: 5 + } + }), { + name: 'TypeError', + code: 'ERR_INVALID_ARG_TYPE', + message: 'The "options.shards.index" property must be of type number. Received undefined' + }); + }); + + it('should require shards.total to be greater than 0 when having shards option', () => { + assert.throws(() => run({ + files: [join(testFixtures, 'test/random.cjs')], + shards: { + total: 0, + index: 1 + } + }), { + name: 'TypeError', + code: 'ERR_INVALID_ARG_VALUE', + message: 'The property \'options.shards.total\' total shards must be greater than 0. Received 0' + }); + }); + + it('should require shards.index to be greater than 0 when having shards option', () => { + assert.throws(() => run({ + files: [join(testFixtures, 'test/random.cjs')], + shards: { + total: 6, + index: 0 + } + }), { + name: 'TypeError', + code: 'ERR_INVALID_ARG_VALUE', + message: 'The property \'options.shards.index\' shard index must be greater than 0. Received 0' + }); + }); + + it('should require shards.index to not be greater than the shards total when having shards option', () => { + assert.throws(() => run({ + files: [join(testFixtures, 'test/random.cjs')], + shards: { + total: 6, + index: 7 + } + }), { + name: 'TypeError', + code: 'ERR_INVALID_ARG_VALUE', + message: 'The property \'options.shards.index\' shard index must be less than total shards. Received 7' + }); + }); + + it('should require watch mode to e disabled when having shards option', () => { + assert.throws(() => run({ + files: [join(testFixtures, 'test/random.cjs')], + watch: true, + shards: { + total: 6, + index: 1 + } + }), { + name: 'TypeError', + code: 'ERR_INVALID_ARG_VALUE', + message: 'The property \'options.shards\' shards not supported with watch mode. Received true' + }); + }); + }); + + it('should run only the tests files matching the shard index', async () => { + + }); + + it('should run only the tests file', async () => { + + }); + }); }); From bf9d2e564ea3621403b77de07cda90a7f2992d3b Mon Sep 17 00:00:00 2001 From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Mon, 3 Jul 2023 01:38:30 +0300 Subject: [PATCH 03/18] test_runner: add tests for the shards --- test/fixtures/test-runner/shards/a.cjs | 4 ++ test/fixtures/test-runner/shards/b.cjs | 4 ++ test/fixtures/test-runner/shards/c.cjs | 4 ++ test/fixtures/test-runner/shards/d.cjs | 4 ++ test/fixtures/test-runner/shards/e.cjs | 4 ++ test/fixtures/test-runner/shards/f.cjs | 4 ++ test/fixtures/test-runner/shards/g.cjs | 4 ++ test/fixtures/test-runner/shards/h.cjs | 4 ++ test/fixtures/test-runner/shards/i.cjs | 4 ++ test/fixtures/test-runner/shards/j.cjs | 4 ++ test/parallel/test-runner-run.mjs | 99 ++++++++++++++++++++++++-- 11 files changed, 132 insertions(+), 7 deletions(-) create mode 100644 test/fixtures/test-runner/shards/a.cjs create mode 100644 test/fixtures/test-runner/shards/b.cjs create mode 100644 test/fixtures/test-runner/shards/c.cjs create mode 100644 test/fixtures/test-runner/shards/d.cjs create mode 100644 test/fixtures/test-runner/shards/e.cjs create mode 100644 test/fixtures/test-runner/shards/f.cjs create mode 100644 test/fixtures/test-runner/shards/g.cjs create mode 100644 test/fixtures/test-runner/shards/h.cjs create mode 100644 test/fixtures/test-runner/shards/i.cjs create mode 100644 test/fixtures/test-runner/shards/j.cjs diff --git a/test/fixtures/test-runner/shards/a.cjs b/test/fixtures/test-runner/shards/a.cjs new file mode 100644 index 00000000000000..2a722c504b9fa5 --- /dev/null +++ b/test/fixtures/test-runner/shards/a.cjs @@ -0,0 +1,4 @@ +'use strict'; +const test = require('node:test'); + +test('this should pass'); diff --git a/test/fixtures/test-runner/shards/b.cjs b/test/fixtures/test-runner/shards/b.cjs new file mode 100644 index 00000000000000..2a722c504b9fa5 --- /dev/null +++ b/test/fixtures/test-runner/shards/b.cjs @@ -0,0 +1,4 @@ +'use strict'; +const test = require('node:test'); + +test('this should pass'); diff --git a/test/fixtures/test-runner/shards/c.cjs b/test/fixtures/test-runner/shards/c.cjs new file mode 100644 index 00000000000000..2a722c504b9fa5 --- /dev/null +++ b/test/fixtures/test-runner/shards/c.cjs @@ -0,0 +1,4 @@ +'use strict'; +const test = require('node:test'); + +test('this should pass'); diff --git a/test/fixtures/test-runner/shards/d.cjs b/test/fixtures/test-runner/shards/d.cjs new file mode 100644 index 00000000000000..2a722c504b9fa5 --- /dev/null +++ b/test/fixtures/test-runner/shards/d.cjs @@ -0,0 +1,4 @@ +'use strict'; +const test = require('node:test'); + +test('this should pass'); diff --git a/test/fixtures/test-runner/shards/e.cjs b/test/fixtures/test-runner/shards/e.cjs new file mode 100644 index 00000000000000..2a722c504b9fa5 --- /dev/null +++ b/test/fixtures/test-runner/shards/e.cjs @@ -0,0 +1,4 @@ +'use strict'; +const test = require('node:test'); + +test('this should pass'); diff --git a/test/fixtures/test-runner/shards/f.cjs b/test/fixtures/test-runner/shards/f.cjs new file mode 100644 index 00000000000000..2a722c504b9fa5 --- /dev/null +++ b/test/fixtures/test-runner/shards/f.cjs @@ -0,0 +1,4 @@ +'use strict'; +const test = require('node:test'); + +test('this should pass'); diff --git a/test/fixtures/test-runner/shards/g.cjs b/test/fixtures/test-runner/shards/g.cjs new file mode 100644 index 00000000000000..2a722c504b9fa5 --- /dev/null +++ b/test/fixtures/test-runner/shards/g.cjs @@ -0,0 +1,4 @@ +'use strict'; +const test = require('node:test'); + +test('this should pass'); diff --git a/test/fixtures/test-runner/shards/h.cjs b/test/fixtures/test-runner/shards/h.cjs new file mode 100644 index 00000000000000..2a722c504b9fa5 --- /dev/null +++ b/test/fixtures/test-runner/shards/h.cjs @@ -0,0 +1,4 @@ +'use strict'; +const test = require('node:test'); + +test('this should pass'); diff --git a/test/fixtures/test-runner/shards/i.cjs b/test/fixtures/test-runner/shards/i.cjs new file mode 100644 index 00000000000000..2a722c504b9fa5 --- /dev/null +++ b/test/fixtures/test-runner/shards/i.cjs @@ -0,0 +1,4 @@ +'use strict'; +const test = require('node:test'); + +test('this should pass'); diff --git a/test/fixtures/test-runner/shards/j.cjs b/test/fixtures/test-runner/shards/j.cjs new file mode 100644 index 00000000000000..2a722c504b9fa5 --- /dev/null +++ b/test/fixtures/test-runner/shards/j.cjs @@ -0,0 +1,4 @@ +'use strict'; +const test = require('node:test'); + +test('this should pass'); diff --git a/test/parallel/test-runner-run.mjs b/test/parallel/test-runner-run.mjs index 6aa8d37d283f35..581b36977d9140 100644 --- a/test/parallel/test-runner-run.mjs +++ b/test/parallel/test-runner-run.mjs @@ -147,9 +147,23 @@ describe('require(\'node:test\').run', { concurrency: true }, () => { }); describe('sharding', () => { + const shardsTestsFixtures = fixtures.path('test-runner/shards'); + const shardsTestsFiles = [ + 'a.cjs', + 'b.cjs', + 'c.cjs', + 'd.cjs', + 'e.cjs', + 'f.cjs', + 'g.cjs', + 'h.cjs', + 'i.cjs', + 'j.cjs', + ].map((file) => join(shardsTestsFixtures, file)); + describe('validation', () => { it('should require shards.total when having shards option', () => { - assert.throws(() => run({ files: [join(testFixtures, 'test/random.cjs')], shards: {} }), { + assert.throws(() => run({ files: shardsTestsFiles, shards: {} }), { name: 'TypeError', code: 'ERR_INVALID_ARG_TYPE', message: 'The "options.shards.total" property must be of type number. Received undefined' @@ -158,7 +172,7 @@ describe('require(\'node:test\').run', { concurrency: true }, () => { it('should require shards.index when having shards option', () => { assert.throws(() => run({ - files: [join(testFixtures, 'test/random.cjs')], + files: shardsTestsFiles, shards: { total: 5 } @@ -171,7 +185,7 @@ describe('require(\'node:test\').run', { concurrency: true }, () => { it('should require shards.total to be greater than 0 when having shards option', () => { assert.throws(() => run({ - files: [join(testFixtures, 'test/random.cjs')], + files: shardsTestsFiles, shards: { total: 0, index: 1 @@ -185,7 +199,7 @@ describe('require(\'node:test\').run', { concurrency: true }, () => { it('should require shards.index to be greater than 0 when having shards option', () => { assert.throws(() => run({ - files: [join(testFixtures, 'test/random.cjs')], + files: shardsTestsFiles, shards: { total: 6, index: 0 @@ -199,7 +213,7 @@ describe('require(\'node:test\').run', { concurrency: true }, () => { it('should require shards.index to not be greater than the shards total when having shards option', () => { assert.throws(() => run({ - files: [join(testFixtures, 'test/random.cjs')], + files: shardsTestsFiles, shards: { total: 6, index: 7 @@ -213,7 +227,7 @@ describe('require(\'node:test\').run', { concurrency: true }, () => { it('should require watch mode to e disabled when having shards option', () => { assert.throws(() => run({ - files: [join(testFixtures, 'test/random.cjs')], + files: shardsTestsFiles, watch: true, shards: { total: 6, @@ -228,11 +242,82 @@ describe('require(\'node:test\').run', { concurrency: true }, () => { }); it('should run only the tests files matching the shard index', async () => { + const stream = run({ + files: shardsTestsFiles, + shards: { + total: 5, + index: 1 + } + }); + + const executedTestFiles = []; + stream.on('test:fail', common.mustNotCall()); + stream.on('test:pass', (passedTest) => { + executedTestFiles.push(passedTest.file); + }); + // eslint-disable-next-line no-unused-vars + for await (const _ of stream) ; + + assert.deepStrictEqual(executedTestFiles, [ + join(shardsTestsFixtures, 'a.cjs'), + join(shardsTestsFixtures, 'f.cjs'), + ]); + }); + + it('different shards should not run the same file', async () => { + const executedTestFiles = []; + + const testStreams = []; + const shards = 5; + for (let i = 1; i <= shards; i++) { + const stream = run({ + files: shardsTestsFiles, + shards: { + total: shards, + index: i + } + }); + stream.on('test:fail', common.mustNotCall()); + stream.on('test:pass', (passedTest) => { + executedTestFiles.push(passedTest.file); + }); + testStreams.push(stream); + } + + await Promise.all(testStreams.map(async (stream) => { + // eslint-disable-next-line no-unused-vars + for await (const _ of stream) ; + })); + assert.deepStrictEqual(executedTestFiles, [...new Set(executedTestFiles)]); }); - it('should run only the tests file', async () => { + it('combination of all shards should be all the tests', async () => { + const executedTestFiles = []; + + const testStreams = []; + const shards = 5; + for (let i = 1; i <= shards; i++) { + const stream = run({ + files: shardsTestsFiles, + shards: { + total: shards, + index: i + } + }); + stream.on('test:fail', common.mustNotCall()); + stream.on('test:pass', (passedTest) => { + executedTestFiles.push(passedTest.file); + }); + testStreams.push(stream); + } + + await Promise.all(testStreams.map(async (stream) => { + // eslint-disable-next-line no-unused-vars + for await (const _ of stream) ; + })); + assert.deepStrictEqual(executedTestFiles.sort(), shardsTestsFiles.sort()); }); }); }); From 3663fb2e53691b26337c7ec09b6d3f59924be01f Mon Sep 17 00:00:00 2001 From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Mon, 3 Jul 2023 01:50:57 +0300 Subject: [PATCH 04/18] reset unwanted changes --- test/parallel/test-runner-run.mjs | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/test/parallel/test-runner-run.mjs b/test/parallel/test-runner-run.mjs index 581b36977d9140..950aff2b4dfbea 100644 --- a/test/parallel/test-runner-run.mjs +++ b/test/parallel/test-runner-run.mjs @@ -14,7 +14,7 @@ describe('require(\'node:test\').run', { concurrency: true }, () => { stream.on('test:fail', common.mustNotCall()); stream.on('test:pass', common.mustNotCall()); // eslint-disable-next-line no-unused-vars - for await (const _ of stream) ; + for await (const _ of stream); }); it('should fail with non existing file', async () => { @@ -22,7 +22,7 @@ describe('require(\'node:test\').run', { concurrency: true }, () => { stream.on('test:fail', common.mustCall(1)); stream.on('test:pass', common.mustNotCall()); // eslint-disable-next-line no-unused-vars - for await (const _ of stream) ; + for await (const _ of stream); }); it('should succeed with a file', async () => { @@ -30,7 +30,7 @@ describe('require(\'node:test\').run', { concurrency: true }, () => { stream.on('test:fail', common.mustNotCall()); stream.on('test:pass', common.mustCall(1)); // eslint-disable-next-line no-unused-vars - for await (const _ of stream) ; + for await (const _ of stream); }); it('should run same file twice', async () => { @@ -38,7 +38,7 @@ describe('require(\'node:test\').run', { concurrency: true }, () => { stream.on('test:fail', common.mustNotCall()); stream.on('test:pass', common.mustCall(2)); // eslint-disable-next-line no-unused-vars - for await (const _ of stream) ; + for await (const _ of stream); }); it('should run a failed test', async () => { @@ -46,25 +46,22 @@ describe('require(\'node:test\').run', { concurrency: true }, () => { stream.on('test:fail', common.mustCall(1)); stream.on('test:pass', common.mustNotCall()); // eslint-disable-next-line no-unused-vars - for await (const _ of stream) ; + for await (const _ of stream); }); it('should support timeout', async () => { - const stream = run({ - timeout: 50, files: [ - fixtures.path('test-runner', 'never_ending_sync.js'), - fixtures.path('test-runner', 'never_ending_async.js'), - ] - }); + const stream = run({ timeout: 50, files: [ + fixtures.path('test-runner', 'never_ending_sync.js'), + fixtures.path('test-runner', 'never_ending_async.js'), + ] }); stream.on('test:fail', common.mustCall(2)); stream.on('test:pass', common.mustNotCall()); // eslint-disable-next-line no-unused-vars - for await (const _ of stream) ; + for await (const _ of stream); }); it('should validate files', async () => { - [Symbol(), {}, () => { - }, 0, 1, 0n, 1n, '', '1', Promise.resolve([]), true, false] + [Symbol(), {}, () => {}, 0, 1, 0n, 1n, '', '1', Promise.resolve([]), true, false] .forEach((files) => assert.throws(() => run({ files }), { code: 'ERR_INVALID_ARG_TYPE' })); From 09c873431aa77a1fc868f413e159998d803cd689 Mon Sep 17 00:00:00 2001 From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Mon, 3 Jul 2023 01:55:07 +0300 Subject: [PATCH 05/18] do not mutate array --- test/parallel/test-runner-run.mjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-runner-run.mjs b/test/parallel/test-runner-run.mjs index 950aff2b4dfbea..6506d6c0c9609a 100644 --- a/test/parallel/test-runner-run.mjs +++ b/test/parallel/test-runner-run.mjs @@ -314,7 +314,7 @@ describe('require(\'node:test\').run', { concurrency: true }, () => { for await (const _ of stream) ; })); - assert.deepStrictEqual(executedTestFiles.sort(), shardsTestsFiles.sort()); + assert.deepStrictEqual(executedTestFiles.sort(), [...shardsTestsFiles].sort()); }); }); }); From ed19709d2f59f89ef1381a0e419c1c501488ee6e Mon Sep 17 00:00:00 2001 From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Mon, 3 Jul 2023 11:40:56 +0300 Subject: [PATCH 06/18] update validation --- lib/internal/test_runner/runner.js | 16 +++++----------- test/parallel/test-runner-run.mjs | 22 ++++++++++++---------- 2 files changed, 17 insertions(+), 21 deletions(-) diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js index d5eb9fcd6cfd56..69050a0e3d331a 100644 --- a/lib/internal/test_runner/runner.js +++ b/lib/internal/test_runner/runner.js @@ -41,6 +41,7 @@ const { ERR_INVALID_ARG_TYPE, ERR_INVALID_ARG_VALUE, ERR_TEST_FAILURE, + ERR_OUT_OF_RANGE, }, } = require('internal/errors'); const { @@ -434,19 +435,12 @@ function run(options) { } if (shards != null) { validateObject(shards, 'options.shards'); - validateNumber(shards.total, 'options.shards.total'); + validateNumber(shards.total, 'options.shards.total', 1); validateNumber(shards.index, 'options.shards.index'); + // validateNumber(shards.index, 'options.shards.index', 1, shards.total); - if (shards.total <= 0) { - throw new ERR_INVALID_ARG_VALUE('options.shards.total', shards.total, 'total shards must be greater than 0'); - } - if (shards.index <= 0) { - throw new ERR_INVALID_ARG_VALUE('options.shards.index', shards.index, 'shard index must be greater than 0'); - } - if (shards.total < shards.index) { - throw new ERR_INVALID_ARG_VALUE( - 'options.shards.index', shards.index, 'shard index must be less than total shards', - ); + if (shards.index <= 0 || shards.total < shards.index) { + throw new ERR_OUT_OF_RANGE('options.shards.index', `>= 1 && <= ${shards.total} ("options.shards.total")`, shards.index); } if (watch) { diff --git a/test/parallel/test-runner-run.mjs b/test/parallel/test-runner-run.mjs index 6506d6c0c9609a..9dc9c1b32d7c5d 100644 --- a/test/parallel/test-runner-run.mjs +++ b/test/parallel/test-runner-run.mjs @@ -188,9 +188,9 @@ describe('require(\'node:test\').run', { concurrency: true }, () => { index: 1 } }), { - name: 'TypeError', - code: 'ERR_INVALID_ARG_VALUE', - message: 'The property \'options.shards.total\' total shards must be greater than 0. Received 0' + name: 'RangeError', + code: 'ERR_OUT_OF_RANGE', + message: 'The value of "options.shards.total" is out of range. It must be >= 1. Received 0' }); }); @@ -202,9 +202,10 @@ describe('require(\'node:test\').run', { concurrency: true }, () => { index: 0 } }), { - name: 'TypeError', - code: 'ERR_INVALID_ARG_VALUE', - message: 'The property \'options.shards.index\' shard index must be greater than 0. Received 0' + name: 'RangeError', + code: 'ERR_OUT_OF_RANGE', + // eslint-disable-next-line max-len + message: 'The value of "options.shards.index" is out of range. It must be >= 1 && <= 6 ("options.shards.total"). Received 0' }); }); @@ -216,13 +217,14 @@ describe('require(\'node:test\').run', { concurrency: true }, () => { index: 7 } }), { - name: 'TypeError', - code: 'ERR_INVALID_ARG_VALUE', - message: 'The property \'options.shards.index\' shard index must be less than total shards. Received 7' + name: 'RangeError', + code: 'ERR_OUT_OF_RANGE', + // eslint-disable-next-line max-len + message: 'The value of "options.shards.index" is out of range. It must be >= 1 && <= 6 ("options.shards.total"). Received 7' }); }); - it('should require watch mode to e disabled when having shards option', () => { + it('should require watch mode to be disabled when having shards option', () => { assert.throws(() => run({ files: shardsTestsFiles, watch: true, From 5e39d75ac9699a7e46c562b82326e64a740740db Mon Sep 17 00:00:00 2001 From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Mon, 3 Jul 2023 12:31:08 +0300 Subject: [PATCH 07/18] allow passing shards in CLI --- lib/internal/main/test_runner.js | 27 +++++++++++++++++++++++++-- src/node_options.cc | 4 ++++ src/node_options.h | 1 + 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/lib/internal/main/test_runner.js b/lib/internal/main/test_runner.js index 5ce9e51e4b6af6..580209d32c2d99 100644 --- a/lib/internal/main/test_runner.js +++ b/lib/internal/main/test_runner.js @@ -7,7 +7,8 @@ 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 { exitCodes: { kGenericUserError } } = internalBinding('errors'); +const { exitCodes: { kGenericUserError }, codes: { ERR_INVALID_ARG_VALUE } } = internalBinding('errors'); +const { NumberParseInt, NumberIsNaN } = primordials; prepareMainThreadExecution(false); markBootstrapComplete(); @@ -22,7 +23,29 @@ if (isUsingInspector()) { inspectPort = process.debugPort; } -run({ concurrency, inspectPort, watch: getOptionValue('--watch'), setup: setupTestReporters }) +let shards; +const shardsOption = getOptionValue('--shards'); +if (shardsOption) { + const { 0: indexStr, 1: totalStr } = shardsOption[0].split('/'); + + const index = NumberParseInt(indexStr); + const total = NumberParseInt(totalStr); + + if (NumberIsNaN(index) || NumberIsNaN(total)) { + throw new ERR_INVALID_ARG_VALUE( + '--shards', + shardsOption, + 'must be in the form of /', + ); + } + + shards = { + index, + total, + }; +} + +run({ concurrency, inspectPort, watch: getOptionValue('--watch'), setup: setupTestReporters, shards }) .once('test:fail', () => { process.exitCode = kGenericUserError; }); diff --git a/src/node_options.cc b/src/node_options.cc index fc6050877eb8b4..a49b89ad6911c4 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -596,6 +596,10 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() { "run tests with 'only' option set", &EnvironmentOptions::test_only, kAllowedInEnvvar); + AddOption("--shards", + "run test specific shard", + &EnvironmentOptions::test_shards, + kAllowedInEnvvar); AddOption("--test-udp-no-try-send", "", // For testing only. &EnvironmentOptions::test_udp_no_try_send); AddOption("--throw-deprecation", diff --git a/src/node_options.h b/src/node_options.h index e0d338f203b0c4..598a32951a5bf2 100644 --- a/src/node_options.h +++ b/src/node_options.h @@ -165,6 +165,7 @@ class EnvironmentOptions : public Options { std::vector test_reporter_destination; bool test_only = false; bool test_udp_no_try_send = false; + std::string test_shards; bool throw_deprecation = false; bool trace_atomics_wait = false; bool trace_deprecation = false; From 5c6ab88b27e1b6a6acfefa45dc8464ad40af5c29 Mon Sep 17 00:00:00 2001 From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Mon, 3 Jul 2023 12:56:38 +0300 Subject: [PATCH 08/18] test_runner: add cli support --- lib/internal/main/test_runner.js | 23 +++++- test/fixtures/test-runner/shards/a.cjs | 2 +- test/fixtures/test-runner/shards/b.cjs | 2 +- test/fixtures/test-runner/shards/c.cjs | 2 +- test/fixtures/test-runner/shards/d.cjs | 2 +- test/fixtures/test-runner/shards/e.cjs | 2 +- test/fixtures/test-runner/shards/f.cjs | 2 +- test/fixtures/test-runner/shards/g.cjs | 2 +- test/fixtures/test-runner/shards/h.cjs | 2 +- test/fixtures/test-runner/shards/i.cjs | 2 +- test/fixtures/test-runner/shards/j.cjs | 2 +- test/parallel/test-runner-cli.js | 104 +++++++++++++++++++++++++ 12 files changed, 135 insertions(+), 12 deletions(-) diff --git a/lib/internal/main/test_runner.js b/lib/internal/main/test_runner.js index 580209d32c2d99..04c7ad7ee63155 100644 --- a/lib/internal/main/test_runner.js +++ b/lib/internal/main/test_runner.js @@ -7,7 +7,12 @@ 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 { exitCodes: { kGenericUserError }, codes: { ERR_INVALID_ARG_VALUE } } = internalBinding('errors'); +const { exitCodes: { kGenericUserError } } = internalBinding('errors'); +const { + codes: { + ERR_INVALID_ARG_VALUE, + }, +} = require('internal/errors'); const { NumberParseInt, NumberIsNaN } = primordials; prepareMainThreadExecution(false); @@ -26,12 +31,26 @@ if (isUsingInspector()) { let shards; const shardsOption = getOptionValue('--shards'); if (shardsOption) { - const { 0: indexStr, 1: totalStr } = shardsOption[0].split('/'); + const shardsParts = shardsOption.split('/'); + + if (shardsParts.length !== 2) { + process.exitCode = kGenericUserError; + + throw new ERR_INVALID_ARG_VALUE( + '--shards', + shardsOption, + 'must be in the form of /', + ); + } + + const { 0: indexStr, 1: totalStr } = shardsParts; const index = NumberParseInt(indexStr); const total = NumberParseInt(totalStr); if (NumberIsNaN(index) || NumberIsNaN(total)) { + process.exitCode = kGenericUserError; + throw new ERR_INVALID_ARG_VALUE( '--shards', shardsOption, diff --git a/test/fixtures/test-runner/shards/a.cjs b/test/fixtures/test-runner/shards/a.cjs index 2a722c504b9fa5..650e61eda80b09 100644 --- a/test/fixtures/test-runner/shards/a.cjs +++ b/test/fixtures/test-runner/shards/a.cjs @@ -1,4 +1,4 @@ 'use strict'; const test = require('node:test'); -test('this should pass'); +test('a.cjs this should pass'); diff --git a/test/fixtures/test-runner/shards/b.cjs b/test/fixtures/test-runner/shards/b.cjs index 2a722c504b9fa5..12677e2505cdcf 100644 --- a/test/fixtures/test-runner/shards/b.cjs +++ b/test/fixtures/test-runner/shards/b.cjs @@ -1,4 +1,4 @@ 'use strict'; const test = require('node:test'); -test('this should pass'); +test('b.cjs this should pass'); diff --git a/test/fixtures/test-runner/shards/c.cjs b/test/fixtures/test-runner/shards/c.cjs index 2a722c504b9fa5..4a4c938a74d44f 100644 --- a/test/fixtures/test-runner/shards/c.cjs +++ b/test/fixtures/test-runner/shards/c.cjs @@ -1,4 +1,4 @@ 'use strict'; const test = require('node:test'); -test('this should pass'); +test('c.cjs this should pass'); diff --git a/test/fixtures/test-runner/shards/d.cjs b/test/fixtures/test-runner/shards/d.cjs index 2a722c504b9fa5..d643276b112801 100644 --- a/test/fixtures/test-runner/shards/d.cjs +++ b/test/fixtures/test-runner/shards/d.cjs @@ -1,4 +1,4 @@ 'use strict'; const test = require('node:test'); -test('this should pass'); +test('d.cjs this should pass'); diff --git a/test/fixtures/test-runner/shards/e.cjs b/test/fixtures/test-runner/shards/e.cjs index 2a722c504b9fa5..e4af3d49fcc335 100644 --- a/test/fixtures/test-runner/shards/e.cjs +++ b/test/fixtures/test-runner/shards/e.cjs @@ -1,4 +1,4 @@ 'use strict'; const test = require('node:test'); -test('this should pass'); +test('e.cjs this should pass'); diff --git a/test/fixtures/test-runner/shards/f.cjs b/test/fixtures/test-runner/shards/f.cjs index 2a722c504b9fa5..4ed5c4eb4ed841 100644 --- a/test/fixtures/test-runner/shards/f.cjs +++ b/test/fixtures/test-runner/shards/f.cjs @@ -1,4 +1,4 @@ 'use strict'; const test = require('node:test'); -test('this should pass'); +test('f.cjs this should pass'); diff --git a/test/fixtures/test-runner/shards/g.cjs b/test/fixtures/test-runner/shards/g.cjs index 2a722c504b9fa5..da8d297d35b4b7 100644 --- a/test/fixtures/test-runner/shards/g.cjs +++ b/test/fixtures/test-runner/shards/g.cjs @@ -1,4 +1,4 @@ 'use strict'; const test = require('node:test'); -test('this should pass'); +test('g.cjs this should pass'); diff --git a/test/fixtures/test-runner/shards/h.cjs b/test/fixtures/test-runner/shards/h.cjs index 2a722c504b9fa5..502250974dbc4b 100644 --- a/test/fixtures/test-runner/shards/h.cjs +++ b/test/fixtures/test-runner/shards/h.cjs @@ -1,4 +1,4 @@ 'use strict'; const test = require('node:test'); -test('this should pass'); +test('h.cjs this should pass'); diff --git a/test/fixtures/test-runner/shards/i.cjs b/test/fixtures/test-runner/shards/i.cjs index 2a722c504b9fa5..42df5e74c9c483 100644 --- a/test/fixtures/test-runner/shards/i.cjs +++ b/test/fixtures/test-runner/shards/i.cjs @@ -1,4 +1,4 @@ 'use strict'; const test = require('node:test'); -test('this should pass'); +test('i.cjs this should pass'); diff --git a/test/fixtures/test-runner/shards/j.cjs b/test/fixtures/test-runner/shards/j.cjs index 2a722c504b9fa5..e2167de43d9b94 100644 --- a/test/fixtures/test-runner/shards/j.cjs +++ b/test/fixtures/test-runner/shards/j.cjs @@ -1,4 +1,4 @@ 'use strict'; const test = require('node:test'); -test('this should pass'); +test('j.cjs this should pass'); diff --git a/test/parallel/test-runner-cli.js b/test/parallel/test-runner-cli.js index 704e72b2df49d6..b42f9b8d495ab2 100644 --- a/test/parallel/test-runner-cli.js +++ b/test/parallel/test-runner-cli.js @@ -197,3 +197,107 @@ const testFixtures = fixtures.path('test-runner'); const stdout = child.stdout.toString(); assert.match(stdout, /ok 1 - this should pass/); } + +{ + // --shards option validation + const args = ['--test', '--shards=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 '--shards' must be in the form of \/\. Received '1'/); + const stdout = child.stdout.toString(); + assert.strictEqual(stdout, ''); +} + +{ + // --shards option validation + const args = ['--test', '--shards=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 '--shards' must be in the form of \/\. Received '1\/2\/3'/); + const stdout = child.stdout.toString(); + assert.strictEqual(stdout, ''); +} + +{ + // --shards option validation + const args = ['--test', '--shards=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 '--shards' must be in the form of \/\. Received 'hello'/); + const stdout = child.stdout.toString(); + assert.strictEqual(stdout, ''); +} + +{ + // --shards option, first shard + const args = [ + '--test', + '--shards=1/2', + join(testFixtures, 'shards/*.cjs'), + ]; + 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/); +} + +{ + // --shards option, last shard + const args = [ + '--test', + '--shards=2/2', + join(testFixtures, 'shards/*.cjs'), + ]; + 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/); +} From d293da548399d7be4479e8fe241204e4cdeab6fa Mon Sep 17 00:00:00 2001 From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Mon, 3 Jul 2023 13:07:02 +0300 Subject: [PATCH 09/18] test_runner: added docs --- doc/api/cli.md | 20 ++++++++++++++++++++ doc/api/test.md | 5 +++++ 2 files changed, 25 insertions(+) diff --git a/doc/api/cli.md b/doc/api/cli.md index 47e03a29763671..46341f8781d93b 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -1500,6 +1500,26 @@ changes: Configures the test runner to only execute top level tests that have the `only` option set. +### `--shards` + + + +Test suite shard to execute in a format of `/`, 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: + +```sh +node --test --shard=1/3 +node --test --shard=2/3 +node --test --shard=3/3 +``` + ### `--throw-deprecation`