Skip to content

Commit

Permalink
test_runner: run after hooks even if test is aborted
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
cjihrig committed Aug 4, 2024
1 parent 76d10a1 commit 67f7137
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 8 deletions.
10 changes: 2 additions & 8 deletions lib/internal/test_runner/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -763,10 +763,7 @@ class Test extends AsyncResource {
}

[kShouldAbort]() {
if (this.signal.aborted) {
return true;
}
if (this.outerSignal?.aborted) {
if (this.signal.aborted || this.outerSignal?.aborted) {
this.#abortHandler();
return true;
}
Expand Down Expand Up @@ -866,10 +863,7 @@ class Test extends AsyncResource {
await SafePromiseRace([PromiseResolve(promise), stopPromise]);
}

if (this[kShouldAbort]()) {
this.postRun();
return;
}
this[kShouldAbort]();
this.plan?.check();
this.pass();
await afterEach();
Expand Down
14 changes: 14 additions & 0 deletions test/fixtures/test-runner/output/abort-runs-after-hook.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
'use strict';
const { test } = require('node:test');

test('test that aborts', (t, done) => {
t.after(() => {
// This should still run.
console.log('AFTER');
});

setImmediate(() => {
// This creates an uncaughtException, which aborts the test.
throw new Error('boom');
});
});
23 changes: 23 additions & 0 deletions test/fixtures/test-runner/output/abort-runs-after-hook.snapshot
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
TAP version 13
AFTER
# Subtest: test that aborts
not ok 1 - test that aborts
---
duration_ms: *
location: '/test/fixtures/test-runner/output/abort-runs-after-hook.js:(LINE):1'
failureType: 'uncaughtException'
error: 'boom'
code: 'ERR_TEST_FAILURE'
stack: |-
*
*
...
1..1
# tests 1
# suites 0
# pass 0
# fail 1
# cancelled 0
# skipped 0
# todo 0
# duration_ms *
1 change: 1 addition & 0 deletions test/parallel/test-runner-output.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ const lcovTransform = snapshot.transform(

const tests = [
{ name: 'test-runner/output/abort.js' },
{ name: 'test-runner/output/abort-runs-after-hook.js' },
{ name: 'test-runner/output/abort_suite.js' },
{ name: 'test-runner/output/abort_hooks.js' },
{ name: 'test-runner/output/describe_it.js' },
Expand Down

0 comments on commit 67f7137

Please sign in to comment.