From 5167a852aca4d42dfe32ce6b832ef58e51d74ac1 Mon Sep 17 00:00:00 2001 From: cjihrig Date: Fri, 22 Mar 2024 16:37:11 -0400 Subject: [PATCH 01/10] test: add error only reporter for node:test This commit introduces a node:test reporter to the common utils. This reporter can be used to silence output other than errors from node:test. This is useful because in Node's own test suite, the output of node:test is included in the output of the Python test runner. Refs: https://github.com/nodejs/node/issues/49120 --- test/common/test-error-reporter.js | 35 ++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 test/common/test-error-reporter.js diff --git a/test/common/test-error-reporter.js b/test/common/test-error-reporter.js new file mode 100644 index 00000000000000..3098b10885f19d --- /dev/null +++ b/test/common/test-error-reporter.js @@ -0,0 +1,35 @@ +'use strict'; +const { relative } = require('node:path'); +const { inspect } = require('node:util'); +const cwd = process.cwd(); + +module.exports = async function* errorReporter(source) { + for await (const event of source) { + if (event.type === 'test:fail') { + const { name, details, line, column, file } = event.data; + let { error } = details; + + if (error?.failureType === 'subtestsFailed') { + // In the interest of keeping things concise, skip failures that are + // only due to nested failures. + continue; + } + + if (error?.code === 'ERR_TEST_FAILURE') { + error = error.cause; + } + + const output = [ + `Test failure: '${name}'`, + ]; + + if (file) { + output.push(`Location: ${relative(cwd, file)}:${line}:${column}`); + } + + output.push(inspect(error)); + output.push('\n'); + yield output.join('\n'); + } + } +}; From f71e6d8d1bfb59177529fe2468d6c48ccc9ccc6a Mon Sep 17 00:00:00 2001 From: cjihrig Date: Fri, 22 Mar 2024 17:17:04 -0400 Subject: [PATCH 02/10] temp - add a failure --- test/parallel/test-runner-cli-concurrency.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/parallel/test-runner-cli-concurrency.js b/test/parallel/test-runner-cli-concurrency.js index ac522d7861c1bb..c6a7a227ce55b4 100644 --- a/test/parallel/test-runner-cli-concurrency.js +++ b/test/parallel/test-runner-cli-concurrency.js @@ -23,6 +23,7 @@ test('concurrency of two', async () => { const args = ['--test', '--test-concurrency=2']; const cp = spawnSync(process.execPath, args, { cwd, env }); assert.match(cp.stderr.toString(), /concurrency: 2,/); + throw new Error('bye'); }); test('isolation=none uses a concurrency of one', async () => { From d004f4637eab2932b2234242c6bb65c3e9b0b575 Mon Sep 17 00:00:00 2001 From: cjihrig Date: Wed, 18 Dec 2024 15:22:45 -0500 Subject: [PATCH 03/10] apply to GHA --- .github/workflows/build-tarball.yml | 2 +- .github/workflows/coverage-linux-without-intl.yml | 2 +- .github/workflows/coverage-linux.yml | 2 +- .github/workflows/doc.yml | 2 +- .github/workflows/test-asan.yml | 2 +- .github/workflows/test-linux.yml | 2 +- .github/workflows/test-macos.yml | 2 +- .github/workflows/test-ubsan.yml | 2 +- 8 files changed, 8 insertions(+), 8 deletions(-) diff --git a/.github/workflows/build-tarball.yml b/.github/workflows/build-tarball.yml index 7d8b38a21f8831..2ed30ba0c6c568 100644 --- a/.github/workflows/build-tarball.yml +++ b/.github/workflows/build-tarball.yml @@ -105,4 +105,4 @@ jobs: - name: Test run: | cd $TAR_DIR - make run-ci -j4 V=1 TEST_CI_ARGS="-p dots --node-args='--test-reporter=spec' --measure-flakiness 9" + make run-ci -j4 V=1 TEST_CI_ARGS="-p dots --node-args='--test-reporter=./test/common/test-error-reporter.js' --measure-flakiness 9" diff --git a/.github/workflows/coverage-linux-without-intl.yml b/.github/workflows/coverage-linux-without-intl.yml index 1977eda3f97e03..e88310dc414cab 100644 --- a/.github/workflows/coverage-linux-without-intl.yml +++ b/.github/workflows/coverage-linux-without-intl.yml @@ -68,7 +68,7 @@ jobs: # TODO(bcoe): fix the couple tests that fail with the inspector enabled. # The cause is most likely coverage's use of the inspector. - name: Test - run: NODE_V8_COVERAGE=coverage/tmp make test-cov -j4 V=1 TEST_CI_ARGS="-p dots --node-args='--test-reporter=spec' --measure-flakiness 9" || exit 0 + run: NODE_V8_COVERAGE=coverage/tmp make test-cov -j4 V=1 TEST_CI_ARGS="-p dots --node-args='--test-reporter=./test/common/test-error-reporter.js' --measure-flakiness 9" || exit 0 - name: Report JS run: npx c8 report --check-coverage env: diff --git a/.github/workflows/coverage-linux.yml b/.github/workflows/coverage-linux.yml index 164c0b540a9f45..770205a317088c 100644 --- a/.github/workflows/coverage-linux.yml +++ b/.github/workflows/coverage-linux.yml @@ -68,7 +68,7 @@ jobs: # TODO(bcoe): fix the couple tests that fail with the inspector enabled. # The cause is most likely coverage's use of the inspector. - name: Test - run: NODE_V8_COVERAGE=coverage/tmp make test-cov -j4 V=1 TEST_CI_ARGS="-p dots --node-args='--test-reporter=spec' --measure-flakiness 9" || exit 0 + run: NODE_V8_COVERAGE=coverage/tmp make test-cov -j4 V=1 TEST_CI_ARGS="-p dots --node-args='--test-reporter=./test/common/test-error-reporter.js' --measure-flakiness 9" || exit 0 - name: Report JS run: npx c8 report --check-coverage env: diff --git a/.github/workflows/doc.yml b/.github/workflows/doc.yml index 5ab4277879ecb9..c86d0c5822f82f 100644 --- a/.github/workflows/doc.yml +++ b/.github/workflows/doc.yml @@ -40,4 +40,4 @@ jobs: name: docs path: out/doc - name: Test - run: NODE=$(command -v node) make test-doc-ci TEST_CI_ARGS="-p actions --node-args='--test-reporter=spec' --node-args='--test-reporter-destination=stdout' --measure-flakiness 9" + run: NODE=$(command -v node) make test-doc-ci TEST_CI_ARGS="-p actions --node-args='--test-reporter=./test/common/test-error-reporter.js' --node-args='--test-reporter-destination=stdout' --measure-flakiness 9" diff --git a/.github/workflows/test-asan.yml b/.github/workflows/test-asan.yml index d918fa7d87300b..e579e9e2fd2275 100644 --- a/.github/workflows/test-asan.yml +++ b/.github/workflows/test-asan.yml @@ -63,4 +63,4 @@ jobs: - name: Build run: make build-ci -j4 V=1 - name: Test - run: make run-ci -j4 V=1 TEST_CI_ARGS="-p actions --node-args='--test-reporter=spec' --node-args='--test-reporter-destination=stdout' -t 300 --measure-flakiness 9" + run: make run-ci -j4 V=1 TEST_CI_ARGS="-p actions --node-args='--test-reporter=./test/common/test-error-reporter.js' --node-args='--test-reporter-destination=stdout' -t 300 --measure-flakiness 9" diff --git a/.github/workflows/test-linux.yml b/.github/workflows/test-linux.yml index bfe9885afb7b46..e899e12fdfdefe 100644 --- a/.github/workflows/test-linux.yml +++ b/.github/workflows/test-linux.yml @@ -54,7 +54,7 @@ jobs: - name: Build run: make -C node build-ci -j4 V=1 CONFIG_FLAGS="--error-on-warn" - name: Test - run: make -C node run-ci -j4 V=1 TEST_CI_ARGS="-p actions --node-args='--test-reporter=spec' --node-args='--test-reporter-destination=stdout' --measure-flakiness 9" + run: make -C node run-ci -j4 V=1 TEST_CI_ARGS="-p actions --node-args='--test-reporter=./test/common/test-error-reporter.js' --node-args='--test-reporter-destination=stdout' --measure-flakiness 9" - name: Re-run test in a folder whose name contains unusual chars run: | mv node "$DIR" diff --git a/.github/workflows/test-macos.yml b/.github/workflows/test-macos.yml index f73c0b2e656751..94d0d1585bfc87 100644 --- a/.github/workflows/test-macos.yml +++ b/.github/workflows/test-macos.yml @@ -89,7 +89,7 @@ jobs: - name: Free Space After Build run: df -h - name: Test - run: make -C node run-ci -j$(getconf _NPROCESSORS_ONLN) V=1 TEST_CI_ARGS="-p actions --node-args='--test-reporter=spec' --node-args='--test-reporter-destination=stdout' --measure-flakiness 9" + run: make -C node run-ci -j$(getconf _NPROCESSORS_ONLN) V=1 TEST_CI_ARGS="-p actions --node-args='--test-reporter=./test/common/test-error-reporter.js' --node-args='--test-reporter-destination=stdout' --measure-flakiness 9" - name: Re-run test in a folder whose name contains unusual chars run: | mv node "$DIR" diff --git a/.github/workflows/test-ubsan.yml b/.github/workflows/test-ubsan.yml index 9f33fa670b8231..3587e9d3125666 100644 --- a/.github/workflows/test-ubsan.yml +++ b/.github/workflows/test-ubsan.yml @@ -64,4 +64,4 @@ jobs: - name: Build run: make build-ci -j2 V=1 - name: Test - run: make run-ci -j2 V=1 TEST_CI_ARGS="-p actions --node-args='--test-reporter=spec' --node-args='--test-reporter-destination=stdout' -t 300 --measure-flakiness 9" + run: make run-ci -j2 V=1 TEST_CI_ARGS="-p actions --node-args='--test-reporter=./test/common/test-error-reporter.js' --node-args='--test-reporter-destination=stdout' -t 300 --measure-flakiness 9" From d34f6497dcdf42a008e4322256ea57441e8e9e12 Mon Sep 17 00:00:00 2001 From: Carlos Espa Date: Thu, 2 Jan 2025 18:58:54 +0100 Subject: [PATCH 04/10] fixup! test: add error only reporter for node:test --- test/common/test-error-reporter.js | 6 ++++ .../error-reporter-fail-fast/a.mjs | 6 ++++ .../error-reporter-fail-fast/b.mjs | 6 ++++ test/parallel/test-runner-error-reporter.js | 33 +++++++++++++++++++ 4 files changed, 51 insertions(+) create mode 100644 test/fixtures/test-runner/error-reporter-fail-fast/a.mjs create mode 100644 test/fixtures/test-runner/error-reporter-fail-fast/b.mjs create mode 100644 test/parallel/test-runner-error-reporter.js diff --git a/test/common/test-error-reporter.js b/test/common/test-error-reporter.js index 3098b10885f19d..fed92d9776c8a8 100644 --- a/test/common/test-error-reporter.js +++ b/test/common/test-error-reporter.js @@ -30,6 +30,12 @@ module.exports = async function* errorReporter(source) { output.push(inspect(error)); output.push('\n'); yield output.join('\n'); + + if (process.env.FAIL_FAST) { + yield `\n\u001b[31m✖ Bailing on failed test: ${event.data.name}\u001b[0m\n`; + process.exitCode = 1; + process.emit('SIGINT'); + } } } }; diff --git a/test/fixtures/test-runner/error-reporter-fail-fast/a.mjs b/test/fixtures/test-runner/error-reporter-fail-fast/a.mjs new file mode 100644 index 00000000000000..6508394bbc682d --- /dev/null +++ b/test/fixtures/test-runner/error-reporter-fail-fast/a.mjs @@ -0,0 +1,6 @@ +const assert = require('node:assert'); +const { test } = require('node:test'); + +test('fail', () => { + assert.fail('a.mjs fail'); +}); diff --git a/test/fixtures/test-runner/error-reporter-fail-fast/b.mjs b/test/fixtures/test-runner/error-reporter-fail-fast/b.mjs new file mode 100644 index 00000000000000..87abd62db2773f --- /dev/null +++ b/test/fixtures/test-runner/error-reporter-fail-fast/b.mjs @@ -0,0 +1,6 @@ +const assert = require('node:assert'); +const { test } = require('node:test'); + +test('fail', () => { + assert.fail('b.mjs fail'); +}); diff --git a/test/parallel/test-runner-error-reporter.js b/test/parallel/test-runner-error-reporter.js new file mode 100644 index 00000000000000..c99a9b42cb0b03 --- /dev/null +++ b/test/parallel/test-runner-error-reporter.js @@ -0,0 +1,33 @@ +'use strict'; + +require('../common'); +const fixtures = require('../common/fixtures'); +const assert = require('node:assert'); +const { spawnSync } = require('node:child_process'); +const { test } = require('node:test'); +const cwd = fixtures.path('test-runner', 'error-reporter-fail-fast'); +const env = { ...process.env }; + +test('all tests failures reported without FAIL_FAST flag', async () => { + const args = [ + '--test-reporter=./test/common/test-error-reporter.js', + '--test-concurrency=1', + '--test', + `${cwd}/*.mjs`, + ]; + const cp = spawnSync(process.execPath, args, { env }); + const failureCount = (cp.stdout.toString().match(/Test failure:/g) || []).length; + assert.strictEqual(failureCount, 2); +}); + +test('FAIL_FAST stops test execution after first failure', async () => { + const args = [ + '--test-reporter=./test/common/test-error-reporter.js', + '--test-concurrency=1', + '--test', + `${cwd}/*.mjs`, + ]; + const cp = spawnSync(process.execPath, args, { env: { ...env, FAIL_FAST: 'true' } }); + const failureCount = (cp.stdout.toString().match(/Test failure:/g) || []).length; + assert.strictEqual(failureCount, 1); +}); From 0e1ecd17556c77b80b2594c666caa601f0516b58 Mon Sep 17 00:00:00 2001 From: Carlos Espa <43477095+Ceres6@users.noreply.github.com> Date: Tue, 7 Jan 2025 19:26:04 +0100 Subject: [PATCH 05/10] remove example test fail --- test/parallel/test-runner-cli-concurrency.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/parallel/test-runner-cli-concurrency.js b/test/parallel/test-runner-cli-concurrency.js index c6a7a227ce55b4..ac522d7861c1bb 100644 --- a/test/parallel/test-runner-cli-concurrency.js +++ b/test/parallel/test-runner-cli-concurrency.js @@ -23,7 +23,6 @@ test('concurrency of two', async () => { const args = ['--test', '--test-concurrency=2']; const cp = spawnSync(process.execPath, args, { cwd, env }); assert.match(cp.stderr.toString(), /concurrency: 2,/); - throw new Error('bye'); }); test('isolation=none uses a concurrency of one', async () => { From 63c7facbabf5db10ba2903420618952dc9d8655d Mon Sep 17 00:00:00 2001 From: Carlos Espa <43477095+Ceres6@users.noreply.github.com> Date: Fri, 10 Jan 2025 10:00:01 +0100 Subject: [PATCH 06/10] Apply suggestions from code review Co-authored-by: Colin Ihrig --- test/common/test-error-reporter.js | 2 +- test/parallel/test-runner-error-reporter.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/common/test-error-reporter.js b/test/common/test-error-reporter.js index fed92d9776c8a8..d6db28e675ad6f 100644 --- a/test/common/test-error-reporter.js +++ b/test/common/test-error-reporter.js @@ -32,7 +32,7 @@ module.exports = async function* errorReporter(source) { yield output.join('\n'); if (process.env.FAIL_FAST) { - yield `\n\u001b[31m✖ Bailing on failed test: ${event.data.name}\u001b[0m\n`; + yield `\nBailing on failed test: ${event.data.name}\n`; process.exitCode = 1; process.emit('SIGINT'); } diff --git a/test/parallel/test-runner-error-reporter.js b/test/parallel/test-runner-error-reporter.js index c99a9b42cb0b03..2648743e5d6ff3 100644 --- a/test/parallel/test-runner-error-reporter.js +++ b/test/parallel/test-runner-error-reporter.js @@ -15,7 +15,7 @@ test('all tests failures reported without FAIL_FAST flag', async () => { '--test', `${cwd}/*.mjs`, ]; - const cp = spawnSync(process.execPath, args, { env }); + const cp = spawnSync(process.execPath, args); const failureCount = (cp.stdout.toString().match(/Test failure:/g) || []).length; assert.strictEqual(failureCount, 2); }); @@ -27,7 +27,7 @@ test('FAIL_FAST stops test execution after first failure', async () => { '--test', `${cwd}/*.mjs`, ]; - const cp = spawnSync(process.execPath, args, { env: { ...env, FAIL_FAST: 'true' } }); + const cp = spawnSync(process.execPath, args, { env: { ...process.env, FAIL_FAST: 'true' } }); const failureCount = (cp.stdout.toString().match(/Test failure:/g) || []).length; assert.strictEqual(failureCount, 1); }); From 01f7d0971c11299ca88dd050bd5d9b696d70c14e Mon Sep 17 00:00:00 2001 From: cjihrig Date: Wed, 18 Dec 2024 15:22:45 -0500 Subject: [PATCH 07/10] apply to GHA --- .github/workflows/test-macos.yml | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/.github/workflows/test-macos.yml b/.github/workflows/test-macos.yml index 94d0d1585bfc87..7fb85024b49183 100644 --- a/.github/workflows/test-macos.yml +++ b/.github/workflows/test-macos.yml @@ -89,11 +89,4 @@ jobs: - name: Free Space After Build run: df -h - name: Test - run: make -C node run-ci -j$(getconf _NPROCESSORS_ONLN) V=1 TEST_CI_ARGS="-p actions --node-args='--test-reporter=./test/common/test-error-reporter.js' --node-args='--test-reporter-destination=stdout' --measure-flakiness 9" - - name: Re-run test in a folder whose name contains unusual chars - run: | - mv node "$DIR" - cd "$DIR" - ./tools/test.py --flaky-tests keep_retrying -p actions -j 4 - env: - DIR: dir%20with $unusual"chars?'åß∂ƒ©∆¬…` + run: make run-ci -j$(getconf _NPROCESSORS_ONLN) V=1 TEST_CI_ARGS="-p actions --node-args='--test-reporter=./test/common/test-error-reporter.js' --node-args='--test-reporter-destination=stdout' --measure-flakiness 9" From 0ce6e5cdf975d2750028a896821f156ce34756a3 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Mon, 30 Dec 2024 01:17:44 +0100 Subject: [PATCH 08/10] test: use unusual chars in the path to ensure our tests are robust PR-URL: https://github.com/nodejs/node/pull/48409 Reviewed-By: Jacob Smith --- .github/workflows/test-macos.yml | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/.github/workflows/test-macos.yml b/.github/workflows/test-macos.yml index 7fb85024b49183..f73c0b2e656751 100644 --- a/.github/workflows/test-macos.yml +++ b/.github/workflows/test-macos.yml @@ -89,4 +89,11 @@ jobs: - name: Free Space After Build run: df -h - name: Test - run: make run-ci -j$(getconf _NPROCESSORS_ONLN) V=1 TEST_CI_ARGS="-p actions --node-args='--test-reporter=./test/common/test-error-reporter.js' --node-args='--test-reporter-destination=stdout' --measure-flakiness 9" + run: make -C node run-ci -j$(getconf _NPROCESSORS_ONLN) V=1 TEST_CI_ARGS="-p actions --node-args='--test-reporter=spec' --node-args='--test-reporter-destination=stdout' --measure-flakiness 9" + - name: Re-run test in a folder whose name contains unusual chars + run: | + mv node "$DIR" + cd "$DIR" + ./tools/test.py --flaky-tests keep_retrying -p actions -j 4 + env: + DIR: dir%20with $unusual"chars?'åß∂ƒ©∆¬…` From d2f1d56d18440a563ea795461278a3f2a8f64637 Mon Sep 17 00:00:00 2001 From: cjihrig Date: Wed, 18 Dec 2024 15:22:45 -0500 Subject: [PATCH 09/10] apply to GHA --- .github/workflows/test-macos.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test-macos.yml b/.github/workflows/test-macos.yml index f73c0b2e656751..94d0d1585bfc87 100644 --- a/.github/workflows/test-macos.yml +++ b/.github/workflows/test-macos.yml @@ -89,7 +89,7 @@ jobs: - name: Free Space After Build run: df -h - name: Test - run: make -C node run-ci -j$(getconf _NPROCESSORS_ONLN) V=1 TEST_CI_ARGS="-p actions --node-args='--test-reporter=spec' --node-args='--test-reporter-destination=stdout' --measure-flakiness 9" + run: make -C node run-ci -j$(getconf _NPROCESSORS_ONLN) V=1 TEST_CI_ARGS="-p actions --node-args='--test-reporter=./test/common/test-error-reporter.js' --node-args='--test-reporter-destination=stdout' --measure-flakiness 9" - name: Re-run test in a folder whose name contains unusual chars run: | mv node "$DIR" From 4547f09f718c2862c2c1e8d1f7f0a972b34209fc Mon Sep 17 00:00:00 2001 From: Carlos Espa Date: Fri, 10 Jan 2025 10:15:05 +0100 Subject: [PATCH 10/10] fixup! apply to GHA --- test/parallel/test-runner-error-reporter.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/parallel/test-runner-error-reporter.js b/test/parallel/test-runner-error-reporter.js index 2648743e5d6ff3..84ae37fbc86a9d 100644 --- a/test/parallel/test-runner-error-reporter.js +++ b/test/parallel/test-runner-error-reporter.js @@ -6,7 +6,6 @@ const assert = require('node:assert'); const { spawnSync } = require('node:child_process'); const { test } = require('node:test'); const cwd = fixtures.path('test-runner', 'error-reporter-fail-fast'); -const env = { ...process.env }; test('all tests failures reported without FAIL_FAST flag', async () => { const args = [