diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index ca23a94000c4b8..7e86a79bad8eef 100644 --- a/lib/internal/test_runner/test.js +++ b/lib/internal/test_runner/test.js @@ -777,7 +777,13 @@ class Test extends AsyncResource { loc: [child.line, child.column, child.file], }, noop); t.endTime = t.startTime = hrtime(); - t.start(); + // For suites, Suite.run() starts the subtests via SafePromiseAll. + // Starting them here as well would run them twice, re-invoking the + // synthetic children-creator against a now-incremented disambiguator + // and producing spurious failures. + if (this.reportedType !== 'suite') { + t.start(); + } } }; } diff --git a/test/fixtures/test-runner/rerun.js b/test/fixtures/test-runner/rerun.js index 563e92e4c1bb80..583c7777f9ca15 100644 --- a/test/fixtures/test-runner/rerun.js +++ b/test/fixtures/test-runner/rerun.js @@ -46,3 +46,22 @@ describe('describe rerun', { timeout: 1000, concurrency: 1000 }, () => { }); test('a'); }); + + +// Shared helper creates subtests at the same source location each time it's +// called, producing ambiguous test identifiers (disambiguated with ":(N)" +// suffixes in the rerun state file). Regression coverage for a bug where the +// suite's synthetic rerun fn double-started its direct children, which then +// re-invoked the synthetic descendant-creator against an already-incremented +// disambiguator map and emitted spurious failures. +function ambiguousHelper(t) { + return Promise.all([ + t.test('shared sub A', () => {}), + t.test('shared sub B', () => {}), + ]); +} + +describe('rerun with ambiguous shared helper', { timeout: 1000, concurrency: 1000 }, () => { + test('first caller', (t) => ambiguousHelper(t)); + test('second caller', (t) => ambiguousHelper(t)); +}); diff --git a/test/parallel/test-runner-test-rerun-failures.js b/test/parallel/test-runner-test-rerun-failures.js index 4a9ea5d4ac0bee..585d5f25f04a59 100644 --- a/test/parallel/test-runner-test-rerun-failures.js +++ b/test/parallel/test-runner-test-rerun-failures.js @@ -27,6 +27,13 @@ const expectedStateFile = [ 'test/fixtures/test-runner/rerun.js:44:3': { passed_on_attempt: 0, name: 'passed on first attempt' }, 'test/fixtures/test-runner/rerun.js:47:3': { passed_on_attempt: 0, name: 'a' }, 'test/fixtures/test-runner/rerun.js:43:1': { passed_on_attempt: 0, name: 'describe rerun' }, + 'test/fixtures/test-runner/rerun.js:59:7': { passed_on_attempt: 0, name: 'shared sub A' }, + 'test/fixtures/test-runner/rerun.js:60:7': { passed_on_attempt: 0, name: 'shared sub B' }, + 'test/fixtures/test-runner/rerun.js:65:3': { passed_on_attempt: 0, name: 'first caller' }, + 'test/fixtures/test-runner/rerun.js:59:7:(1)': { passed_on_attempt: 0, name: 'shared sub A' }, + 'test/fixtures/test-runner/rerun.js:60:7:(1)': { passed_on_attempt: 0, name: 'shared sub B' }, + 'test/fixtures/test-runner/rerun.js:66:3': { passed_on_attempt: 0, name: 'second caller' }, + 'test/fixtures/test-runner/rerun.js:64:1': { passed_on_attempt: 0, name: 'rerun with ambiguous shared helper' }, }, { 'test/fixtures/test-runner/rerun.js:9:1': { passed_on_attempt: 0, name: 'ok' }, @@ -38,11 +45,17 @@ const expectedStateFile = [ 'test/fixtures/test-runner/rerun.js:39:1': { passed_on_attempt: 0, name: 'nested ambiguous (expectedAttempts=0)' }, 'test/fixtures/test-runner/rerun.js:30:16:(1)': { passed_on_attempt: 0, name: '2 levels deep' }, 'test/fixtures/test-runner/rerun.js:35:13:(1)': { passed_on_attempt: 0, name: 'ok' }, - 'test/fixtures/test-runner/rerun.js:43:1': { passed_on_attempt: 0, name: 'describe rerun' }, - 'test/fixtures/test-runner/rerun.js:44:3': { passed_on_attempt: 0, name: 'passed on first attempt' }, 'test/fixtures/test-runner/rerun.js:45:13': { passed_on_attempt: 0, name: 'nested' }, - 'test/fixtures/test-runner/rerun.js:45:13:(1)': { passed_on_attempt: 1, name: 'nested' }, + 'test/fixtures/test-runner/rerun.js:44:3': { passed_on_attempt: 0, name: 'passed on first attempt' }, 'test/fixtures/test-runner/rerun.js:47:3': { passed_on_attempt: 0, name: 'a' }, + 'test/fixtures/test-runner/rerun.js:43:1': { passed_on_attempt: 0, name: 'describe rerun' }, + 'test/fixtures/test-runner/rerun.js:59:7': { passed_on_attempt: 0, name: 'shared sub A' }, + 'test/fixtures/test-runner/rerun.js:60:7': { passed_on_attempt: 0, name: 'shared sub B' }, + 'test/fixtures/test-runner/rerun.js:65:3': { passed_on_attempt: 0, name: 'first caller' }, + 'test/fixtures/test-runner/rerun.js:59:7:(1)': { passed_on_attempt: 0, name: 'shared sub A' }, + 'test/fixtures/test-runner/rerun.js:60:7:(1)': { passed_on_attempt: 0, name: 'shared sub B' }, + 'test/fixtures/test-runner/rerun.js:66:3': { passed_on_attempt: 0, name: 'second caller' }, + 'test/fixtures/test-runner/rerun.js:64:1': { passed_on_attempt: 0, name: 'rerun with ambiguous shared helper' }, }, { 'test/fixtures/test-runner/rerun.js:3:1': { passed_on_attempt: 2, name: 'should fail on first two attempts' }, @@ -53,15 +66,21 @@ const expectedStateFile = [ 'test/fixtures/test-runner/rerun.js:29:13': { passed_on_attempt: 0, name: 'nested' }, 'test/fixtures/test-runner/rerun.js:35:13': { passed_on_attempt: 0, name: 'ok' }, 'test/fixtures/test-runner/rerun.js:39:1': { passed_on_attempt: 0, name: 'nested ambiguous (expectedAttempts=0)' }, - 'test/fixtures/test-runner/rerun.js:29:13:(1)': { passed_on_attempt: 2, name: 'nested' }, 'test/fixtures/test-runner/rerun.js:30:16:(1)': { passed_on_attempt: 0, name: '2 levels deep' }, + 'test/fixtures/test-runner/rerun.js:29:13:(1)': { passed_on_attempt: 2, name: 'nested' }, 'test/fixtures/test-runner/rerun.js:35:13:(1)': { passed_on_attempt: 0, name: 'ok' }, 'test/fixtures/test-runner/rerun.js:40:1': { passed_on_attempt: 2, name: 'nested ambiguous (expectedAttempts=1)' }, - 'test/fixtures/test-runner/rerun.js:43:1': { passed_on_attempt: 0, name: 'describe rerun' }, - 'test/fixtures/test-runner/rerun.js:44:3': { passed_on_attempt: 0, name: 'passed on first attempt' }, 'test/fixtures/test-runner/rerun.js:45:13': { passed_on_attempt: 0, name: 'nested' }, - 'test/fixtures/test-runner/rerun.js:45:13:(1)': { passed_on_attempt: 1, name: 'nested' }, + 'test/fixtures/test-runner/rerun.js:44:3': { passed_on_attempt: 0, name: 'passed on first attempt' }, 'test/fixtures/test-runner/rerun.js:47:3': { passed_on_attempt: 0, name: 'a' }, + 'test/fixtures/test-runner/rerun.js:43:1': { passed_on_attempt: 0, name: 'describe rerun' }, + 'test/fixtures/test-runner/rerun.js:59:7': { passed_on_attempt: 0, name: 'shared sub A' }, + 'test/fixtures/test-runner/rerun.js:60:7': { passed_on_attempt: 0, name: 'shared sub B' }, + 'test/fixtures/test-runner/rerun.js:65:3': { passed_on_attempt: 0, name: 'first caller' }, + 'test/fixtures/test-runner/rerun.js:59:7:(1)': { passed_on_attempt: 0, name: 'shared sub A' }, + 'test/fixtures/test-runner/rerun.js:60:7:(1)': { passed_on_attempt: 0, name: 'shared sub B' }, + 'test/fixtures/test-runner/rerun.js:66:3': { passed_on_attempt: 0, name: 'second caller' }, + 'test/fixtures/test-runner/rerun.js:64:1': { passed_on_attempt: 0, name: 'rerun with ambiguous shared helper' }, }, ]; @@ -81,26 +100,26 @@ test('test should pass on third rerun', async () => { let { code, stdout, signal } = await common.spawnPromisified(process.execPath, args); assert.strictEqual(code, 1); assert.strictEqual(signal, null); - assert.match(stdout, /pass 11/); + assert.match(stdout, /pass 17/); assert.match(stdout, /fail 4/); - assert.match(stdout, /suites 1/); + assert.match(stdout, /suites 2/); assert.deepStrictEqual(await getStateFile(), expectedStateFile.slice(0, 1)); ({ code, stdout, signal } = await common.spawnPromisified(process.execPath, args)); assert.strictEqual(code, 1); assert.strictEqual(signal, null); - assert.match(stdout, /pass 13/); + assert.match(stdout, /pass 18/); assert.match(stdout, /fail 3/); - assert.match(stdout, /suites 1/); + assert.match(stdout, /suites 2/); assert.deepStrictEqual(await getStateFile(), expectedStateFile.slice(0, 2)); ({ code, stdout, signal } = await common.spawnPromisified(process.execPath, args)); assert.strictEqual(code, 0); assert.strictEqual(signal, null); - assert.match(stdout, /pass 18/); + assert.match(stdout, /pass 21/); assert.match(stdout, /fail 0/); - assert.match(stdout, /suites 1/); + assert.match(stdout, /suites 2/); assert.deepStrictEqual(await getStateFile(), expectedStateFile); }); @@ -110,32 +129,32 @@ test('test should pass on third rerun with `--test`', async () => { let { code, stdout, signal } = await common.spawnPromisified(process.execPath, args); assert.strictEqual(code, 1); assert.strictEqual(signal, null); - assert.match(stdout, /pass 11/); + assert.match(stdout, /pass 17/); assert.match(stdout, /fail 4/); - assert.match(stdout, /suites 1/); + assert.match(stdout, /suites 2/); assert.deepStrictEqual(await getStateFile(), expectedStateFile.slice(0, 1)); ({ code, stdout, signal } = await common.spawnPromisified(process.execPath, args)); assert.strictEqual(code, 1); assert.strictEqual(signal, null); - assert.match(stdout, /pass 13/); + assert.match(stdout, /pass 18/); assert.match(stdout, /fail 3/); - assert.match(stdout, /suites 1/); + assert.match(stdout, /suites 2/); assert.deepStrictEqual(await getStateFile(), expectedStateFile.slice(0, 2)); ({ code, stdout, signal } = await common.spawnPromisified(process.execPath, args)); assert.strictEqual(code, 0); assert.strictEqual(signal, null); - assert.match(stdout, /pass 18/); + assert.match(stdout, /pass 21/); assert.match(stdout, /fail 0/); - assert.match(stdout, /suites 1/); + assert.match(stdout, /suites 2/); assert.deepStrictEqual(await getStateFile(), expectedStateFile); }); test('using `run` api', async () => { let stream = run({ files: [fixture], rerunFailuresFilePath: stateFile }); - stream.on('test:pass', common.mustCall(12)); + stream.on('test:pass', common.mustCall(19)); stream.on('test:fail', common.mustCall(4)); // eslint-disable-next-line no-unused-vars @@ -145,7 +164,7 @@ test('using `run` api', async () => { stream = run({ files: [fixture], rerunFailuresFilePath: stateFile }); - stream.on('test:pass', common.mustCall(14)); + stream.on('test:pass', common.mustCall(20)); stream.on('test:fail', common.mustCall(3)); // eslint-disable-next-line no-unused-vars @@ -155,7 +174,7 @@ test('using `run` api', async () => { stream = run({ files: [fixture], rerunFailuresFilePath: stateFile }); - stream.on('test:pass', common.mustCall(19)); + stream.on('test:pass', common.mustCall(23)); stream.on('test:fail', common.mustNotCall()); // eslint-disable-next-line no-unused-vars