Skip to content

Commit f5803cc

Browse files
MoLowaduh95
MoLow
authored and
aduh95
committed
test_runner: fix rerun ambiguous test failures
PR-URL: #61392 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
1 parent bacba16 commit f5803cc

File tree

4 files changed

+113
-25
lines changed

4 files changed

+113
-25
lines changed

‎lib/internal/test_runner/reporter/rerun.js‎

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
'use strict';
22

33
const {
4+
ArrayPrototypeMap,
45
ArrayPrototypePush,
56
JSONStringify,
67
} = primordials;
@@ -11,19 +12,55 @@ function reportReruns(previousRuns, globalOptions) {
1112
return async function reporter(source) {
1213
const obj = { __proto__: null };
1314
const disambiguator = { __proto__: null };
15+
let currentSuite = null;
16+
const roots = [];
17+
18+
function getTestId(data) {
19+
return `${relative(globalOptions.cwd, data.file)}:${data.line}:${data.column}`;
20+
}
21+
22+
function startTest(data) {
23+
const originalSuite = currentSuite;
24+
currentSuite = { __proto__: null, data, parent: currentSuite, children: [] };
25+
if (originalSuite?.children) {
26+
ArrayPrototypePush(originalSuite.children, currentSuite);
27+
}
28+
if (!currentSuite.parent) {
29+
ArrayPrototypePush(roots, currentSuite);
30+
}
31+
}
1432

1533
for await (const { type, data } of source) {
34+
let currentTest;
35+
if (type === 'test:start') {
36+
startTest(data);
37+
} else if (type === 'test:fail' || type === 'test:pass') {
38+
if (!currentSuite) {
39+
startTest({ __proto__: null, name: 'root', nesting: 0 });
40+
}
41+
if (currentSuite.data.name !== data.name || currentSuite.data.nesting !== data.nesting) {
42+
startTest(data);
43+
}
44+
currentTest = currentSuite;
45+
if (currentSuite?.data.nesting === data.nesting) {
46+
currentSuite = currentSuite.parent;
47+
}
48+
}
49+
50+
1651
if (type === 'test:pass') {
17-
let identifier = `${relative(globalOptions.cwd, data.file)}:${data.line}:${data.column}`;
52+
let identifier = getTestId(data);
1853
if (disambiguator[identifier] !== undefined) {
1954
identifier += `:(${disambiguator[identifier]})`;
2055
disambiguator[identifier] += 1;
2156
} else {
2257
disambiguator[identifier] = 1;
2358
}
59+
const children = ArrayPrototypeMap(currentTest.children, (child) => child.data);
2460
obj[identifier] = {
2561
__proto__: null,
2662
name: data.name,
63+
children,
2764
passed_on_attempt: data.details.passed_on_attempt ?? data.details.attempt,
2865
};
2966
}

‎lib/internal/test_runner/test.js‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -699,10 +699,18 @@ class Test extends AsyncResource {
699699
this.root.testDisambiguator.set(testIdentifier, 1);
700700
}
701701
this.attempt = this.root.harness.previousRuns.length;
702-
const previousAttempt = this.root.harness.previousRuns[this.attempt - 1]?.[testIdentifier]?.passed_on_attempt;
702+
const previousAttempt = this.root.harness.previousRuns[this.attempt - 1]?.[testIdentifier];
703703
if (previousAttempt != null) {
704-
this.passedAttempt = previousAttempt;
705-
this.fn = noop;
704+
this.passedAttempt = previousAttempt.passed_on_attempt;
705+
this.fn = () => {
706+
for (let i = 0; i < (previousAttempt.children?.length ?? 0); i++) {
707+
const child = previousAttempt.children[i];
708+
this.createSubtest(Test, child.name, { __proto__: null }, noop, {
709+
__proto__: null,
710+
loc: [child.line, child.column, child.file],
711+
}, noop).start();
712+
}
713+
};
706714
}
707715
}
708716
}

‎test/fixtures/test-runner/rerun.js‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,4 +22,19 @@ function ambiguousTest(expectedAttempts) {
2222
}
2323

2424
ambiguousTest(0);
25-
ambiguousTest(1);
25+
ambiguousTest(1);
26+
27+
function nestedAmbiguousTest(expectedAttempts) {
28+
return async (t) => {
29+
await t.test('nested', async (tt) => {
30+
await tt.test('2 levels deep', () => {});
31+
if (t.attempt < expectedAttempts) {
32+
throw new Error(`This test is expected to fail on the first ${expectedAttempts} attempts`);
33+
}
34+
});
35+
await t.test('ok', () => {});
36+
};
37+
}
38+
39+
test('nested ambiguous (expectedAttempts=0)', nestedAmbiguousTest(0));
40+
test('nested ambiguous (expectedAttempts=1)', nestedAmbiguousTest(2));

‎test/parallel/test-runner-test-rerun-failures.js‎

Lines changed: 48 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -15,46 +15,74 @@ afterEach(() => rm(stateFile, { force: true }));
1515

1616
const expectedStateFile = [
1717
{
18-
'test/fixtures/test-runner/rerun.js:17:3': { passed_on_attempt: 0, name: 'ambiguous (expectedAttempts=0)' },
1918
'test/fixtures/test-runner/rerun.js:9:1': { passed_on_attempt: 0, name: 'ok' },
19+
'test/fixtures/test-runner/rerun.js:17:3': { passed_on_attempt: 0, name: 'ambiguous (expectedAttempts=0)' },
20+
'test/fixtures/test-runner/rerun.js:30:16': { passed_on_attempt: 0, name: '2 levels deep' },
21+
'test/fixtures/test-runner/rerun.js:29:13': { passed_on_attempt: 0, name: 'nested' },
22+
'test/fixtures/test-runner/rerun.js:35:13': { passed_on_attempt: 0, name: 'ok' },
23+
'test/fixtures/test-runner/rerun.js:39:1': { passed_on_attempt: 0, name: 'nested ambiguous (expectedAttempts=0)' },
24+
'test/fixtures/test-runner/rerun.js:30:16:(1)': { passed_on_attempt: 0, name: '2 levels deep' },
25+
'test/fixtures/test-runner/rerun.js:35:13:(1)': { passed_on_attempt: 0, name: 'ok' },
2026
},
2127
{
28+
'test/fixtures/test-runner/rerun.js:9:1': { passed_on_attempt: 0, name: 'ok' },
2229
'test/fixtures/test-runner/rerun.js:17:3': { passed_on_attempt: 0, name: 'ambiguous (expectedAttempts=0)' },
2330
'test/fixtures/test-runner/rerun.js:17:3:(1)': { passed_on_attempt: 1, name: 'ambiguous (expectedAttempts=1)' },
24-
'test/fixtures/test-runner/rerun.js:9:1': { passed_on_attempt: 0, name: 'ok' },
31+
'test/fixtures/test-runner/rerun.js:30:16': { passed_on_attempt: 0, name: '2 levels deep' },
32+
'test/fixtures/test-runner/rerun.js:29:13': { passed_on_attempt: 0, name: 'nested' },
33+
'test/fixtures/test-runner/rerun.js:35:13': { passed_on_attempt: 0, name: 'ok' },
34+
'test/fixtures/test-runner/rerun.js:39:1': { passed_on_attempt: 0, name: 'nested ambiguous (expectedAttempts=0)' },
35+
'test/fixtures/test-runner/rerun.js:30:16:(1)': { passed_on_attempt: 0, name: '2 levels deep' },
36+
'test/fixtures/test-runner/rerun.js:35:13:(1)': { passed_on_attempt: 0, name: 'ok' },
2537
},
2638
{
39+
'test/fixtures/test-runner/rerun.js:3:1': { passed_on_attempt: 2, name: 'should fail on first two attempts' },
40+
'test/fixtures/test-runner/rerun.js:9:1': { passed_on_attempt: 0, name: 'ok' },
2741
'test/fixtures/test-runner/rerun.js:17:3': { passed_on_attempt: 0, name: 'ambiguous (expectedAttempts=0)' },
2842
'test/fixtures/test-runner/rerun.js:17:3:(1)': { passed_on_attempt: 1, name: 'ambiguous (expectedAttempts=1)' },
29-
'test/fixtures/test-runner/rerun.js:9:1': { passed_on_attempt: 0, name: 'ok' },
30-
'test/fixtures/test-runner/rerun.js:3:1': { passed_on_attempt: 2, name: 'should fail on first two attempts' },
43+
'test/fixtures/test-runner/rerun.js:30:16': { passed_on_attempt: 0, name: '2 levels deep' },
44+
'test/fixtures/test-runner/rerun.js:29:13': { passed_on_attempt: 0, name: 'nested' },
45+
'test/fixtures/test-runner/rerun.js:35:13': { passed_on_attempt: 0, name: 'ok' },
46+
'test/fixtures/test-runner/rerun.js:39:1': { passed_on_attempt: 0, name: 'nested ambiguous (expectedAttempts=0)' },
47+
'test/fixtures/test-runner/rerun.js:29:13:(1)': { passed_on_attempt: 2, name: 'nested' },
48+
'test/fixtures/test-runner/rerun.js:30:16:(1)': { passed_on_attempt: 0, name: '2 levels deep' },
49+
'test/fixtures/test-runner/rerun.js:35:13:(1)': { passed_on_attempt: 0, name: 'ok' },
50+
'test/fixtures/test-runner/rerun.js:40:1': { passed_on_attempt: 2, name: 'nested ambiguous (expectedAttempts=1)' },
3151
},
3252
];
3353

34-
const getStateFile = async () => JSON.parse((await readFile(stateFile, 'utf8')).replaceAll('\\\\', '/'));
54+
const getStateFile = async () => {
55+
const res = JSON.parse((await readFile(stateFile, 'utf8')).replaceAll('\\\\', '/'));
56+
res.forEach((entry) => {
57+
for (const item in entry) {
58+
delete entry[item].children;
59+
}
60+
});
61+
return res;
62+
};
3563

3664
test('test should pass on third rerun', async () => {
3765
const args = ['--test-rerun-failures', stateFile, fixture];
3866

3967
let { code, stdout, signal } = await common.spawnPromisified(process.execPath, args);
4068
assert.strictEqual(code, 1);
4169
assert.strictEqual(signal, null);
42-
assert.match(stdout, /pass 2/);
43-
assert.match(stdout, /fail 2/);
70+
assert.match(stdout, /pass 8/);
71+
assert.match(stdout, /fail 4/);
4472
assert.deepStrictEqual(await getStateFile(), expectedStateFile.slice(0, 1));
4573

4674
({ code, stdout, signal } = await common.spawnPromisified(process.execPath, args));
4775
assert.strictEqual(code, 1);
4876
assert.strictEqual(signal, null);
49-
assert.match(stdout, /pass 3/);
50-
assert.match(stdout, /fail 1/);
77+
assert.match(stdout, /pass 9/);
78+
assert.match(stdout, /fail 3/);
5179
assert.deepStrictEqual(await getStateFile(), expectedStateFile.slice(0, 2));
5280

5381

5482
({ code, stdout, signal } = await common.spawnPromisified(process.execPath, args));
5583
assert.strictEqual(code, 0);
5684
assert.strictEqual(signal, null);
57-
assert.match(stdout, /pass 4/);
85+
assert.match(stdout, /pass 12/);
5886
assert.match(stdout, /fail 0/);
5987
assert.deepStrictEqual(await getStateFile(), expectedStateFile);
6088
});
@@ -65,30 +93,30 @@ test('test should pass on third rerun with `--test`', async () => {
6593
let { code, stdout, signal } = await common.spawnPromisified(process.execPath, args);
6694
assert.strictEqual(code, 1);
6795
assert.strictEqual(signal, null);
68-
assert.match(stdout, /pass 2/);
69-
assert.match(stdout, /fail 2/);
96+
assert.match(stdout, /pass 8/);
97+
assert.match(stdout, /fail 4/);
7098
assert.deepStrictEqual(await getStateFile(), expectedStateFile.slice(0, 1));
7199

72100
({ code, stdout, signal } = await common.spawnPromisified(process.execPath, args));
73101
assert.strictEqual(code, 1);
74102
assert.strictEqual(signal, null);
75-
assert.match(stdout, /pass 3/);
76-
assert.match(stdout, /fail 1/);
103+
assert.match(stdout, /pass 9/);
104+
assert.match(stdout, /fail 3/);
77105
assert.deepStrictEqual(await getStateFile(), expectedStateFile.slice(0, 2));
78106

79107

80108
({ code, stdout, signal } = await common.spawnPromisified(process.execPath, args));
81109
assert.strictEqual(code, 0);
82110
assert.strictEqual(signal, null);
83-
assert.match(stdout, /pass 4/);
111+
assert.match(stdout, /pass 12/);
84112
assert.match(stdout, /fail 0/);
85113
assert.deepStrictEqual(await getStateFile(), expectedStateFile);
86114
});
87115

88116
test('using `run` api', async () => {
89117
let stream = run({ files: [fixture], rerunFailuresFilePath: stateFile });
90-
stream.on('test:pass', common.mustCall(2));
91-
stream.on('test:fail', common.mustCall(2));
118+
stream.on('test:pass', common.mustCall(8));
119+
stream.on('test:fail', common.mustCall(4));
92120

93121
// eslint-disable-next-line no-unused-vars
94122
for await (const _ of stream);
@@ -97,8 +125,8 @@ test('using `run` api', async () => {
97125

98126

99127
stream = run({ files: [fixture], rerunFailuresFilePath: stateFile });
100-
stream.on('test:pass', common.mustCall(3));
101-
stream.on('test:fail', common.mustCall(1));
128+
stream.on('test:pass', common.mustCall(9));
129+
stream.on('test:fail', common.mustCall(3));
102130

103131
// eslint-disable-next-line no-unused-vars
104132
for await (const _ of stream);
@@ -107,7 +135,7 @@ test('using `run` api', async () => {
107135

108136

109137
stream = run({ files: [fixture], rerunFailuresFilePath: stateFile });
110-
stream.on('test:pass', common.mustCall(4));
138+
stream.on('test:pass', common.mustCall(12));
111139
stream.on('test:fail', common.mustNotCall());
112140

113141
// eslint-disable-next-line no-unused-vars

0 commit comments

Comments
 (0)