Skip to content

test_runner: fix nested rerun with t.test #63428

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
MoLow wants to merge 1 commit into from
+89 −87
Closed

test_runner: fix nested rerun with t.test #63428

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 13 additions & 11 deletions lib/internal/test_runner/reporter/rerun.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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,
Expand Down
23 changes: 11 additions & 12 deletions lib/internal/test_runner/test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 = () => {
Expand All @@ -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();
}
Expand Down
18 changes: 18 additions & 0 deletions test/fixtures/test-runner/rerun-cross-parent-subtests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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'));
});
111 changes: 47 additions & 64 deletions test/parallel/test-runner-test-rerun-failures.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)' },
},
];

Expand Down Expand Up @@ -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));
Expand Down
Loading