Skip to content

Commit

Permalink
fix: run t.after() if test body throws
Browse files Browse the repository at this point in the history
This commit fixes a bug where t.after() was not called if the
test body threw an exception.

PR-URL: nodejs/node#45870
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
(cherry picked from commit 385d595a4f1d887f6d4221e6071571132498d57c)
  • Loading branch information
cjihrig authored and MoLow committed Feb 2, 2023
1 parent d1343a7 commit c80e426
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 6 deletions.
10 changes: 8 additions & 2 deletions lib/internal/test_runner/test.js
@@ -1,4 +1,4 @@
// https://github.com/nodejs/node/blob/232efb06fe8787e9573e298ce7ac293ad23b7684/lib/internal/test_runner/test.js
// https://github.com/nodejs/node/blob/385d595a4f1d887f6d4221e6071571132498d57c/lib/internal/test_runner/test.js

'use strict'

Expand Down Expand Up @@ -496,6 +496,11 @@ class Test extends AsyncResource {
}

const { args, ctx } = this.getRunArgs()
const after = runOnce(async () => {
if (this.hooks.after.length > 0) {
await this.runHook('after', { args, ctx })
}
})
const afterEach = runOnce(async () => {
if (this.parent?.hooks.afterEach.length > 0) {
await this.parent.runHook('afterEach', { args, ctx })
Expand Down Expand Up @@ -537,10 +542,11 @@ class Test extends AsyncResource {
return
}

await this.runHook('after', { args, ctx })
await after()
await afterEach()
this.pass()
} catch (err) {
try { await after() } catch { /* Ignore error. */ }
try { await afterEach() } catch { /* test is already failing, let's the error */ }
if (isTestFailureError(err)) {
if (err.failureType === kTestTimeoutFailure) {
Expand Down
9 changes: 8 additions & 1 deletion test/message/test_runner_hooks.js
@@ -1,4 +1,4 @@
// https://github.com/nodejs/node/blob/215c5317d4837287fddb2e3b97872babd53183ac/test/message/test_runner_hooks.js
// https://github.com/nodejs/node/blob/385d595a4f1d887f6d4221e6071571132498d57c/test/message/test_runner_hooks.js
// Flags: --no-warnings
'use strict'
const common = require('../common')
Expand Down Expand Up @@ -142,3 +142,10 @@ test('afterEach throws and test fails', async (t) => {
await t.test('1', () => { throw new Error('test') })
await t.test('2', () => {})
})

test('t.after() is called if test body throws', (t) => {
t.after(() => {
t.diagnostic('- after() called')
})
throw new Error('bye')
})
24 changes: 21 additions & 3 deletions test/message/test_runner_hooks.out
Expand Up @@ -475,10 +475,28 @@ not ok 12 - afterEach throws and test fails
error: '2 subtests failed'
code: 'ERR_TEST_FAILURE'
...
1..12
# tests 12
# Subtest: t.after() is called if test body throws
not ok 13 - t.after() is called if test body throws
---
duration_ms: *
failureType: 'testCodeFailure'
error: 'bye'
code: 'ERR_TEST_FAILURE'
stack: |-
*
*
*
*
*
*
*
*
...
# - after() called
1..13
# tests 13
# pass 2
# fail 10
# fail 11
# cancelled 0
# skipped 0
# todo 0
Expand Down

0 comments on commit c80e426

Please sign in to comment.