Skip to content

Commit

Permalink
test_runner: fix global after not failing the tests
Browse files Browse the repository at this point in the history
PR-URL: nodejs#48913
Fixes: nodejs#48867
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
  • Loading branch information
rluvaton authored and pluris committed Aug 7, 2023
1 parent f1d7321 commit 6c72f40
Show file tree
Hide file tree
Showing 6 changed files with 103 additions and 34 deletions.
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

0 comments on commit 6c72f40

Please sign in to comment.