From 2226155ee638eaca281e919de1b73b291b24e8d6 Mon Sep 17 00:00:00 2001
From: Colin Ihrig <cjihrig@gmail.com>
Date: Sun, 18 Aug 2024 13:34:11 -0400
Subject: [PATCH] test_runner: report failures in filtered suites

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: https://github.com/nodejs/node/pull/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>
---
 lib/internal/test_runner/test.js              |  6 +-
 .../output/filtered-suite-throws.js           | 14 +++++
 .../output/filtered-suite-throws.snapshot     | 62 +++++++++++++++++++
 test/parallel/test-runner-output.mjs          |  1 +
 4 files changed, 80 insertions(+), 3 deletions(-)
 create mode 100644 test/fixtures/test-runner/output/filtered-suite-throws.js
 create mode 100644 test/fixtures/test-runner/output/filtered-suite-throws.snapshot

diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js
index 99872c1e1d773a..e4ebbb2ee9238b 100644
--- 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
new file mode 100644
index 00000000000000..7c9a68ce6db35f
--- /dev/null
+++ 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
new file mode 100644
index 00000000000000..6242e345891c42
--- /dev/null
+++ 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
index 23a7cc711feaef..bd2db22ee6cc36 100644
--- 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 },