From cd6d33f27f0868739896dfdbc030865c89ec27c2 Mon Sep 17 00:00:00 2001 From: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com> Date: Sat, 22 Mar 2025 23:48:27 +0100 Subject: [PATCH 1/3] test: fix dangling promise in test_runner no isolation test setup --- .../test-runner/no-isolation/global-hooks.cjs | 6 ++++++ .../test-runner/no-isolation/global-hooks.js | 6 ------ .../test-runner/no-isolation/global-hooks.mjs | 6 ++++++ .../test-runner-no-isolation-hooks.mjs | 18 +++++++++++------- 4 files changed, 23 insertions(+), 13 deletions(-) create mode 100644 test/fixtures/test-runner/no-isolation/global-hooks.cjs delete mode 100644 test/fixtures/test-runner/no-isolation/global-hooks.js create mode 100644 test/fixtures/test-runner/no-isolation/global-hooks.mjs diff --git a/test/fixtures/test-runner/no-isolation/global-hooks.cjs b/test/fixtures/test-runner/no-isolation/global-hooks.cjs new file mode 100644 index 00000000000000..9a2c7f9950efe7 --- /dev/null +++ b/test/fixtures/test-runner/no-isolation/global-hooks.cjs @@ -0,0 +1,6 @@ +const test = require('node:test'); + +test.before(() => console.log('before(): global')); +test.beforeEach(() => console.log('beforeEach(): global')); +test.after(() => console.log('after(): global')); +test.afterEach(() => console.log('afterEach(): global')); diff --git a/test/fixtures/test-runner/no-isolation/global-hooks.js b/test/fixtures/test-runner/no-isolation/global-hooks.js deleted file mode 100644 index cc8fab98603a2f..00000000000000 --- a/test/fixtures/test-runner/no-isolation/global-hooks.js +++ /dev/null @@ -1,6 +0,0 @@ -import('node:test').then((test) => { - test.before(() => console.log('before(): global')); - test.beforeEach(() => console.log('beforeEach(): global')); - test.after(() => console.log('after(): global')); - test.afterEach(() => console.log('afterEach(): global')); -}); diff --git a/test/fixtures/test-runner/no-isolation/global-hooks.mjs b/test/fixtures/test-runner/no-isolation/global-hooks.mjs new file mode 100644 index 00000000000000..962c6fecb3975f --- /dev/null +++ b/test/fixtures/test-runner/no-isolation/global-hooks.mjs @@ -0,0 +1,6 @@ +import test from 'node:test'; + +test.before(() => console.log('before(): global')); +test.beforeEach(() => console.log('beforeEach(): global')); +test.after(() => console.log('after(): global')); +test.afterEach(() => console.log('afterEach(): global')); diff --git a/test/parallel/test-runner-no-isolation-hooks.mjs b/test/parallel/test-runner-no-isolation-hooks.mjs index 0c07737c4decf4..d90e5a98ed6068 100644 --- a/test/parallel/test-runner-no-isolation-hooks.mjs +++ b/test/parallel/test-runner-no-isolation-hooks.mjs @@ -43,24 +43,28 @@ const order = [ 'after(): global', 'after one: ', 'after two: ', -]; +].join('\n'); test('Using --require to define global hooks works', async (t) => { - const spawned = await common.spawnPromisified(process.execPath, [ + const { stdout } = await common.spawnPromisified(process.execPath, [ ...testArguments, - '--require', fixtures.path('test-runner', 'no-isolation', 'global-hooks.js'), + '--require', fixtures.path('test-runner', 'no-isolation', 'global-hooks.cjs'), ...testFiles, ]); - t.assert.ok(spawned.stdout.includes(order.join('\n'))); + const testHookOutput = stdout.split('\n▶')[0]; + + t.assert.equal(testHookOutput, order); }); test('Using --import to define global hooks works', async (t) => { - const spawned = await common.spawnPromisified(process.execPath, [ + const { stdout } = await common.spawnPromisified(process.execPath, [ ...testArguments, - '--import', fixtures.fileURL('test-runner', 'no-isolation', 'global-hooks.js'), + '--import', fixtures.fileURL('test-runner', 'no-isolation', 'global-hooks.mjs'), ...testFiles, ]); - t.assert.ok(spawned.stdout.includes(order.join('\n'))); + const testHookOutput = stdout.split('\n▶')[0]; + + t.assert.equal(testHookOutput, order); }); From 6a38d3246b89fe3d101b085187a9f2df662fc65c Mon Sep 17 00:00:00 2001 From: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com> Date: Mon, 24 Mar 2025 09:40:54 +0100 Subject: [PATCH 2/3] fixup!: add import cjs case --- test/parallel/test-runner-no-isolation-hooks.mjs | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/test/parallel/test-runner-no-isolation-hooks.mjs b/test/parallel/test-runner-no-isolation-hooks.mjs index d90e5a98ed6068..9f505795d663b5 100644 --- a/test/parallel/test-runner-no-isolation-hooks.mjs +++ b/test/parallel/test-runner-no-isolation-hooks.mjs @@ -45,7 +45,7 @@ const order = [ 'after two: ', ].join('\n'); -test('Using --require to define global hooks works', async (t) => { +test('use --require to define global hooks', async (t) => { const { stdout } = await common.spawnPromisified(process.execPath, [ ...testArguments, '--require', fixtures.path('test-runner', 'no-isolation', 'global-hooks.cjs'), @@ -57,7 +57,19 @@ test('Using --require to define global hooks works', async (t) => { t.assert.equal(testHookOutput, order); }); -test('Using --import to define global hooks works', async (t) => { +test('use --import (CJS) to define global hooks', async (t) => { + const { stdout } = await common.spawnPromisified(process.execPath, [ + ...testArguments, + '--import', fixtures.fileURL('test-runner', 'no-isolation', 'global-hooks.cjs'), + ...testFiles, + ]); + + const testHookOutput = stdout.split('\n▶')[0]; + + t.assert.equal(testHookOutput, order); +}); + +test('use --import (ESM) to define global hooks', async (t) => { const { stdout } = await common.spawnPromisified(process.execPath, [ ...testArguments, '--import', fixtures.fileURL('test-runner', 'no-isolation', 'global-hooks.mjs'), From aee5b62bcccde02cb109f4567961eb6b0d91109d Mon Sep 17 00:00:00 2001 From: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com> Date: Wed, 2 Apr 2025 12:11:05 +0200 Subject: [PATCH 3/3] fixup!: move test-case to "known issues" --- .../test-runner-no-isolation-hooks.mjs | 68 +++++++++++++++++++ .../test-runner-no-isolation-hooks.mjs | 12 ---- 2 files changed, 68 insertions(+), 12 deletions(-) create mode 100644 test/known_issues/test-runner-no-isolation-hooks.mjs diff --git a/test/known_issues/test-runner-no-isolation-hooks.mjs b/test/known_issues/test-runner-no-isolation-hooks.mjs new file mode 100644 index 00000000000000..7ef087ce0158ab --- /dev/null +++ b/test/known_issues/test-runner-no-isolation-hooks.mjs @@ -0,0 +1,68 @@ +import * as common from '../common/index.mjs'; +import * as fixtures from '../common/fixtures.mjs'; +import { test } from 'node:test'; + +const testArguments = [ + '--test', + '--test-isolation=none', +]; + +const testFiles = [ + fixtures.path('test-runner', 'no-isolation', 'one.test.js'), + fixtures.path('test-runner', 'no-isolation', 'two.test.js'), +]; + +const order = [ + 'before(): global', + + 'before one: ', + 'suite one', + + 'before two: ', + 'suite two', + + 'beforeEach(): global', + 'beforeEach one: suite one - test', + 'beforeEach two: suite one - test', + + 'suite one - test', + 'afterEach(): global', + 'afterEach one: suite one - test', + 'afterEach two: suite one - test', + + 'before suite two: suite two', + 'beforeEach(): global', + 'beforeEach one: suite two - test', + 'beforeEach two: suite two - test', + + 'suite two - test', + 'afterEach(): global', + 'afterEach one: suite two - test', + 'afterEach two: suite two - test', + + 'after(): global', + 'after one: ', + 'after two: ', +].join('\n'); + +/** + * TODO: The `--require` flag is processed in `loadPreloadModules` (process/pre_execution.js) BEFORE + * the root test is created by the test runner. This causes a global `before` hook to register (and + * run) but then the root test-case is created, causing the "subsequent" hooks to get lost. This + * behaviour (CJS route only) is different from the ESM route, where test runner explicitly handles + * `--import` in `root.runInAsyncScope` (test_runner/runner.js). + * @see https://github.com/nodejs/node/pull/57595#issuecomment-2770724492 + * @see https://github.com/nodejs/node/issues/57728 + * Moved from test/parallel/test-runner-no-isolation-hooks.mjs + */ +test('use --require to define global hooks', async (t) => { + const { stdout } = await common.spawnPromisified(process.execPath, [ + ...testArguments, + '--require', fixtures.path('test-runner', 'no-isolation', 'global-hooks.cjs'), + ...testFiles, + ]); + + const testHookOutput = stdout.split('\n▶')[0]; + + t.assert.equal(testHookOutput, order); +}); diff --git a/test/parallel/test-runner-no-isolation-hooks.mjs b/test/parallel/test-runner-no-isolation-hooks.mjs index 9f505795d663b5..7896ae2fec819f 100644 --- a/test/parallel/test-runner-no-isolation-hooks.mjs +++ b/test/parallel/test-runner-no-isolation-hooks.mjs @@ -45,18 +45,6 @@ const order = [ 'after two: ', ].join('\n'); -test('use --require to define global hooks', async (t) => { - const { stdout } = await common.spawnPromisified(process.execPath, [ - ...testArguments, - '--require', fixtures.path('test-runner', 'no-isolation', 'global-hooks.cjs'), - ...testFiles, - ]); - - const testHookOutput = stdout.split('\n▶')[0]; - - t.assert.equal(testHookOutput, order); -}); - test('use --import (CJS) to define global hooks', async (t) => { const { stdout } = await common.spawnPromisified(process.execPath, [ ...testArguments,