Skip to content

Commit 7309ddf

Browse files
barakorrichardlau
authored andcommitted
test_runner: rerun failing todo tests in suites
A failing todo subtest does not fail its parent, so the parent suite was recorded as passed in the rerun state file along with its children. On the next run the suite was replayed synthetically and the failing todo subtest was marked as passed instead of being rerun. Do not record a passed test whose descendant failed, so the suite body runs again while passing siblings are still replayed from their own entries. Document that a failing todo test counts as not yet passed. Signed-off-by: Barak <[email protected]> PR-URL: #66605 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Chemi Atlow <[email protected]>
1 parent adef722 commit 7309ddf

4 files changed

Lines changed: 105 additions & 2 deletions

File tree

‎doc/api/test.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,8 @@ in this example, there are two run attempts, with two tests defined in `test.js`
160160
the first test succeeded on the first attempt, and the second test succeeded on the second attempt.
161161

162162
When the `--test-rerun-failures` option is used, the test runner will only run tests that have not yet passed.
163+
A `todo` test that fails counts as not yet passed and is rerun, even though its failure does not
164+
cause the test run to fail.
163165

164166
```bash
165167
node --test-rerun-failures /path/to/state/file

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

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,13 @@ function reportReruns(previousRuns, globalOptions) {
2121

2222
function startTest(data) {
2323
const originalSuite = currentSuite;
24-
currentSuite = { __proto__: null, data, parent: currentSuite, children: [] };
24+
currentSuite = {
25+
__proto__: null,
26+
data,
27+
parent: currentSuite,
28+
children: [],
29+
hasFailedDescendant: false,
30+
};
2531
if (originalSuite?.children) {
2632
ArrayPrototypePush(originalSuite.children, currentSuite);
2733
}
@@ -57,7 +63,11 @@ function reportReruns(previousRuns, globalOptions) {
5763
} else {
5864
disambiguator[baseIdentifier] = 1;
5965
}
60-
if (type === 'test:pass') {
66+
if (type === 'test:fail') {
67+
for (let t = currentTest.parent; t !== null; t = t.parent) {
68+
t.hasFailedDescendant = true;
69+
}
70+
} else if (!currentTest.hasFailedDescendant) {
6171
const children = ArrayPrototypeMap(currentTest.children, (child) => child.data);
6272
obj[identifier] = {
6373
__proto__: null,
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
const { test, describe } = require('node:test');
2+
3+
test('top-level todo fails on first attempt', { todo: true }, ({ attempt }) => {
4+
if (attempt < 1) {
5+
throw new Error('This todo test is expected to fail on the first attempt');
6+
}
7+
});
8+
9+
describe('suite with failing todo', () => {
10+
test('nested todo fails on first attempt', { todo: true }, ({ attempt }) => {
11+
if (attempt < 1) {
12+
throw new Error('This todo test is expected to fail on the first attempt');
13+
}
14+
});
15+
16+
test('ok', ({ attempt }) => {
17+
if (attempt > 0) {
18+
throw new Error('Test should not rerun once it has passed');
19+
}
20+
});
21+
});

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

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,3 +283,73 @@ test('using `run` api', async () => {
283283
await setTimeout(common.platformTimeout(10)); // Wait for the stream to finish processing
284284
assert.deepStrictEqual(await getStateFile(), expectedStateFile);
285285
});
286+
287+
test('failing todo test is rerun even when its parent suite passed', async () => {
288+
const fixturePath = fixtures.path('test-runner', 'rerun-todo.js');
289+
const fixtureKey = relative(process.cwd(), fixturePath).replaceAll('\\', '/');
290+
const topLevelTodo = `${fixtureKey}:3:1`;
291+
const suite = `${fixtureKey}:9:1`;
292+
const nestedTodo = `${fixtureKey}:10:3`;
293+
const ok = `${fixtureKey}:16:3`;
294+
295+
async function runAttempt() {
296+
const results = { __proto__: null, passed: [], failed: [], summaries: [] };
297+
const stream = run({ files: [fixturePath], rerunFailuresFilePath: stateFile });
298+
for await (const { type, data } of stream) {
299+
const { name, todo, details } = data;
300+
if (type === 'test:pass') {
301+
results.passed.push({ name, todo, attempt: details.attempt, passed_on_attempt: details.passed_on_attempt });
302+
} else if (type === 'test:fail') {
303+
results.failed.push({ name, todo, attempt: details.attempt });
304+
} else if (type === 'test:summary') {
305+
results.summaries.push(data);
306+
}
307+
}
308+
await setTimeout(common.platformTimeout(10)); // Wait for the stream to finish processing
309+
return results;
310+
}
311+
312+
let results = await runAttempt();
313+
assert.deepStrictEqual(results.failed, [
314+
{ name: 'top-level todo fails on first attempt', todo: true, attempt: 0 },
315+
{ name: 'nested todo fails on first attempt', todo: true, attempt: 0 },
316+
]);
317+
assert.deepStrictEqual(results.passed, [
318+
{ name: 'ok', todo: undefined, attempt: 0, passed_on_attempt: undefined },
319+
{ name: 'suite with failing todo', todo: undefined, attempt: 0, passed_on_attempt: undefined },
320+
]);
321+
for (const { success, counts } of results.summaries) {
322+
assert.strictEqual(success, true);
323+
assert.strictEqual(counts.failed, 0);
324+
assert.strictEqual(counts.passed, 1);
325+
assert.strictEqual(counts.todo, 2);
326+
}
327+
let state = await getStateFile();
328+
assert.strictEqual(state.length, 1);
329+
assert.deepStrictEqual(state[0], {
330+
[ok]: { passed_on_attempt: 0, name: 'ok' },
331+
});
332+
333+
results = await runAttempt();
334+
assert.deepStrictEqual(results.failed, []);
335+
assert.deepStrictEqual(results.passed, [
336+
{ name: 'top-level todo fails on first attempt', todo: true, attempt: 1, passed_on_attempt: undefined },
337+
{ name: 'nested todo fails on first attempt', todo: true, attempt: 1, passed_on_attempt: undefined },
338+
{ name: 'ok', todo: undefined, attempt: 1, passed_on_attempt: 0 },
339+
{ name: 'suite with failing todo', todo: undefined, attempt: 1, passed_on_attempt: undefined },
340+
]);
341+
for (const { success, counts } of results.summaries) {
342+
assert.strictEqual(success, true);
343+
assert.strictEqual(counts.failed, 0);
344+
assert.strictEqual(counts.passed, 1);
345+
assert.strictEqual(counts.todo, 2);
346+
}
347+
state = await getStateFile();
348+
assert.strictEqual(state.length, 2);
349+
assert.deepStrictEqual(state[1], {
350+
[topLevelTodo]: { passed_on_attempt: 1, name: 'top-level todo fails on first attempt' },
351+
[nestedTodo]: { passed_on_attempt: 1, name: 'nested todo fails on first attempt' },
352+
[ok]: { passed_on_attempt: 0, name: 'ok' },
353+
[suite]: { passed_on_attempt: 1, name: 'suite with failing todo' },
354+
});
355+
});

0 commit comments

Comments
 (0)