test_runner: abort unfinished tests on async error

This commit updates the test runner's uncaughtException handler to abort tests instead of assuming they finished running. Fixes: #51381 PR-URL: #51996 Reviewed-By: Chemi Atlow <[email protected]> Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
nodejs · Mar 9, 2024 · 29b2317 · 29b2317
1 parent b241a1d
commit 29b2317
Show file tree

Hide file tree

Showing 10 changed files with 60 additions and 18 deletions.
diff --git a/lib/internal/test_runner/harness.js b/lib/internal/test_runner/harness.js
@@ -74,7 +74,7 @@ function createProcessEventHandler(eventName, rootTest) {
     }
 
     test.fail(new ERR_TEST_FAILURE(err, eventName));
-    test.postRun();
+    test.abortController.abort();
   };
 }
 

diff --git a/test/fixtures/test-runner/output/describe_it.snapshot b/test/fixtures/test-runner/output/describe_it.snapshot
@@ -494,10 +494,9 @@ not ok 50 - custom inspect symbol that throws fail
         *
         *
         *
+        async Promise.all (index 0)
         *
         *
-        *
-        async Promise.all (index 0)
       ...
     1..2
 not ok 51 - subtest sync throw fails
@@ -628,7 +627,6 @@ not ok 54 - timeouts
       code: 'ERR_TEST_FAILURE'
       stack: |-
         *
-        *
       ...
     1..2
 not ok 55 - successful thenable

diff --git a/test/fixtures/test-runner/output/junit_reporter.snapshot b/test/fixtures/test-runner/output/junit_reporter.snapshot
@@ -372,8 +372,6 @@ Error [ERR_TEST_FAILURE]: thrown from subtest sync throw fails at second
       *
       *
       *
-      *
-      *
 }
 			</failure>
 		</testcase>

diff --git a/test/fixtures/test-runner/output/output.snapshot b/test/fixtures/test-runner/output/output.snapshot
@@ -552,8 +552,6 @@ not ok 51 - custom inspect symbol that throws fail
         *
         *
         *
-        *
-        *
       ...
     1..2
 not ok 52 - subtest sync throw fails

diff --git a/test/fixtures/test-runner/output/output_cli.snapshot b/test/fixtures/test-runner/output/output_cli.snapshot
@@ -552,8 +552,6 @@ not ok 51 - custom inspect symbol that throws fail
         *
         *
         *
-        *
-        *
       ...
     1..2
 not ok 52 - subtest sync throw fails

diff --git a/test/fixtures/test-runner/output/spec_reporter.snapshot b/test/fixtures/test-runner/output/spec_reporter.snapshot
@@ -229,8 +229,6 @@
         *
         *
         *
-        *
-        *
 
  subtest sync throw fails (*ms)
 
@@ -515,8 +513,6 @@
       *
       *
       *
-      *
-      *
 
 *
  timed out async test (*ms)

diff --git a/test/fixtures/test-runner/output/spec_reporter_cli.snapshot b/test/fixtures/test-runner/output/spec_reporter_cli.snapshot
@@ -229,8 +229,6 @@
         *
         *
         *
-        *
-        *
 
  subtest sync throw fails (*ms)
 
@@ -515,8 +513,6 @@
       *
       *
       *
-      *
-      *
 
 *
  timed out async test (*ms)

diff --git a/test/fixtures/test-runner/output/unfinished-suite-async-error.js b/test/fixtures/test-runner/output/unfinished-suite-async-error.js
@@ -0,0 +1,14 @@
+'use strict';
+const { describe, it } = require('node:test');
+
+describe('unfinished suite with asynchronous error', () => {
+  it('uses callback', (t, done) => {
+    setImmediate(() => {
+      throw new Error('callback test does not complete');
+    });
+  });
+
+  it('should pass 1');
+});
+
+it('should pass 2');
diff --git a/test/fixtures/test-runner/output/unfinished-suite-async-error.snapshot b/test/fixtures/test-runner/output/unfinished-suite-async-error.snapshot
@@ -0,0 +1,43 @@
+TAP version 13
+# Subtest: unfinished suite with asynchronous error
+    # Subtest: uses callback
+    not ok 1 - uses callback
+      ---
+      duration_ms: *
+      location: '/test/fixtures/test-runner/output/unfinished-suite-async-error.js:(LINE):3'
+      failureType: 'uncaughtException'
+      error: 'callback test does not complete'
+      code: 'ERR_TEST_FAILURE'
+      stack: |-
+        *
+        *
+      ...
+    # Subtest: should pass 1
+    ok 2 - should pass 1
+      ---
+      duration_ms: *
+      ...
+    1..2
+not ok 1 - unfinished suite with asynchronous error
+  ---
+  duration_ms: *
+  type: 'suite'
+  location: '/test/fixtures/test-runner/output/unfinished-suite-async-error.js:(LINE):1'
+  failureType: 'subtestsFailed'
+  error: '1 subtest failed'
+  code: 'ERR_TEST_FAILURE'
+  ...
+# Subtest: should pass 2
+ok 2 - should pass 2
+  ---
+  duration_ms: *
+  ...
+1..2
+# tests 3
+# suites 1
+# pass 2
+# fail 1
+# cancelled 0
+# skipped 0
+# todo 0
+# duration_ms *
diff --git a/test/parallel/test-runner-output.mjs b/test/parallel/test-runner-output.mjs
@@ -112,6 +112,7 @@ const tests = [
   { name: 'test-runner/output/output_cli.js' },
   { name: 'test-runner/output/name_pattern.js' },
   { name: 'test-runner/output/name_pattern_with_only.js' },
+  { name: 'test-runner/output/unfinished-suite-async-error.js' },
   { name: 'test-runner/output/unresolved_promise.js' },
   { name: 'test-runner/output/default_output.js', transform: specTransform, tty: true },
   { name: 'test-runner/output/arbitrary-output.js' },
-Original file line number
+Diff line change
@@ Expand Up / @@ -74,7 +74,7 @@ function createProcessEventHandler(eventName, rootTest) { @@
         }
         test.fail(new ERR_TEST_FAILURE(err, eventName));
-        test.postRun();
+        test.abortController.abort();
       };
     }
@@ Expand Down @@