diff --git a/lib/internal/test_runner/reporter/rerun.js b/lib/internal/test_runner/reporter/rerun.js index 9658a7fb70ff0b..46ee19fecde856 100644 --- a/lib/internal/test_runner/reporter/rerun.js +++ b/lib/internal/test_runner/reporter/rerun.js @@ -11,17 +11,26 @@ const { writeFileSync } = require('fs'); function reportReruns(previousRuns, globalOptions) { return async function reporter(source) { const obj = { __proto__: null }; - const disambiguator = { __proto__: null }; + const rootDisambiguator = { __proto__: null }; let currentSuite = null; const roots = []; - function getTestId(data) { + function getTestLoc(data) { return `${relative(globalOptions.cwd, data.file)}:${data.line}:${data.column}`; } function startTest(data) { const originalSuite = currentSuite; - currentSuite = { __proto__: null, data, parent: currentSuite, children: [] }; + const childLoc = getTestLoc(data); + const parentDisambig = originalSuite ? + (originalSuite.disambiguator ??= { __proto__: null }) : + rootDisambiguator; + const n = parentDisambig[childLoc] ?? 0; + parentDisambig[childLoc] = n + 1; + const localId = n === 0 ? childLoc : `${childLoc}:(${n})`; + const parentRerunId = originalSuite?.rerunId ?? ''; + const rerunId = parentRerunId === '' ? localId : `${parentRerunId}/${localId}`; + currentSuite = { __proto__: null, data, parent: originalSuite, children: [], rerunId }; if (originalSuite?.children) { ArrayPrototypePush(originalSuite.children, currentSuite); } @@ -49,15 +58,8 @@ function reportReruns(previousRuns, globalOptions) { if (type === 'test:pass') { - let identifier = getTestId(data); - if (disambiguator[identifier] !== undefined) { - identifier += `:(${disambiguator[identifier]})`; - disambiguator[identifier] += 1; - } else { - disambiguator[identifier] = 1; - } const children = ArrayPrototypeMap(currentTest.children, (child) => child.data); - obj[identifier] = { + obj[currentTest.rerunId] = { __proto__: null, name: data.name, children, diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index eb888905798bcd..fec27401b33999 100644 --- a/lib/internal/test_runner/test.js +++ b/lib/internal/test_runner/test.js @@ -614,7 +614,6 @@ class Test extends AsyncResource { this.childNumber = 0; this.timeout = kDefaultTimeout; this.entryFile = entryFile; - this.testDisambiguator = new SafeMap(); this.nextTestId = 1; this.testId = 0; } else { @@ -791,16 +790,16 @@ class Test extends AsyncResource { } if (this.loc != null && this.root.harness.previousRuns != null) { - let testIdentifier = `${relative(this.config.cwd, this.loc.file)}:${this.loc.line}:${this.loc.column}`; - const disambiguator = this.root.testDisambiguator.get(testIdentifier); - if (disambiguator !== undefined) { - testIdentifier += `:(${disambiguator})`; - this.root.testDisambiguator.set(testIdentifier, disambiguator + 1); - } else { - this.root.testDisambiguator.set(testIdentifier, 1); - } + const childLoc = `${relative(this.config.cwd, this.loc.file)}:${this.loc.line}:${this.loc.column}`; + const siblings = this.parent.rerunChildren ??= new SafeMap(); + const n = siblings.get(childLoc) ?? 0; + siblings.set(childLoc, n + 1); + const localId = n === 0 ? childLoc : `${childLoc}:(${n})`; + const parentId = this.parent.rerunId ?? ''; + this.rerunId = parentId === '' ? localId : `${parentId}/${localId}`; + this.attempt = this.root.harness.previousRuns.length; - const previousAttempt = this.root.harness.previousRuns[this.attempt - 1]?.[testIdentifier]; + const previousAttempt = this.root.harness.previousRuns[this.attempt - 1]?.[this.rerunId]; if (previousAttempt != null) { this.passedAttempt = previousAttempt.passed_on_attempt; this.fn = () => { @@ -813,8 +812,8 @@ class Test extends AsyncResource { t.endTime = t.startTime = hrtime(); // 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. + // synthetic children-creator against an already-incremented + // rerunChildren counter and producing spurious failures. if (this.reportedType !== 'suite') { t.start(); } diff --git a/test/fixtures/test-runner/rerun-cross-parent-subtests.js b/test/fixtures/test-runner/rerun-cross-parent-subtests.js new file mode 100644 index 00000000000000..3e24700ce202a8 --- /dev/null +++ b/test/fixtures/test-runner/rerun-cross-parent-subtests.js @@ -0,0 +1,18 @@ +'use strict'; + +const { describe, it } = require('node:test'); +const assert = require('node:assert'); + +function makeSuite(shouldPass, label) { + return async (t) => { + await t.test('inner', async () => { + if (!shouldPass) assert.fail(`${label} should fail`); + }); + }; +} + +describe('parents', { concurrency: false }, () => { + it('A passes', makeSuite(true, 'A')); + it('B fails', makeSuite(false, 'B')); + it('C passes', makeSuite(true, 'C')); +}); diff --git a/test/parallel/test-runner-test-rerun-failures.js b/test/parallel/test-runner-test-rerun-failures.js index 585d5f25f04a59..f29957fd7205f8 100644 --- a/test/parallel/test-runner-test-rerun-failures.js +++ b/test/parallel/test-runner-test-rerun-failures.js @@ -13,74 +13,42 @@ const stateFile = fixtures.path('test-runner', 'rerun-state.json'); beforeEach(() => rm(stateFile, { force: true })); afterEach(() => rm(stateFile, { force: true })); +const F = 'test/fixtures/test-runner/rerun.js'; + +const passOnceState = { + [`${F}:9:1`]: { passed_on_attempt: 0, name: 'ok' }, + [`${F}:17:3`]: { passed_on_attempt: 0, name: 'ambiguous (expectedAttempts=0)' }, + [`${F}:39:1/${F}:29:13/${F}:30:16`]: { passed_on_attempt: 0, name: '2 levels deep' }, + [`${F}:39:1/${F}:29:13`]: { passed_on_attempt: 0, name: 'nested' }, + [`${F}:39:1/${F}:35:13`]: { passed_on_attempt: 0, name: 'ok' }, + [`${F}:39:1`]: { passed_on_attempt: 0, name: 'nested ambiguous (expectedAttempts=0)' }, + [`${F}:40:1/${F}:29:13/${F}:30:16`]: { passed_on_attempt: 0, name: '2 levels deep' }, + [`${F}:40:1/${F}:35:13`]: { passed_on_attempt: 0, name: 'ok' }, + [`${F}:43:1/${F}:44:3/${F}:45:13`]: { passed_on_attempt: 0, name: 'nested' }, + [`${F}:43:1/${F}:44:3`]: { passed_on_attempt: 0, name: 'passed on first attempt' }, + [`${F}:43:1/${F}:47:3`]: { passed_on_attempt: 0, name: 'a' }, + [`${F}:43:1`]: { passed_on_attempt: 0, name: 'describe rerun' }, + [`${F}:64:1/${F}:65:3/${F}:59:7`]: { passed_on_attempt: 0, name: 'shared sub A' }, + [`${F}:64:1/${F}:65:3/${F}:60:7`]: { passed_on_attempt: 0, name: 'shared sub B' }, + [`${F}:64:1/${F}:65:3`]: { passed_on_attempt: 0, name: 'first caller' }, + [`${F}:64:1/${F}:66:3/${F}:59:7`]: { passed_on_attempt: 0, name: 'shared sub A' }, + [`${F}:64:1/${F}:66:3/${F}:60:7`]: { passed_on_attempt: 0, name: 'shared sub B' }, + [`${F}:64:1/${F}:66:3`]: { passed_on_attempt: 0, name: 'second caller' }, + [`${F}:64:1`]: { passed_on_attempt: 0, name: 'rerun with ambiguous shared helper' }, +}; + const expectedStateFile = [ + { ...passOnceState }, { - 'test/fixtures/test-runner/rerun.js:9:1': { passed_on_attempt: 0, name: 'ok' }, - 'test/fixtures/test-runner/rerun.js:17:3': { passed_on_attempt: 0, name: 'ambiguous (expectedAttempts=0)' }, - 'test/fixtures/test-runner/rerun.js:30:16': { passed_on_attempt: 0, name: '2 levels deep' }, - '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: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:45:13': { passed_on_attempt: 0, 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:9:1': { passed_on_attempt: 0, name: 'ok' }, - 'test/fixtures/test-runner/rerun.js:17:3': { passed_on_attempt: 0, name: 'ambiguous (expectedAttempts=0)' }, - 'test/fixtures/test-runner/rerun.js:17:3:(1)': { passed_on_attempt: 1, name: 'ambiguous (expectedAttempts=1)' }, - 'test/fixtures/test-runner/rerun.js:30:16': { passed_on_attempt: 0, name: '2 levels deep' }, - '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: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:45:13': { passed_on_attempt: 0, 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' }, + ...passOnceState, + [`${F}:17:3:(1)`]: { passed_on_attempt: 1, name: 'ambiguous (expectedAttempts=1)' }, }, { - 'test/fixtures/test-runner/rerun.js:3:1': { passed_on_attempt: 2, name: 'should fail on first two attempts' }, - 'test/fixtures/test-runner/rerun.js:9:1': { passed_on_attempt: 0, name: 'ok' }, - 'test/fixtures/test-runner/rerun.js:17:3': { passed_on_attempt: 0, name: 'ambiguous (expectedAttempts=0)' }, - 'test/fixtures/test-runner/rerun.js:17:3:(1)': { passed_on_attempt: 1, name: 'ambiguous (expectedAttempts=1)' }, - 'test/fixtures/test-runner/rerun.js:30:16': { passed_on_attempt: 0, name: '2 levels deep' }, - '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: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:45:13': { passed_on_attempt: 0, 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' }, + ...passOnceState, + [`${F}:3:1`]: { passed_on_attempt: 2, name: 'should fail on first two attempts' }, + [`${F}:17:3:(1)`]: { passed_on_attempt: 1, name: 'ambiguous (expectedAttempts=1)' }, + [`${F}:40:1/${F}:29:13`]: { passed_on_attempt: 2, name: 'nested' }, + [`${F}:40:1`]: { passed_on_attempt: 2, name: 'nested ambiguous (expectedAttempts=1)' }, }, ]; @@ -152,6 +120,21 @@ test('test should pass on third rerun with `--test`', async () => { assert.deepStrictEqual(await getStateFile(), expectedStateFile); }); +test('failures from shared-location subtests are not swallowed on retry', async () => { + const crossParentFixture = fixtures.path('test-runner', 'rerun-cross-parent-subtests.js'); + const args = ['--test-rerun-failures', stateFile, crossParentFixture]; + + let { code, stdout, signal } = await common.spawnPromisified(process.execPath, args); + assert.strictEqual(signal, null); + assert.strictEqual(code, 1); + assert.match(stdout, /fail 2/); + + ({ code, stdout, signal } = await common.spawnPromisified(process.execPath, args)); + assert.strictEqual(signal, null); + assert.notStrictEqual(code, 0); + assert.doesNotMatch(stdout, /fail 0/, 'B fails -> inner must not be silently passed'); +}); + test('using `run` api', async () => { let stream = run({ files: [fixture], rerunFailuresFilePath: stateFile }); stream.on('test:pass', common.mustCall(19));