Commit 65b6fec
test_runner: run after hooks even if test is aborted
If a test is run, but aborted, any after hooks should still be
run, as they may need to perform cleanup.
PR-URL: #54151
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Jake Yuesong Li <jake.yuesong@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Raz Luvaton <rluvaton@gmail.com>1 parent f131dc6 commit 65b6fec
4 files changed
Lines changed: 40 additions & 8 deletions
File tree
- lib/internal/test_runner
- test
- fixtures/test-runner/output
- parallel
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -694,10 +694,7 @@ class Test extends AsyncResource { | |||
| 694 | 694 | } | |
| 695 | 695 | ||
| 696 | 696 | [kShouldAbort]() { | |
| 697 | - if (this.signal.aborted) { | ||
| 698 | - return true; | ||
| 699 | - } | ||
| 700 | - if (this.outerSignal?.aborted) { | ||
| 697 | + if (this.signal.aborted || this.outerSignal?.aborted) { | ||
| 701 | 698 | this.#abortHandler(); | |
| 702 | 699 | return true; | |
| 703 | 700 | } | |
@@ -790,10 +787,7 @@ class Test extends AsyncResource { | |||
| 790 | 787 | await SafePromiseRace([PromiseResolve(promise), stopPromise]); | |
| 791 | 788 | } | |
| 792 | 789 | ||
| 793 | - if (this[kShouldAbort]()) { | ||
| 794 | - this.postRun(); | ||
| 795 | - return; | ||
| 796 | - } | ||
| 790 | + this[kShouldAbort](); | ||
| 797 | 791 | this.plan?.check(); | |
| 798 | 792 | this.pass(); | |
| 799 | 793 | await afterEach(); | |
Lines changed: 14 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -0,0 +1,14 @@ | |||
| 1 | + 'use strict'; | ||
| 2 | + const { test } = require('node:test'); | ||
| 3 | + | ||
| 4 | + test('test that aborts', (t, done) => { | ||
| 5 | + t.after(() => { | ||
| 6 | + // This should still run. | ||
| 7 | + console.log('AFTER'); | ||
| 8 | + }); | ||
| 9 | + | ||
| 10 | + setImmediate(() => { | ||
| 11 | + // This creates an uncaughtException, which aborts the test. | ||
| 12 | + throw new Error('boom'); | ||
| 13 | + }); | ||
| 14 | + }); | ||
Lines changed: 23 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -0,0 +1,23 @@ | |||
| 1 | + TAP version 13 | ||
| 2 | + AFTER | ||
| 3 | + # Subtest: test that aborts | ||
| 4 | + not ok 1 - test that aborts | ||
| 5 | + --- | ||
| 6 | + duration_ms: * | ||
| 7 | + location: '/test/fixtures/test-runner/output/abort-runs-after-hook.js:(LINE):1' | ||
| 8 | + failureType: 'uncaughtException' | ||
| 9 | + error: 'boom' | ||
| 10 | + code: 'ERR_TEST_FAILURE' | ||
| 11 | + stack: |- | ||
| 12 | + * | ||
| 13 | + * | ||
| 14 | + ... | ||
| 15 | + 1..1 | ||
| 16 | + # tests 1 | ||
| 17 | + # suites 0 | ||
| 18 | + # pass 0 | ||
| 19 | + # fail 1 | ||
| 20 | + # cancelled 0 | ||
| 21 | + # skipped 0 | ||
| 22 | + # todo 0 | ||
| 23 | + # duration_ms * | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -93,6 +93,7 @@ const lcovTransform = snapshot.transform( | |||
| 93 | 93 | ||
| 94 | 94 | const tests = [ | |
| 95 | 95 | { name: 'test-runner/output/abort.js' }, | |
| 96 | + { name: 'test-runner/output/abort-runs-after-hook.js' }, | ||
| 96 | 97 | { name: 'test-runner/output/abort_suite.js' }, | |
| 97 | 98 | { name: 'test-runner/output/abort_hooks.js' }, | |
| 98 | 99 | { name: 'test-runner/output/describe_it.js' }, | |
0 commit comments