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: nodejs#54387
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Jake Yuesong Li <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
  • Loading branch information
cjihrig authored Aug 18, 2024
1 parent 18f455b commit 2226155
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 2226155

Please sign in to comment.