From c09cfd7a71c9bd4498d89f42d9d1cd3ac3e5ba49 Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Tue, 12 Jul 2022 17:16:40 +0300 Subject: [PATCH] fix: catch errors thrown within `describe` PR-URL: https://github.com/nodejs/node/pull/43729 Reviewed-By: Antoine du Hamel Reviewed-By: Benjamin Gruenbaum (cherry picked from commit 3aec7da5c6fcbedffdb034a9b7f6b60958b88d93) --- lib/internal/test_runner/test.js | 14 +- test/message/test_runner_desctibe_it.js | 15 +++ test/message/test_runner_desctibe_it.out | 158 ++++++++++++++++------- 3 files changed, 137 insertions(+), 50 deletions(-) diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index 4c73274..3af0f70 100644 --- a/lib/internal/test_runner/test.js +++ b/lib/internal/test_runner/test.js @@ -365,6 +365,7 @@ class Test extends AsyncResource { if (this.endTime < this.startTime) { this.endTime = hrtime() } + if (this.startTime == null) this.startTime = this.endTime // The test has run, so recursively cancel any outstanding subtests and // mark this test as failed if any subtests failed. @@ -462,7 +463,11 @@ class Suite extends Test { constructor (options) { super(options) - this.runInAsyncScope(this.fn) + try { + this.buildSuite = this.runInAsyncScope(this.fn) + } catch (err) { + this.fail(new ERR_TEST_FAILURE(err, kTestCodeFailure)) + } this.fn = () => {} this.finished = true // Forbid adding subtests to this suite } @@ -472,9 +477,14 @@ class Suite extends Test { } async run () { + try { + await this.buildSuite + } catch (err) { + this.fail(new ERR_TEST_FAILURE(err, kTestCodeFailure)) + } this.parent.activeSubtests++ this.startTime = hrtime() - const subtests = this.skipped ? [] : this.subtests + const subtests = this.skipped || this.error ? [] : this.subtests await ArrayPrototypeReduce(subtests, async (prev, subtest) => { await prev await subtest.run() diff --git a/test/message/test_runner_desctibe_it.js b/test/message/test_runner_desctibe_it.js index c857668..f4673c6 100644 --- a/test/message/test_runner_desctibe_it.js +++ b/test/message/test_runner_desctibe_it.js @@ -45,6 +45,11 @@ it('async throw fail', async () => { throw new Error('thrown from async throw fail') }) +it('async skip fail', async (t) => { + t.skip() + throw new Error('thrown from async throw fail') +}) + it('async assertion fail', async () => { // Make sure the assert module is handled. assert.strictEqual(true, false) @@ -301,3 +306,13 @@ describe('subtest sync throw fails', () => { throw new Error('thrown from subtest sync throw fails at second') }) }) + +describe('describe sync throw fails', () => { + it('should not run', () => {}) + throw new Error('thrown from describe') +}) + +describe('describe async throw fails', async () => { + it('should not run', () => {}) + throw new Error('thrown from describe') +}) diff --git a/test/message/test_runner_desctibe_it.out b/test/message/test_runner_desctibe_it.out index 93e7b06..99da34e 100644 --- a/test/message/test_runner_desctibe_it.out +++ b/test/message/test_runner_desctibe_it.out @@ -23,7 +23,6 @@ not ok 3 - sync fail todo # TODO * * * - * ... # Subtest: sync fail todo with message not ok 4 - sync fail todo with message # TODO this is a failing todo @@ -42,7 +41,6 @@ not ok 4 - sync fail todo with message # TODO this is a failing todo * * * - * ... # Subtest: sync skip pass ok 5 - sync skip pass # SKIP @@ -101,8 +99,16 @@ not ok 11 - async throw fail * * ... +# Subtest: async skip fail +not ok 12 - async skip fail + --- + duration_ms: * + failureType: 'callbackAndPromisePresent' + error: 'passed a callback but also returned a Promise' + code: 'ERR_TEST_FAILURE' + ... # Subtest: async assertion fail -not ok 12 - async assertion fail +not ok 13 - async assertion fail --- duration_ms: * failureType: 'testCodeFailure' @@ -122,12 +128,12 @@ not ok 12 - async assertion fail * ... # Subtest: resolve pass -ok 13 - resolve pass +ok 14 - resolve pass --- duration_ms: * ... # Subtest: reject fail -not ok 14 - reject fail +not ok 15 - reject fail --- duration_ms: * failureType: 'testCodeFailure' @@ -143,27 +149,27 @@ not ok 14 - reject fail * ... # Subtest: unhandled rejection - passes but warns -ok 15 - unhandled rejection - passes but warns +ok 16 - unhandled rejection - passes but warns --- duration_ms: * ... # Subtest: async unhandled rejection - passes but warns -ok 16 - async unhandled rejection - passes but warns +ok 17 - async unhandled rejection - passes but warns --- duration_ms: * ... # Subtest: immediate throw - passes but warns -ok 17 - immediate throw - passes but warns +ok 18 - immediate throw - passes but warns --- duration_ms: * ... # Subtest: immediate reject - passes but warns -ok 18 - immediate reject - passes but warns +ok 19 - immediate reject - passes but warns --- duration_ms: * ... # Subtest: immediate resolve pass -ok 19 - immediate resolve pass +ok 20 - immediate resolve pass --- duration_ms: * ... @@ -184,7 +190,7 @@ ok 19 - immediate resolve pass * ... 1..1 -not ok 20 - subtest sync throw fail +not ok 21 - subtest sync throw fail --- duration_ms: * failureType: 'subtestsFailed' @@ -193,7 +199,7 @@ not ok 20 - subtest sync throw fail stack: |- ... # Subtest: sync throw non-error fail -not ok 21 - sync throw non-error fail +not ok 22 - sync throw non-error fail --- duration_ms: * failureType: 'testCodeFailure' @@ -223,7 +229,7 @@ not ok 21 - sync throw non-error fail duration_ms: * ... 1..4 -ok 22 - level 0a +ok 23 - level 0a --- duration_ms: * ... @@ -245,27 +251,27 @@ ok 22 - level 0a duration_ms: * ... 1..2 -ok 23 - top level +ok 24 - top level --- duration_ms: * ... # Subtest: invalid subtest - pass but subtest fails -ok 24 - invalid subtest - pass but subtest fails +ok 25 - invalid subtest - pass but subtest fails --- duration_ms: * ... # Subtest: sync skip option -ok 25 - sync skip option # SKIP +ok 26 - sync skip option # SKIP --- duration_ms: * ... # Subtest: sync skip option with message -ok 26 - sync skip option with message # SKIP this is skipped +ok 27 - sync skip option with message # SKIP this is skipped --- duration_ms: * ... # Subtest: sync skip option is false fail -not ok 27 - sync skip option is false fail +not ok 28 - sync skip option is false fail --- duration_ms: * failureType: 'testCodeFailure' @@ -281,67 +287,67 @@ not ok 27 - sync skip option is false fail * ... # Subtest: -ok 28 - +ok 29 - --- duration_ms: * ... # Subtest: functionOnly -ok 29 - functionOnly +ok 30 - functionOnly --- duration_ms: * ... # Subtest: -ok 30 - +ok 31 - --- duration_ms: * ... # Subtest: test with only a name provided -ok 31 - test with only a name provided +ok 32 - test with only a name provided --- duration_ms: * ... # Subtest: -ok 32 - +ok 33 - --- duration_ms: * ... # Subtest: -ok 33 - # SKIP +ok 34 - # SKIP --- duration_ms: * ... # Subtest: test with a name and options provided -ok 34 - test with a name and options provided # SKIP +ok 35 - test with a name and options provided # SKIP --- duration_ms: * ... # Subtest: functionAndOptions -ok 35 - functionAndOptions # SKIP +ok 36 - functionAndOptions # SKIP --- duration_ms: * ... # Subtest: escaped description \\ \# \\\#\\ -ok 36 - escaped description \\ \# \\\#\\ +ok 37 - escaped description \\ \# \\\#\\ --- duration_ms: * ... # Subtest: escaped skip message -ok 37 - escaped skip message # SKIP \#skip +ok 38 - escaped skip message # SKIP \#skip --- duration_ms: * ... # Subtest: escaped todo message -ok 38 - escaped todo message # TODO \#todo +ok 39 - escaped todo message # TODO \#todo --- duration_ms: * ... # Subtest: callback pass -ok 39 - callback pass +ok 40 - callback pass --- duration_ms: * ... # Subtest: callback fail -not ok 40 - callback fail +not ok 41 - callback fail --- duration_ms: * failureType: 'testCodeFailure' @@ -352,22 +358,22 @@ not ok 40 - callback fail * ... # Subtest: sync t is this in test -ok 41 - sync t is this in test +ok 42 - sync t is this in test --- duration_ms: * ... # Subtest: async t is this in test -ok 42 - async t is this in test +ok 43 - async t is this in test --- duration_ms: * ... # Subtest: callback t is this in test -ok 43 - callback t is this in test +ok 44 - callback t is this in test --- duration_ms: * ... # Subtest: callback also returns a Promise -not ok 44 - callback also returns a Promise +not ok 45 - callback also returns a Promise --- duration_ms: * failureType: 'callbackAndPromisePresent' @@ -376,7 +382,7 @@ not ok 44 - callback also returns a Promise stack: |- ... # Subtest: callback throw -not ok 45 - callback throw +not ok 46 - callback throw --- duration_ms: * failureType: 'testCodeFailure' @@ -392,7 +398,7 @@ not ok 45 - callback throw * ... # Subtest: callback called twice -not ok 46 - callback called twice +not ok 47 - callback called twice --- duration_ms: * failureType: 'multipleCallbackInvocations' @@ -403,12 +409,12 @@ not ok 46 - callback called twice * ... # Subtest: callback called twice in different ticks -ok 47 - callback called twice in different ticks +ok 48 - callback called twice in different ticks --- duration_ms: * ... # Subtest: callback called twice in future tick -not ok 48 - callback called twice in future tick +not ok 49 - callback called twice in future tick --- duration_ms: * failureType: 'uncaughtException' @@ -418,7 +424,7 @@ not ok 48 - callback called twice in future tick * ... # Subtest: callback async throw -not ok 49 - callback async throw +not ok 50 - callback async throw --- duration_ms: * failureType: 'uncaughtException' @@ -428,12 +434,12 @@ not ok 49 - callback async throw * ... # Subtest: callback async throw after done -ok 50 - callback async throw after done +ok 51 - callback async throw after done --- duration_ms: * ... # Subtest: custom inspect symbol fail -not ok 51 - custom inspect symbol fail +not ok 52 - custom inspect symbol fail --- duration_ms: * failureType: 'testCodeFailure' @@ -442,7 +448,7 @@ not ok 51 - custom inspect symbol fail stack: |- ... # Subtest: custom inspect symbol that throws fail -not ok 52 - custom inspect symbol that throws fail +not ok 53 - custom inspect symbol that throws fail --- duration_ms: * failureType: 'testCodeFailure' @@ -487,7 +493,7 @@ not ok 52 - custom inspect symbol that throws fail * ... 1..2 -not ok 53 - subtest sync throw fails +not ok 54 - subtest sync throw fails --- duration_ms: * failureType: 'subtestsFailed' @@ -495,8 +501,64 @@ not ok 53 - subtest sync throw fails code: 'ERR_TEST_FAILURE' stack: |- ... +# Subtest: describe sync throw fails + # Subtest: should not run + not ok 1 - should not run + --- + duration_ms: * + failureType: 'cancelledByParent' + error: 'test did not finish before its parent and was cancelled' + code: 'ERR_TEST_FAILURE' + ... + 1..1 +not ok 55 - describe sync throw fails + --- + duration_ms: * + failureType: 'testCodeFailure' + error: 'thrown from describe' + code: 'ERR_TEST_FAILURE' + stack: |- + * + * + * + * + * + * + * + * + * + * + ... +# Subtest: describe async throw fails + # Subtest: should not run + not ok 1 - should not run + --- + duration_ms: * + failureType: 'cancelledByParent' + error: 'test did not finish before its parent and was cancelled' + code: 'ERR_TEST_FAILURE' + ... + 1..1 +not ok 56 - describe async throw fails + --- + duration_ms: * + failureType: 'testCodeFailure' + error: 'thrown from describe' + code: 'ERR_TEST_FAILURE' + stack: |- + * + * + * + * + * + * + * + * + * + * + ... # Subtest: invalid subtest fail -not ok 54 - invalid subtest fail +not ok 57 - invalid subtest fail --- duration_ms: * failureType: 'parentAlreadyFinished' @@ -505,16 +567,16 @@ not ok 54 - invalid subtest fail stack: |- * ... -1..54 +1..57 # Warning: Test "unhandled rejection - passes but warns" generated asynchronous activity after the test ended. This activity created the error "Error: rejected from unhandled rejection fail" and would have caused the test to fail, but instead triggered an unhandledRejection event. # Warning: Test "async unhandled rejection - passes but warns" generated asynchronous activity after the test ended. This activity created the error "Error: rejected from async unhandled rejection fail" and would have caused the test to fail, but instead triggered an unhandledRejection event. # Warning: Test "immediate throw - passes but warns" generated asynchronous activity after the test ended. This activity created the error "Error: thrown from immediate throw fail" and would have caused the test to fail, but instead triggered an uncaughtException event. # Warning: Test "immediate reject - passes but warns" generated asynchronous activity after the test ended. This activity created the error "Error: rejected from immediate reject fail" and would have caused the test to fail, but instead triggered an unhandledRejection event. # Warning: Test "callback called twice in different ticks" generated asynchronous activity after the test ended. This activity created the error "Error [ERR_TEST_FAILURE]: callback invoked multiple times" and would have caused the test to fail, but instead triggered an uncaughtException event. # Warning: Test "callback async throw after done" generated asynchronous activity after the test ended. This activity created the error "Error: thrown from callback async throw after done" and would have caused the test to fail, but instead triggered an uncaughtException event. -# tests 54 +# tests 57 # pass 23 -# fail 17 +# fail 20 # cancelled 0 # skipped 9 # todo 5