Skip to content

Commit

Permalink
test_runner: fix afterEach not running on test failures
Browse files Browse the repository at this point in the history 
test_runner: fix afterEach not running on test failures

PR-URL: #45204
Fixes: #45192
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
  • Loading branch information
@MrJithil @danielleadams
MrJithil authored and danielleadams committed Dec 30, 2022
1 parent 1720acf commit 584792c
Show file tree
Hide file tree
Showing 6 changed files with 211 additions and 22 deletions.
28 changes: 18 additions & 10 deletions lib/internal/test_runner/test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ const {
const {
createDeferredPromise,
kEmptyObject,
once: runOnce,
} = require('internal/util');
const { isPromise } = require('internal/util/types');
const {
Expand Down Expand Up @@ -482,8 +483,14 @@ class Test extends AsyncResource {
return;
}

const { args, ctx } = this.getRunArgs();
const afterEach = runOnce(async () => {
if (this.parent?.hooks.afterEach.length > 0) {
await this.parent[kRunHook]('afterEach', { args, ctx });
}
});

try {
const { args, ctx } = this.getRunArgs();
if (this.parent?.hooks.beforeEach.length > 0) {
await this.parent[kRunHook]('beforeEach', { args, ctx });
}
Expand Down Expand Up @@ -518,12 +525,10 @@ class Test extends AsyncResource {
return;
}

if (this.parent?.hooks.afterEach.length > 0) {
await this.parent[kRunHook]('afterEach', { args, ctx });
}

await afterEach();
this.pass();
} catch (err) {
try { await afterEach(); } catch { /* test is already failing, let's the error */ }
if (isTestFailureError(err)) {
if (err.failureType === kTestTimeoutFailure) {
this.cancel(err);
Expand Down Expand Up @@ -728,6 +733,12 @@ class Suite extends Test {
}

async run() {
const hookArgs = this.getRunArgs();
const afterEach = runOnce(async () => {
if (this.parent?.hooks.afterEach.length > 0) {
await this.parent[kRunHook]('afterEach', hookArgs);
}
});
try {
this.parent.activeSubtests++;
await this.buildSuite;
Expand All @@ -739,7 +750,6 @@ class Suite extends Test {
return;
}

const hookArgs = this.getRunArgs();

if (this.parent?.hooks.beforeEach.length > 0) {
await this.parent[kRunHook]('beforeEach', hookArgs);
Expand All @@ -753,13 +763,11 @@ class Suite extends Test {

await SafePromiseRace([promise, stopPromise]);
await this[kRunHook]('after', hookArgs);

if (this.parent?.hooks.afterEach.length > 0) {
await this.parent[kRunHook]('afterEach', hookArgs);
}
await afterEach();

this.pass();
} catch (err) {
try { await afterEach(); } catch { /* test is already failing, let's the error */ }
if (isTestFailureError(err)) {
this.fail(err);
} else {
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/util.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -450,7 +450,7 @@ function once(callback) {
return function(...args) {
if (called) return;
called = true;
ReflectApply(callback, this, args);
return ReflectApply(callback, this, args);
};
}

Expand Down
2 changes: 0 additions & 2 deletions test/message/test_runner_describe_it.out
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,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
Expand Down
27 changes: 26 additions & 1 deletion test/message/test_runner_hooks.js
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Flags: --no-warnings
'use strict';
require('../common');
const common = require('../common');
const assert = require('assert');
const { test, describe, it, before, after, beforeEach, afterEach } = require('node:test');

Expand Down Expand Up @@ -76,6 +76,18 @@ describe('afterEach throws', () => {
it('2', () => {});
});

describe('afterEach when test fails', () => {
afterEach(common.mustCall(2));
it('1', () => { throw new Error('test'); });
it('2', () => {});
});

describe('afterEach throws and test fails', () => {
afterEach(() => { throw new Error('afterEach'); });
it('1', () => { throw new Error('test'); });
it('2', () => {});
});

test('test hooks', async (t) => {
const testArr = [];
t.beforeEach((t) => testArr.push('beforeEach ' + t.name));
Expand Down Expand Up @@ -111,3 +123,16 @@ test('t.afterEach throws', async (t) => {
await t.test('1', () => {});
await t.test('2', () => {});
});


test('afterEach when test fails', async (t) => {
t.afterEach(common.mustCall(2));
await t.test('1', () => { throw new Error('test'); });
await t.test('2', () => {});
});

test('afterEach throws and test fails', async (t) => {
afterEach(() => { throw new Error('afterEach'); });
await t.test('1', () => { throw new Error('test'); });
await t.test('2', () => {});
});
172 changes: 166 additions & 6 deletions test/message/test_runner_hooks.out
View file
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,86 @@ not ok 5 - afterEach throws
error: '2 subtests failed'
code: 'ERR_TEST_FAILURE'
...
# Subtest: afterEach when test fails
# Subtest: 1
not ok 1 - 1
---
duration_ms: *
failureType: 'testCodeFailure'
error: 'test'
code: 'ERR_TEST_FAILURE'
stack: |-
*
*
*
*
*
*
*
*
*
*
...
# Subtest: 2
ok 2 - 2
---
duration_ms: *
...
1..2
not ok 6 - afterEach when test fails
---
duration_ms: *
failureType: 'subtestsFailed'
error: '1 subtest failed'
code: 'ERR_TEST_FAILURE'
...
# Subtest: afterEach throws and test fails
# Subtest: 1
not ok 1 - 1
---
duration_ms: *
failureType: 'testCodeFailure'
error: 'test'
code: 'ERR_TEST_FAILURE'
stack: |-
*
*
*
*
*
*
*
*
*
*
...
# Subtest: 2
not ok 2 - 2
---
duration_ms: *
failureType: 'hookFailed'
error: 'failed running afterEach hook'
code: 'ERR_TEST_FAILURE'
stack: |-
*
*
*
*
*
*
*
*
*
*
...
1..2
not ok 7 - afterEach throws and test fails
---
duration_ms: *
failureType: 'subtestsFailed'
error: '2 subtests failed'
code: 'ERR_TEST_FAILURE'
...
# Subtest: test hooks
# Subtest: 1
ok 1 - 1
Expand Down Expand Up @@ -217,7 +297,7 @@ not ok 5 - afterEach throws
duration_ms: *
...
1..3
ok 6 - test hooks
ok 8 - test hooks
---
duration_ms: *
...
Expand Down Expand Up @@ -261,7 +341,7 @@ ok 6 - test hooks
*
...
1..2
not ok 7 - t.beforeEach throws
not ok 9 - t.beforeEach throws
---
duration_ms: *
failureType: 'subtestsFailed'
Expand Down Expand Up @@ -308,17 +388,97 @@ not ok 7 - t.beforeEach throws
*
...
1..2
not ok 8 - t.afterEach throws
not ok 10 - t.afterEach throws
---
duration_ms: *
failureType: 'subtestsFailed'
error: '2 subtests failed'
code: 'ERR_TEST_FAILURE'
...
# Subtest: afterEach when test fails
# Subtest: 1
not ok 1 - 1
---
duration_ms: *
failureType: 'testCodeFailure'
error: 'test'
code: 'ERR_TEST_FAILURE'
stack: |-
*
*
*
*
*
*
*
*
*
*
...
# Subtest: 2
ok 2 - 2
---
duration_ms: *
...
1..2
not ok 11 - afterEach when test fails
---
duration_ms: *
failureType: 'subtestsFailed'
error: '1 subtest failed'
code: 'ERR_TEST_FAILURE'
...
# Subtest: afterEach throws and test fails
# Subtest: 1
not ok 1 - 1
---
duration_ms: *
failureType: 'testCodeFailure'
error: 'test'
code: 'ERR_TEST_FAILURE'
stack: |-
*
*
*
*
*
*
*
*
*
*
...
# Subtest: 2
not ok 2 - 2
---
duration_ms: *
failureType: 'hookFailed'
error: 'failed running afterEach hook'
code: 'ERR_TEST_FAILURE'
stack: |-
*
*
*
*
*
*
*
*
*
*
...
1..2
not ok 12 - afterEach throws and test fails
---
duration_ms: *
failureType: 'subtestsFailed'
error: '2 subtests failed'
code: 'ERR_TEST_FAILURE'
...
1..8
# tests 8
1..12
# tests 12
# pass 2
# fail 6
# fail 10
# cancelled 0
# skipped 0
# todo 0
Expand Down
2 changes: 0 additions & 2 deletions test/message/test_runner_output.out
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,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
Expand Down

0 comments on commit 584792c

Please sign in to comment.