test_runner: report failures in filtered suites

cjihrig · RafaelGSS · commit e70711e1904c · 2024-08-21T17:49:21.000-03:00
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>
diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js
@@ -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;
diff --git a/test/fixtures/test-runner/output/filtered-suite-throws.js b/test/fixtures/test-runner/output/filtered-suite-throws.js
@@ -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');
+});
diff --git a/test/fixtures/test-runner/output/filtered-suite-throws.snapshot b/test/fixtures/test-runner/output/filtered-suite-throws.snapshot
@@ -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 *
diff --git a/test/parallel/test-runner-output.mjs b/test/parallel/test-runner-output.mjs
@@ -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 },
