From a4cbb61c65c679cc1ba67e1659613dbb2823a547 Mon Sep 17 00:00:00 2001 From: Colin Ihrig Date: Sat, 9 Mar 2024 16:30:02 -0500 Subject: [PATCH] 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: https://github.com/nodejs/node/issues/51381 PR-URL: https://github.com/nodejs/node/pull/51996 Reviewed-By: Chemi Atlow Reviewed-By: Moshe Atlow Reviewed-By: Benjamin Gruenbaum --- lib/internal/test_runner/harness.js | 2 +- .../test-runner/output/describe_it.snapshot | 4 +- .../output/junit_reporter.snapshot | 2 - .../test-runner/output/output.snapshot | 2 - .../test-runner/output/output_cli.snapshot | 2 - .../test-runner/output/spec_reporter.snapshot | 4 -- .../output/spec_reporter_cli.snapshot | 4 -- .../output/unfinished-suite-async-error.js | 14 ++++++ .../unfinished-suite-async-error.snapshot | 43 +++++++++++++++++++ test/parallel/test-runner-output.mjs | 1 + 10 files changed, 60 insertions(+), 18 deletions(-) create mode 100644 test/fixtures/test-runner/output/unfinished-suite-async-error.js create mode 100644 test/fixtures/test-runner/output/unfinished-suite-async-error.snapshot diff --git a/lib/internal/test_runner/harness.js b/lib/internal/test_runner/harness.js index f381ac3ff60a67..f2b8bdde666d80 100644 --- 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 index 1d4f7853ead0d1..f6fed1a481908b 100644 --- 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 index c7136d18e6c3df..f29f72e6ddb2ed 100644 --- 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 * * * - * - * } diff --git a/test/fixtures/test-runner/output/output.snapshot b/test/fixtures/test-runner/output/output.snapshot index 0e6cab8549d4b2..905acaa498bffb 100644 --- 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 index cd2be499a3b7de..37c85137e4eb7f 100644 --- 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 index 5af5f4750569bc..a15ce8261bfc65 100644 --- 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 index 1321cb7321400d..79e7ecbd61d88f 100644 --- 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 new file mode 100644 index 00000000000000..ee6bf1a6918d48 --- /dev/null +++ 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 new file mode 100644 index 00000000000000..593e46d45e779a --- /dev/null +++ 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 index 6faf8e41106d29..c81a0f1160e7eb 100644 --- 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' },