Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test_runner: fix global after not failing the tests #48913

Merged
8 changes: 8 additions & 0 deletions lib/internal/test_runner/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -688,6 +688,14 @@ class Test extends AsyncResource {
this.parent.processReadySubtestRange(false);
this.parent.processPendingSubtests();
} else if (!this.reported) {
if (!this.passed && failed === 0 && this.error) {
this.reporter.fail(0, kFilename, this.subtests.length + 1, kFilename, {
__proto__: null,
duration_ms: this.#duration(),
error: this.error,
}, undefined);
}

this.reported = true;
this.reporter.plan(this.nesting, kFilename, this.root.harness.counters.topLevel);

Expand Down
5 changes: 5 additions & 0 deletions test/common/assertSnapshot.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ function replaceWindowsPaths(str) {
return common.isWindows ? str.replaceAll(path.win32.sep, path.posix.sep) : str;
}

function replaceFullPaths(str) {
return str.replaceAll(process.cwd(), '');
}

function transform(...args) {
return (str) => args.reduce((acc, fn) => fn(acc), str);
}
Expand Down Expand Up @@ -69,6 +73,7 @@ async function spawnAndAssert(filename, transform = (x) => x, { tty = false, ...
module.exports = {
assertSnapshot,
getSnapshotPath,
replaceFullPaths,
replaceStackTrace,
replaceWindowsLineEndings,
replaceWindowsPaths,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
'use strict';
const { it, after } = require('node:test');

after(() => {
throw new Error('this should fail the test')
});

it('this is a test', () => {
console.log('this is a test')
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
this is a test
TAP version 13
# Subtest: this is a test
ok 1 - this is a test
---
duration_ms: *
...
not ok 2 - /test/fixtures/test-runner/output/global_after_should_fail_the_test.js
---
duration_ms: *
failureType: 'hookFailed'
error: 'this should fail the test'
code: 'ERR_TEST_FAILURE'
stack: |-
*
*
*
*
*
*
*
*
*
*
...
1..1
# tests 1
# suites 0
# pass 1
# fail 0
# cancelled 0
# skipped 0
# todo 0
# duration_ms *
54 changes: 24 additions & 30 deletions test/fixtures/test-runner/output/hooks-with-no-global-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,42 +9,36 @@ before(() => testArr.push('global before'));
after(() => {
testArr.push('global after');

try {
assert.deepStrictEqual(testArr, [
'global before',
'describe before',
assert.deepStrictEqual(testArr, [
'global before',
'describe before',

'describe beforeEach',
'describe it 1',
'describe afterEach',
'describe beforeEach',
'describe it 1',
'describe afterEach',

'describe beforeEach',
'describe test 2',
'describe afterEach',
'describe beforeEach',
'describe test 2',
'describe afterEach',

'describe nested before',
'describe nested before',

'describe beforeEach',
'describe nested beforeEach',
'describe nested it 1',
'describe afterEach',
'describe nested afterEach',
'describe beforeEach',
'describe nested beforeEach',
'describe nested it 1',
'describe afterEach',
'describe nested afterEach',

'describe beforeEach',
'describe nested beforeEach',
'describe nested test 2',
'describe afterEach',
'describe nested afterEach',
'describe beforeEach',
'describe nested beforeEach',
'describe nested test 2',
'describe afterEach',
'describe nested afterEach',

'describe nested after',
'describe after',
'global after',
]);
} catch (e) {
// TODO(rluvaton): remove the try catch after #48867 is fixed
console.error(e);
process.exit(1);
}
'describe nested after',
'describe after',
'global after',
]);
});

describe('describe hooks with no global tests', () => {
Expand Down
26 changes: 22 additions & 4 deletions test/parallel/test-runner-output.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,27 @@ function replaceSpecDuration(str) {
.replaceAll(/duration_ms [0-9.]+/g, 'duration_ms *')
.replace(stackTraceBasePath, '$3');
}
const defaultTransform = snapshot
.transform(snapshot.replaceWindowsLineEndings, snapshot.replaceStackTrace, replaceTestDuration);
const specTransform = snapshot
.transform(replaceSpecDuration, snapshot.replaceWindowsLineEndings, snapshot.replaceStackTrace);

function removeWindowsPathEscaping(str) {
return common.isWindows ? str.replaceAll(/\\\\/g, '\\') : str;
}

const defaultTransform = snapshot.transform(
snapshot.replaceWindowsLineEndings,
snapshot.replaceStackTrace,
replaceTestDuration,
);
const specTransform = snapshot.transform(
replaceSpecDuration,
snapshot.replaceWindowsLineEndings,
snapshot.replaceStackTrace,
);
const withFileNameTransform = snapshot.transform(
defaultTransform,
removeWindowsPathEscaping,
snapshot.replaceFullPaths,
snapshot.replaceWindowsPaths,
);


const tests = [
Expand All @@ -41,6 +58,7 @@ const tests = [
{ name: 'test-runner/output/hooks-with-no-global-test.js' },
{ name: 'test-runner/output/before-and-after-each-too-many-listeners.js' },
{ name: 'test-runner/output/before-and-after-each-with-timeout-too-many-listeners.js' },
{ name: 'test-runner/output/global_after_should_fail_the_test.js', transform: withFileNameTransform },
{ name: 'test-runner/output/no_refs.js' },
{ name: 'test-runner/output/no_tests.js' },
{ name: 'test-runner/output/only_tests.js' },
Expand Down