From a0750d0714db70747a9b6932c7e01cadd0e128db Mon Sep 17 00:00:00 2001 From: marcopiraccini Date: Mon, 6 Apr 2026 15:52:18 +0200 Subject: [PATCH] test_runner: scope file-level hooks per file in no-isolation mode Signed-off-by: marcopiraccini --- doc/api/cli.md | 9 ++-- doc/api/test.md | 6 ++- lib/internal/test_runner/runner.js | 43 ++++++++----------- ...test-runner-no-isolation-different-cwd.mjs | 16 +++---- .../test-runner-no-isolation-filtering.js | 12 ++---- .../test-runner-no-isolation-hooks.mjs | 22 ++++------ test/parallel/test-runner-no-isolation.mjs | 19 ++++---- 7 files changed, 57 insertions(+), 70 deletions(-) diff --git a/doc/api/cli.md b/doc/api/cli.md index 18576367d09720..c03236eb08e5ed 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -2744,9 +2744,12 @@ changes: Configures the type of test isolation used in the test runner. When `mode` is `'process'`, each test file is run in a separate child process. When `mode` is -`'none'`, all test files run in the same process as the test runner. The default -isolation mode is `'process'`. This flag is ignored if the `--test` flag is not -present. See the [test runner execution model][] section for more information. +`'none'`, all test files run in the same process as the test runner. Each test +file is wrapped in an implicit suite, so that lifecycle hooks such as +`beforeEach()` and `afterEach()` declared at the top level of a file only apply +to tests within that file. The default isolation mode is `'process'`. This flag +is ignored if the `--test` flag is not present. See the +[test runner execution model][] section for more information. ### `--test-name-pattern` diff --git a/doc/api/test.md b/doc/api/test.md index a9ad7fc5872065..9ab9c1cd1ca6fc 100644 --- a/doc/api/test.md +++ b/doc/api/test.md @@ -1631,8 +1631,10 @@ changes: spawned. **Default:** `undefined`. * `isolation` {string} Configures the type of test isolation. If set to `'process'`, each test file is run in a separate child process. If set to - `'none'`, all test files run in the current process. **Default:** - `'process'`. + `'none'`, all test files run in the current process. Each test file is + wrapped in an implicit suite, so that lifecycle hooks such as + `beforeEach()` and `afterEach()` declared at the top level of a file only + apply to tests within that file. **Default:** `'process'`. * `only` {boolean} If truthy, the test context will only run tests that have the `only` option set * `setup` {Function} A function that accepts the `TestsStream` instance diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js index bb3a8868309a94..1c850dec5615b9 100644 --- a/lib/internal/test_runner/runner.js +++ b/lib/internal/test_runner/runner.js @@ -80,6 +80,7 @@ const { kSubtestsFailed, kTestCodeFailure, kTestTimeoutFailure, + Suite, Test, } = require('internal/test_runner/test'); const { FastBuffer } = require('internal/buffer'); @@ -949,7 +950,6 @@ function run(options = kEmptyObject) { await root.runInAsyncScope(async () => { const parentURL = pathToFileURL(cwd + sep).href; const cascadedLoader = esmLoader.getOrInitializeCascadedLoader(); - let topLevelTestCount = 0; root.harness.bootstrapPromise = root.harness.bootstrapPromise ? SafePromiseAllReturnVoid([root.harness.bootstrapPromise, promise]) : @@ -974,37 +974,32 @@ function run(options = kEmptyObject) { const testFile = testFiles[i]; const fileURL = pathToFileURL(resolve(cwd, testFile)); const parent = i === 0 ? undefined : parentURL; - let threw = false; - let importError; root.entryFile = resolve(testFile); debug('loading test file:', fileURL.href); - try { - await cascadedLoader.import(fileURL, parent, { __proto__: null }); - } catch (err) { - threw = true; - importError = err; - } + + // Wrap each file in an implicit suite so that top-level hooks + // (beforeEach, afterEach, before, after) declared in one file + // do not leak into tests from other files. + const fileSuite = root.createSubtest(Suite, testFile, kEmptyObject, async () => { + try { + await cascadedLoader.import(fileURL, parent, { __proto__: null }); + } catch (err) { + fileSuite.fail(err); + } + }, { + __proto__: null, + loc: [1, 1, resolve(testFile)], + }); + await fileSuite.buildSuite; debug( - 'loaded "%s": top level test count before = %d and after = %d', + 'loaded "%s": file suite subtest count = %d', testFile, - topLevelTestCount, - root.subtests.length, + fileSuite.subtests.length, ); - if (topLevelTestCount === root.subtests.length) { - // This file had no tests in it. Add the placeholder test. - const subtest = root.createSubtest(Test, testFile, kEmptyObject, undefined, { - __proto__: null, - loc: [1, 1, resolve(testFile)], - }); - if (threw) { - subtest.fail(importError); - } - startSubtestAfterBootstrap(subtest); - } - topLevelTestCount = root.subtests.length; + startSubtestAfterBootstrap(fileSuite); } }); diff --git a/test/parallel/test-runner-no-isolation-different-cwd.mjs b/test/parallel/test-runner-no-isolation-different-cwd.mjs index 9138c480bf3e54..df82b1511b5dae 100644 --- a/test/parallel/test-runner-no-isolation-different-cwd.mjs +++ b/test/parallel/test-runner-no-isolation-different-cwd.mjs @@ -9,26 +9,24 @@ const stream = run({ }); -stream.on('test:pass', mustCall(4)); +stream.on('test:pass', mustCall(6)); // eslint-disable-next-line no-unused-vars for await (const _ of stream); allowGlobals(globalThis.GLOBAL_ORDER); assert.deepStrictEqual(globalThis.GLOBAL_ORDER, [ - 'before one: ', 'suite one', - 'before two: ', 'suite two', + + 'before one: one.test.js', 'beforeEach one: suite one - test', - 'beforeEach two: suite one - test', 'suite one - test', 'afterEach one: suite one - test', - 'afterEach two: suite one - test', + 'after one: one.test.js', + + 'before two: two.test.js', 'before suite two: suite two', - 'beforeEach one: suite two - test', 'beforeEach two: suite two - test', 'suite two - test', - 'afterEach one: suite two - test', 'afterEach two: suite two - test', - 'after one: ', - 'after two: ', + 'after two: two.test.js', ]); diff --git a/test/parallel/test-runner-no-isolation-filtering.js b/test/parallel/test-runner-no-isolation-filtering.js index e26130c56b196d..666637d231617c 100644 --- a/test/parallel/test-runner-no-isolation-filtering.js +++ b/test/parallel/test-runner-no-isolation-filtering.js @@ -23,11 +23,9 @@ test('works with --test-only', () => { assert.strictEqual(child.status, 0); assert.strictEqual(child.signal, null); assert.match(stdout, /# tests 2/); - assert.match(stdout, /# suites 2/); + assert.match(stdout, /# suites 4/); assert.match(stdout, /# pass 2/); - assert.match(stdout, /ok 1 - suite one/); assert.match(stdout, /ok 1 - suite one - test/); - assert.match(stdout, /ok 2 - suite two/); assert.match(stdout, /ok 1 - suite two - test/); }); @@ -45,11 +43,9 @@ test('works without --test-only', () => { assert.strictEqual(child.status, 0); assert.strictEqual(child.signal, null); assert.match(stdout, /# tests 2/); - assert.match(stdout, /# suites 2/); + assert.match(stdout, /# suites 4/); assert.match(stdout, /# pass 2/); - assert.match(stdout, /ok 1 - suite one/); assert.match(stdout, /ok 1 - suite one - test/); - assert.match(stdout, /ok 2 - suite two/); assert.match(stdout, /ok 1 - suite two - test/); }); @@ -68,7 +64,7 @@ test('works with --test-name-pattern', () => { assert.strictEqual(child.status, 0); assert.strictEqual(child.signal, null); assert.match(stdout, /# tests 0/); - assert.match(stdout, /# suites 0/); + assert.match(stdout, /# suites 1/); }); test('works with --test-skip-pattern', () => { @@ -86,7 +82,7 @@ test('works with --test-skip-pattern', () => { assert.strictEqual(child.status, 0); assert.strictEqual(child.signal, null); assert.match(stdout, /# tests 1/); - assert.match(stdout, /# suites 1/); + assert.match(stdout, /# suites 2/); assert.match(stdout, /# pass 1/); assert.match(stdout, /ok 1 - suite two - test/); }); diff --git a/test/parallel/test-runner-no-isolation-hooks.mjs b/test/parallel/test-runner-no-isolation-hooks.mjs index d67195bae832c7..2a09afd5bd8458 100644 --- a/test/parallel/test-runner-no-isolation-hooks.mjs +++ b/test/parallel/test-runner-no-isolation-hooks.mjs @@ -15,34 +15,30 @@ const testFiles = [ const order = [ 'before(): global', - 'before one: ', 'suite one', - 'before two: ', 'suite two', + `before one: ${testFiles[0]}`, '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', + 'afterEach(): global', + `after one: ${testFiles[0]}`, + `before two: ${testFiles[1]}`, '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', + 'afterEach(): global', + `after two: ${testFiles[1]}`, 'after(): global', - 'after one: ', - 'after two: ', ].join('\n'); test('use --import (CJS) to define global hooks', async (t) => { @@ -52,7 +48,7 @@ test('use --import (CJS) to define global hooks', async (t) => { ...testFiles, ]); - const testHookOutput = stdout.split('\n▶')[0]; + const testHookOutput = stdout.split('\nafter(): global')[0] + '\nafter(): global'; t.assert.equal(testHookOutput, order); }); @@ -64,7 +60,7 @@ test('use --import (ESM) to define global hooks', async (t) => { ...testFiles, ]); - const testHookOutput = stdout.split('\n▶')[0]; + const testHookOutput = stdout.split('\nafter(): global')[0] + '\nafter(): global'; t.assert.equal(testHookOutput, order); }); @@ -76,7 +72,7 @@ test('use --require to define global hooks', async (t) => { ...testFiles, ]); - const testHookOutput = stdout.split('\n▶')[0]; + const testHookOutput = stdout.split('\nafter(): global')[0] + '\nafter(): global'; t.assert.equal(testHookOutput, order); }); diff --git a/test/parallel/test-runner-no-isolation.mjs b/test/parallel/test-runner-no-isolation.mjs index 6f0aba6b95486b..e9e3022bf0b129 100644 --- a/test/parallel/test-runner-no-isolation.mjs +++ b/test/parallel/test-runner-no-isolation.mjs @@ -11,31 +11,28 @@ const stream = run({ isolation: 'none', }); +const file1 = fixtures.path('test-runner', 'no-isolation', 'one.test.js'); +const file2 = fixtures.path('test-runner', 'no-isolation', 'two.test.js'); + stream.on('test:fail', mustNotCall()); -stream.on('test:pass', mustCall(4)); +stream.on('test:pass', mustCall(6)); // eslint-disable-next-line no-unused-vars for await (const _ of stream); allowGlobals(globalThis.GLOBAL_ORDER); assert.deepStrictEqual(globalThis.GLOBAL_ORDER, [ - 'before one: ', 'suite one', - 'before two: ', 'suite two', + `before one: ${file1}`, 'beforeEach one: suite one - test', - 'beforeEach two: suite one - test', 'suite one - test', 'afterEach one: suite one - test', - 'afterEach two: suite one - test', + `after one: ${file1}`, + `before two: ${file2}`, 'before suite two: suite two', - - 'beforeEach one: suite two - test', 'beforeEach two: suite two - test', 'suite two - test', - 'afterEach one: suite two - test', 'afterEach two: suite two - test', - - 'after one: ', - 'after two: ', + `after two: ${file2}`, ]);