test_runner: scope file-level hooks per file in --test-isolation=none

Author: geeksilva97

lib/internal/test_runner/runner.js

@@ -79,6 +79,7 @@ const {
   kSubtestsFailed,
   kTestCodeFailure,
   kTestTimeoutFailure,
+  Suite,
   Test,
 } = require('internal/test_runner/test');
 const { FastBuffer } = require('internal/buffer');
@@ -905,35 +906,21 @@ 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 {
+
+            // Wrap each file in an implicit suite so that top-level hooks
+            // (e.g. beforeEach, afterEach) are scoped to the file and do
+            // not leak across test files.
+            const fileSuite = root.createSubtest(Suite, testFile, kEmptyObject, async () => {
               await cascadedLoader.import(fileURL, parent, { __proto__: null });
-            } catch (err) {
-              threw = true;
-              importError = err;
-            }
-
-            debug(
-              'loaded "%s": top level test count before = %d and after = %d',
-              testFile,
-              topLevelTestCount,
-              root.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);
-            }
+            }, {
+              __proto__: null,
+              loc: [1, 1, resolve(testFile)],
+            });
+            await fileSuite.buildSuite;
+            startSubtestAfterBootstrap(fileSuite);
 
             topLevelTestCount = root.subtests.length;
           }

test/parallel/test-runner-no-isolation-different-cwd.mjs

@@ -9,26 +9,22 @@ 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,11 @@ 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/);
   assert.match(stdout, /ok 1 - suite two - test/);
 });
 
@@ -45,11 +45,11 @@ 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/);
   assert.match(stdout, /ok 1 - suite two - test/);
 });
 
@@ -68,7 +68,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 +86,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

@@ -12,37 +12,34 @@ const testFiles = [
   fixtures.path('test-runner', 'no-isolation', 'two.test.js'),
 ];
 
+// Each file is wrapped in an implicit suite when using isolation: 'none',
+// so file-level hooks are scoped to their file. Global hooks (registered
+// via --import/--require) still run for all tests.
+const file1 = fixtures.path('test-runner', 'no-isolation', 'one.test.js');
+const file2 = fixtures.path('test-runner', 'no-isolation', 'two.test.js');
 const order = [
   'before(): global',
-
-  'before one: <root>',
   'suite one',
-
-  'before two: <root>',
   'suite two',
 
+  `before one: ${file1}`,
   '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: ${file1}`,
 
+  `before two: ${file2}`,
   '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: ${file2}`,
 
   'after(): global',
-  'after one: <root>',
-  'after two: <root>',
 ].join('\n');
 
 test('use --import (CJS) to define global hooks', async (t) => {

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

@@ -12,30 +12,30 @@ const stream = run({
 });
 
 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);
+
+// Each file is wrapped in an implicit suite when using isolation: 'none',
+// so top-level hooks (before, beforeEach, etc.) are scoped to the file
+// and do not leak across test files.
+const file1 = fixtures.path('test-runner', 'no-isolation', 'one.test.js');
+const file2 = fixtures.path('test-runner', 'no-isolation', 'two.test.js');
 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}`,
 ]);