test_runner: scope file-level hooks per file in no-isolation mode

Signed-off-by: marcopiraccini <marco.piraccini@gmail.com>

Author: marcopiraccini

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`
 

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

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);
           }
         });
 

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: <root>',
   'suite one',
-  'before two: <root>',
   '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: <root>',
-  'after two: <root>',
+  'after two: two.test.js',
 ]);

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/);
 });

test/parallel/test-runner-no-isolation-hooks.mjs

@@ -15,34 +15,30 @@ const testFiles = [
 const order = [
   'before(): global',
 
-  'before one: <root>',
   'suite one',
 
-  'before two: <root>',
   '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: <root>',
-  'after two: <root>',
 ].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);
 });

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: <root>',
   'suite one',
-  'before two: <root>',
   '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: <root>',
-  'after two: <root>',
+  `after two: ${file2}`,
 ]);