Skip to content

Commit 7a86108

Browse files
MoLowaduh95
authored andcommitted
test_runner: fix failing suite hooks when marked with todo
Signed-off-by: Moshe Atlow <moshe@atlow.co.il> PR-URL: #63097 Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Aviv Keller <me@aviv.sh> Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com>
1 parent 94ac62a commit 7a86108

3 files changed

Lines changed: 35 additions & 1 deletion

File tree

lib/internal/test_runner/utils.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -451,7 +451,7 @@ function countCompletedTest(test, harness = test.root.harness) {
451451
}
452452
if (test.reportedType === 'suite') {
453453
harness.counters.suites++;
454-
if (!test.passed) {
454+
if (!test.passed && !test.isTodo) {
455455
harness.success = false;
456456
}
457457
return;
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
import { before, describe, it } from 'node:test';
2+
3+
describe('todo suite with failing before hook', { todo: 'evaluating' }, () => {
4+
before(() => {
5+
throw new Error('simulated cleanup failure');
6+
});
7+
8+
it('child 1', () => {});
9+
it('child 2', () => {});
10+
});
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
'use strict';
2+
require('../common');
3+
const assert = require('assert');
4+
const { spawnSync } = require('child_process');
5+
const fixtures = require('../common/fixtures');
6+
7+
// A `before` hook failure inside a suite marked `todo` must not fail the run.
8+
// The `todo` flag on a suite signals that its results are advisory, the same
9+
// way it does on individual tests. Before this fix, the suite branch of
10+
// `countCompletedTest` flipped `harness.success` for any non-passing suite
11+
// regardless of `isTodo`, so a hook failure exited with code 1 even though
12+
// no failing tests were reported.
13+
const child = spawnSync(process.execPath, [
14+
'--test',
15+
'--test-reporter=tap',
16+
fixtures.path('test-runner', 'todo-suite-failing-hook.mjs'),
17+
]);
18+
19+
const stdout = child.stdout.toString();
20+
assert.strictEqual(child.signal, null);
21+
assert.strictEqual(child.status, 0,
22+
`expected exit 0, got ${child.status}\nstdout:\n${stdout}\nstderr:\n${child.stderr}`);
23+
assert.match(stdout, /# fail 0/);
24+
assert.match(stdout, /# todo 2/);

0 commit comments

Comments
 (0)