From 6c2a758749b1a0a138e1879c77d6a3f93aaf2fa1 Mon Sep 17 00:00:00 2001 From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Tue, 25 Jul 2023 11:07:10 +0300 Subject: [PATCH 01/15] test_runner: fix global after not failing the tests --- lib/internal/test_runner/test.js | 8 +++ .../global-after-should-fail-the-test.js | 10 ++++ .../output/hooks-with-no-global-test.js | 54 +++++++++---------- test/parallel/test-runner-output.mjs | 1 + 4 files changed, 43 insertions(+), 30 deletions(-) create mode 100644 test/fixtures/test-runner/output/global-after-should-fail-the-test.js diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index c3fa0c2ff39721..ae8dda77f75ce9 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(this.nesting, kFilename, this.testNumber, this.name, { + __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/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/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..4500f5dacb8214 100644 --- a/test/parallel/test-runner-output.mjs +++ b/test/parallel/test-runner-output.mjs @@ -41,6 +41,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' }, { name: 'test-runner/output/no_refs.js' }, { name: 'test-runner/output/no_tests.js' }, { name: 'test-runner/output/only_tests.js' }, From fda3b9202407322688e8e6322396a946d783f1c3 Mon Sep 17 00:00:00 2001 From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Tue, 25 Jul 2023 11:23:31 +0300 Subject: [PATCH 02/15] add snapshot --- lib/internal/test_runner/test.js | 2 +- ...global-after-should-fail-the-test.snapshot | 34 +++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) 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 ae8dda77f75ce9..d0216678ee0ce1 100644 --- a/lib/internal/test_runner/test.js +++ b/lib/internal/test_runner/test.js @@ -692,7 +692,7 @@ class Test extends AsyncResource { this.reporter.fail(this.nesting, kFilename, this.testNumber, this.name, { __proto__: null, duration_ms: this.#duration(), - error: this.error + error: this.error, }, undefined); } 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..5cd43a41a26109 --- /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 0 - + --- + duration_ms: * + failureType: 'hookFailed' + error: 'failed running after hook' + code: 'ERR_TEST_FAILURE' + stack: |- + * + * + * + * + * + * + * + * + * + * + ... +1..1 +# tests 1 +# suites 0 +# pass 1 +# fail 0 +# cancelled 0 +# skipped 0 +# todo 0 +# duration_ms * From 29f7f71cf5abd58a13220805788dd686cf1a51d9 Mon Sep 17 00:00:00 2001 From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Tue, 25 Jul 2023 21:12:37 +0300 Subject: [PATCH 03/15] trigger ci From 4781d16f7a8fcc39aaeafd47af5b910f44172757 Mon Sep 17 00:00:00 2001 From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Tue, 25 Jul 2023 22:26:53 +0300 Subject: [PATCH 04/15] output the filename as failing and not the root --- lib/internal/test_runner/test.js | 2 +- ...ld-fail-the-test.js => global_after_should_fail_the_test.js} | 0 ...test.snapshot => global_after_should_fail_the_test.snapshot} | 2 +- 3 files changed, 2 insertions(+), 2 deletions(-) rename test/fixtures/test-runner/output/{global-after-should-fail-the-test.js => global_after_should_fail_the_test.js} (100%) rename test/fixtures/test-runner/output/{global-after-should-fail-the-test.snapshot => global_after_should_fail_the_test.snapshot} (75%) diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index d0216678ee0ce1..f1f630a62d637f 100644 --- a/lib/internal/test_runner/test.js +++ b/lib/internal/test_runner/test.js @@ -689,7 +689,7 @@ class Test extends AsyncResource { this.parent.processPendingSubtests(); } else if (!this.reported) { if (!this.passed && failed === 0 && this.error) { - this.reporter.fail(this.nesting, kFilename, this.testNumber, this.name, { + this.reporter.fail(0, kFilename, NaN, kFilename, { __proto__: null, duration_ms: this.#duration(), error: this.error, 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 similarity index 100% rename from test/fixtures/test-runner/output/global-after-should-fail-the-test.js rename to test/fixtures/test-runner/output/global_after_should_fail_the_test.js 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 similarity index 75% rename from test/fixtures/test-runner/output/global-after-should-fail-the-test.snapshot rename to test/fixtures/test-runner/output/global_after_should_fail_the_test.snapshot index 5cd43a41a26109..9307c1e3277939 100644 --- 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 @@ -5,7 +5,7 @@ ok 1 - this is a test --- duration_ms: * ... -not ok 0 - +not ok NaN - /Users/rluvaton/dev/open-source/node/node-fork/test/fixtures/test-runner/output/global_after_should_fail_the_test.js --- duration_ms: * failureType: 'hookFailed' From 3050d018cb5d81f97ebeb42073860fde287197ad Mon Sep 17 00:00:00 2001 From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Tue, 25 Jul 2023 22:40:12 +0300 Subject: [PATCH 05/15] dont use NaN --- lib/internal/test_runner/test.js | 2 +- .../output/global_after_should_fail_the_test.snapshot | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index f1f630a62d637f..cc7c81cad88c0d 100644 --- a/lib/internal/test_runner/test.js +++ b/lib/internal/test_runner/test.js @@ -689,7 +689,7 @@ class Test extends AsyncResource { this.parent.processPendingSubtests(); } else if (!this.reported) { if (!this.passed && failed === 0 && this.error) { - this.reporter.fail(0, kFilename, NaN, kFilename, { + this.reporter.fail(0, kFilename, this.subtests.length + 1, kFilename, { __proto__: null, duration_ms: this.#duration(), error: this.error, 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 index 9307c1e3277939..16e92b0e2b80c5 100644 --- 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 @@ -5,7 +5,7 @@ ok 1 - this is a test --- duration_ms: * ... -not ok NaN - /Users/rluvaton/dev/open-source/node/node-fork/test/fixtures/test-runner/output/global_after_should_fail_the_test.js +not ok 2 - /Users/rluvaton/dev/open-source/node/node-fork/test/fixtures/test-runner/output/global_after_should_fail_the_test.js --- duration_ms: * failureType: 'hookFailed' From c81f73638f5fe7c6cbfcfcdc9caec97708bda283 Mon Sep 17 00:00:00 2001 From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Wed, 26 Jul 2023 01:13:07 +0300 Subject: [PATCH 06/15] fix test --- test/common/assertSnapshot.js | 5 +++++ .../global_after_should_fail_the_test.snapshot | 2 +- test/parallel/test-runner-output.mjs | 16 ++++++++++++---- 3 files changed, 18 insertions(+), 5 deletions(-) diff --git a/test/common/assertSnapshot.js b/test/common/assertSnapshot.js index 10a5941b41ff37..04d8070c41676f 100644 --- a/test/common/assertSnapshot.js +++ b/test/common/assertSnapshot.js @@ -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); } @@ -69,6 +73,7 @@ async function spawnAndAssert(filename, transform = (x) => x, { tty = false, ... module.exports = { assertSnapshot, getSnapshotPath, + replaceFullPaths, replaceStackTrace, replaceWindowsLineEndings, replaceWindowsPaths, 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 index 16e92b0e2b80c5..db513691f4b781 100644 --- 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 @@ -5,7 +5,7 @@ ok 1 - this is a test --- duration_ms: * ... -not ok 2 - /Users/rluvaton/dev/open-source/node/node-fork/test/fixtures/test-runner/output/global_after_should_fail_the_test.js +not ok 2 - /test/fixtures/test-runner/output/global_after_should_fail_the_test.js --- duration_ms: * failureType: 'hookFailed' diff --git a/test/parallel/test-runner-output.mjs b/test/parallel/test-runner-output.mjs index 4500f5dacb8214..81dcdb84882a32 100644 --- a/test/parallel/test-runner-output.mjs +++ b/test/parallel/test-runner-output.mjs @@ -24,10 +24,18 @@ 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); +const defaultTransform = snapshot.transform( + snapshot.replaceWindowsLineEndings, + snapshot.replaceStackTrace, + replaceTestDuration, + snapshot.replaceFullPaths +); +const specTransform = snapshot.transform( + replaceSpecDuration, + snapshot.replaceWindowsLineEndings, + snapshot.replaceStackTrace, + snapshot.replaceFullPaths +); const tests = [ From 9e982347666a3e0c40d5c1a4dffafba0cd62c00a Mon Sep 17 00:00:00 2001 From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Thu, 27 Jul 2023 10:05:45 +0300 Subject: [PATCH 07/15] fix windows test error --- test/parallel/test-runner-output.mjs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/parallel/test-runner-output.mjs b/test/parallel/test-runner-output.mjs index 81dcdb84882a32..1cd214436b3e90 100644 --- a/test/parallel/test-runner-output.mjs +++ b/test/parallel/test-runner-output.mjs @@ -25,16 +25,16 @@ function replaceSpecDuration(str) { .replace(stackTraceBasePath, '$3'); } const defaultTransform = snapshot.transform( + snapshot.replaceFullPaths, snapshot.replaceWindowsLineEndings, snapshot.replaceStackTrace, replaceTestDuration, - snapshot.replaceFullPaths ); const specTransform = snapshot.transform( replaceSpecDuration, + snapshot.replaceFullPaths, snapshot.replaceWindowsLineEndings, snapshot.replaceStackTrace, - snapshot.replaceFullPaths ); From eabbfec4b38c27e5b2610f12a4585b62d3ccb04f Mon Sep 17 00:00:00 2001 From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Thu, 27 Jul 2023 10:40:02 +0300 Subject: [PATCH 08/15] trigger ci From a984b1951ade40b95a4a534fc3abade886092c0d Mon Sep 17 00:00:00 2001 From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Thu, 27 Jul 2023 10:51:19 +0300 Subject: [PATCH 09/15] fix windows test error --- test/parallel/test-runner-output.mjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-runner-output.mjs b/test/parallel/test-runner-output.mjs index 1cd214436b3e90..747b0de20c1930 100644 --- a/test/parallel/test-runner-output.mjs +++ b/test/parallel/test-runner-output.mjs @@ -31,8 +31,8 @@ const defaultTransform = snapshot.transform( replaceTestDuration, ); const specTransform = snapshot.transform( - replaceSpecDuration, snapshot.replaceFullPaths, + replaceSpecDuration, snapshot.replaceWindowsLineEndings, snapshot.replaceStackTrace, ); From bb062b06e5b79e59d10c8edff8da106b58e6226c Mon Sep 17 00:00:00 2001 From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Thu, 27 Jul 2023 10:51:42 +0300 Subject: [PATCH 10/15] fix windows test error --- test/parallel/test-runner-output.mjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-runner-output.mjs b/test/parallel/test-runner-output.mjs index 747b0de20c1930..1cd214436b3e90 100644 --- a/test/parallel/test-runner-output.mjs +++ b/test/parallel/test-runner-output.mjs @@ -31,8 +31,8 @@ const defaultTransform = snapshot.transform( replaceTestDuration, ); const specTransform = snapshot.transform( - snapshot.replaceFullPaths, replaceSpecDuration, + snapshot.replaceFullPaths, snapshot.replaceWindowsLineEndings, snapshot.replaceStackTrace, ); From efa7a9c9a86b5ebc23ee3aa2bc51fb034b7a702a Mon Sep 17 00:00:00 2001 From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Fri, 28 Jul 2023 09:47:10 +0300 Subject: [PATCH 11/15] fix windows test error --- test/parallel/test-runner-output.mjs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/test/parallel/test-runner-output.mjs b/test/parallel/test-runner-output.mjs index 1cd214436b3e90..f07af7791a08e3 100644 --- a/test/parallel/test-runner-output.mjs +++ b/test/parallel/test-runner-output.mjs @@ -2,6 +2,7 @@ import * as common from '../common/index.mjs'; import * as fixtures from '../common/fixtures.mjs'; import * as snapshot from '../common/assertSnapshot.js'; import { describe, it } from 'node:test'; +import { replaceWindowsPaths } from '../common/assertSnapshot.js'; const skipForceColors = process.config.variables.icu_gyp_path !== 'tools/icu/icu-generic.gyp' || @@ -25,17 +26,20 @@ function replaceSpecDuration(str) { .replace(stackTraceBasePath, '$3'); } const defaultTransform = snapshot.transform( - snapshot.replaceFullPaths, snapshot.replaceWindowsLineEndings, snapshot.replaceStackTrace, replaceTestDuration, ); const specTransform = snapshot.transform( replaceSpecDuration, - snapshot.replaceFullPaths, snapshot.replaceWindowsLineEndings, snapshot.replaceStackTrace, ); +const withFileNameTransform = snapshot.transform( + defaultTransform, + snapshot.replaceFullPaths, + replaceWindowsPaths +); const tests = [ @@ -49,7 +53,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' }, + { 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' }, From 9337e2de19ac7b9ff2b07794ee3b96b60e915960 Mon Sep 17 00:00:00 2001 From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Fri, 28 Jul 2023 08:40:47 +0000 Subject: [PATCH 12/15] fix import --- test/parallel/test-runner-output.mjs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/parallel/test-runner-output.mjs b/test/parallel/test-runner-output.mjs index f07af7791a08e3..a072c62c3606d8 100644 --- a/test/parallel/test-runner-output.mjs +++ b/test/parallel/test-runner-output.mjs @@ -2,7 +2,6 @@ import * as common from '../common/index.mjs'; import * as fixtures from '../common/fixtures.mjs'; import * as snapshot from '../common/assertSnapshot.js'; import { describe, it } from 'node:test'; -import { replaceWindowsPaths } from '../common/assertSnapshot.js'; const skipForceColors = process.config.variables.icu_gyp_path !== 'tools/icu/icu-generic.gyp' || @@ -38,7 +37,7 @@ const specTransform = snapshot.transform( const withFileNameTransform = snapshot.transform( defaultTransform, snapshot.replaceFullPaths, - replaceWindowsPaths + snapshot.replaceWindowsPaths, ); From 872a1dd212b368d2eb9f491b154dd954f1abcbbc Mon Sep 17 00:00:00 2001 From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Sun, 30 Jul 2023 07:06:56 +0300 Subject: [PATCH 13/15] update snapshot after upgrade --- .../output/global_after_should_fail_the_test.snapshot | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index db513691f4b781..16693c1a8a964b 100644 --- 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 @@ -9,7 +9,7 @@ not ok 2 - /test/fixtures/test-runner/output/global_after_should_fail_the_test.j --- duration_ms: * failureType: 'hookFailed' - error: 'failed running after hook' + error: 'this should fail the test' code: 'ERR_TEST_FAILURE' stack: |- * From e429f6dea495be83c21308aad2ec1142e4b8273b Mon Sep 17 00:00:00 2001 From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Sun, 30 Jul 2023 08:59:29 +0300 Subject: [PATCH 14/15] trigger GitHub Actions CI From d6f6ea010716dfcafd3a3cf2aa2a7b9f814ab72a Mon Sep 17 00:00:00 2001 From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Wed, 2 Aug 2023 12:04:03 +0300 Subject: [PATCH 15/15] test: fix test-runner-output in windows --- test/parallel/test-runner-output.mjs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/parallel/test-runner-output.mjs b/test/parallel/test-runner-output.mjs index a072c62c3606d8..8d5233d2de2441 100644 --- a/test/parallel/test-runner-output.mjs +++ b/test/parallel/test-runner-output.mjs @@ -24,6 +24,11 @@ function replaceSpecDuration(str) { .replaceAll(/duration_ms [0-9.]+/g, 'duration_ms *') .replace(stackTraceBasePath, '$3'); } + +function removeWindowsPathEscaping(str) { + return common.isWindows ? str.replaceAll(/\\\\/g, '\\') : str; +} + const defaultTransform = snapshot.transform( snapshot.replaceWindowsLineEndings, snapshot.replaceStackTrace, @@ -36,6 +41,7 @@ const specTransform = snapshot.transform( ); const withFileNameTransform = snapshot.transform( defaultTransform, + removeWindowsPathEscaping, snapshot.replaceFullPaths, snapshot.replaceWindowsPaths, );