Skip to content

Commit

Permalink
test_runner: report failures in filtered suites
Browse files Browse the repository at this point in the history
This commit updates the test runner's filter logic to handle
test suite failures during the build phase. Prior to this commit,
these suites were silently filtered.

PR-URL: #54387
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Jake Yuesong Li <jake.yuesong@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
  • Loading branch information
cjihrig authored and RafaelGSS committed Aug 21, 2024
1 parent 9f115ba commit e70711e
Show file tree
Hide file tree
Showing 4 changed files with 80 additions and 3 deletions.
6 changes: 3 additions & 3 deletions lib/internal/test_runner/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1181,15 +1181,15 @@ class Suite extends Test {
);
} catch (err) {
this.fail(new ERR_TEST_FAILURE(err, kTestCodeFailure));

this.buildPhaseFinished = true;
this.postBuild();
}
this.fn = noop;
}

postBuild() {
this.buildPhaseFinished = true;
if (this.filtered && this.filteredSubtestCount !== this.subtests.length) {
if (this.filtered &&
(this.filteredSubtestCount !== this.subtests.length || this.error)) {
// A suite can transition from filtered to unfiltered based on the
// tests that it contains - in case of children matching patterns.
this.filtered = false;
Expand Down
14 changes: 14 additions & 0 deletions test/fixtures/test-runner/output/filtered-suite-throws.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Flags: --test-name-pattern=enabled
'use strict';
const common = require('../../../common');
const { suite, test } = require('node:test');

suite('suite 1', () => {
throw new Error('boom 1');
test('enabled - should not run', common.mustNotCall());
});

suite('suite 2', () => {
test('enabled - should get cancelled', common.mustNotCall());
throw new Error('boom 1');
});
62 changes: 62 additions & 0 deletions test/fixtures/test-runner/output/filtered-suite-throws.snapshot
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
TAP version 13
# Subtest: suite 1
not ok 1 - suite 1
---
duration_ms: *
type: 'suite'
location: '/test/fixtures/test-runner/output/filtered-suite-throws.js:(LINE):1'
failureType: 'testCodeFailure'
error: 'boom 1'
code: 'ERR_TEST_FAILURE'
stack: |-
*
*
*
*
*
*
*
*
*
*
...
# Subtest: suite 2
# Subtest: enabled - should get cancelled
not ok 1 - enabled - should get cancelled
---
duration_ms: *
location: '/test/fixtures/test-runner/output/filtered-suite-throws.js:(LINE):3'
failureType: 'cancelledByParent'
error: 'test did not finish before its parent and was cancelled'
code: 'ERR_TEST_FAILURE'
...
1..1
not ok 2 - suite 2
---
duration_ms: *
type: 'suite'
location: '/test/fixtures/test-runner/output/filtered-suite-throws.js:(LINE):1'
failureType: 'testCodeFailure'
error: 'boom 1'
code: 'ERR_TEST_FAILURE'
stack: |-
*
*
*
*
*
*
*
*
*
*
...
1..2
# tests 1
# suites 2
# pass 0
# fail 0
# cancelled 1
# skipped 0
# todo 0
# duration_ms *
1 change: 1 addition & 0 deletions test/parallel/test-runner-output.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ const tests = [
{ name: 'test-runner/output/eval_dot.js', transform: specTransform },
{ name: 'test-runner/output/eval_spec.js', transform: specTransform },
{ name: 'test-runner/output/eval_tap.js' },
{ name: 'test-runner/output/filtered-suite-throws.js' },
{ name: 'test-runner/output/hooks.js' },
{ name: 'test-runner/output/hooks_spec_reporter.js', transform: specTransform },
{ name: 'test-runner/output/skip-each-hooks.js', transform: specTransform },
Expand Down

0 comments on commit e70711e

Please sign in to comment.