From 71f7ed728d671a2f83ba6854d32bab54955a54a8 Mon Sep 17 00:00:00 2001 From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Wed, 2 Aug 2023 23:06:25 +0300 Subject: [PATCH] test_runner: fix global after not failing the tests PR-URL: https://github.com/nodejs/node/pull/48913 Backport-PR-URL: https://github.com/nodejs/node/pull/49225 Fixes: https://github.com/nodejs/node/issues/48867 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Moshe Atlow Reviewed-By: Chemi Atlow --- lib/internal/test_runner/test.js | 8 +++ test/common/assertSnapshot.js | 5 ++ .../global_after_should_fail_the_test.js | 10 ++++ ...global_after_should_fail_the_test.snapshot | 34 ++++++++++++ .../output/hooks-with-no-global-test.js | 54 +++++++++---------- test/parallel/test-runner-output.mjs | 26 +++++++-- 6 files changed, 103 insertions(+), 34 deletions(-) create mode 100644 test/fixtures/test-runner/output/global_after_should_fail_the_test.js create mode 100644 test/fixtures/test-runner/output/global_after_should_fail_the_test.snapshot diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index 61aab9f9166d29..aef307ac0cb3b3 100644 --- a/lib/internal/test_runner/test.js +++ b/lib/internal/test_runner/test.js @@ -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); diff --git a/test/common/assertSnapshot.js b/test/common/assertSnapshot.js index 33d6570efbc75e..349912a5c56231 100644 --- a/test/common/assertSnapshot.js +++ b/test/common/assertSnapshot.js @@ -24,6 +24,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); } @@ -73,6 +77,7 @@ async function spawnAndAssert(filename, transform = (x) => x, { tty = false, ... module.exports = { assertSnapshot, getSnapshotPath, + replaceFullPaths, replaceNodeVersion, replaceStackTrace, replaceWindowsLineEndings, diff --git a/test/fixtures/test-runner/output/global_after_should_fail_the_test.js b/test/fixtures/test-runner/output/global_after_should_fail_the_test.js new file mode 100644 index 00000000000000..e2ad4c815b7fcd --- /dev/null +++ b/test/fixtures/test-runner/output/global_after_should_fail_the_test.js @@ -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') +}); diff --git a/test/fixtures/test-runner/output/global_after_should_fail_the_test.snapshot b/test/fixtures/test-runner/output/global_after_should_fail_the_test.snapshot new file mode 100644 index 00000000000000..16693c1a8a964b --- /dev/null +++ b/test/fixtures/test-runner/output/global_after_should_fail_the_test.snapshot @@ -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 * diff --git a/test/fixtures/test-runner/output/hooks-with-no-global-test.js b/test/fixtures/test-runner/output/hooks-with-no-global-test.js index 844aa6ff3c2d59..ea01463fd6cc1f 100644 --- a/test/fixtures/test-runner/output/hooks-with-no-global-test.js +++ b/test/fixtures/test-runner/output/hooks-with-no-global-test.js @@ -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', () => { diff --git a/test/parallel/test-runner-output.mjs b/test/parallel/test-runner-output.mjs index 84fb4b1824dc34..8d5233d2de2441 100644 --- a/test/parallel/test-runner-output.mjs +++ b/test/parallel/test-runner-output.mjs @@ -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 = [ @@ -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' },