test_runner: change root test to be a suite

nodejs · MoLow · Aug 3, 2023 · Jul 26, 2023 · Jul 26, 2023 · Jul 26, 2023
commit a88ec2877b8a989c7008d07409ae1980556fd332
diff --git a/lib/internal/test_runner/harness.js b/lib/internal/test_runner/harness.js
@@ -2,7 +2,6 @@
 const {
   ArrayPrototypeForEach,
   FunctionPrototypeBind,
-  PromiseResolve,
   SafeMap,
 } = primordials;
 const { getCallerLocation } = internalBinding('util');
@@ -23,12 +22,13 @@ const {
   parseCommandLine,
   setupTestReporters,
 } = require('internal/test_runner/utils');
+const { setImmediate } = require('timers');
 const { bigint: hrtime } = process.hrtime;
 
 const testResources = new SafeMap();
 
 function createTestTree(options = kEmptyObject) {
-  return setup(new Test({ __proto__: null, ...options, name: '<root>' }));
+  return setup(new Suite({ __proto__: null, ...options, name: '<root>' }));
 }
 
 function createProcessEventHandler(eventName, rootTest) {
@@ -198,24 +198,24 @@ function getGlobalRoot() {
       }
     });
     reportersSetup = setupTestReporters(globalRoot);
+    setImmediate(() => globalRoot.run());
   }
   return globalRoot;
 }
 
-async function startSubtest(subtest) {
+async function startSubtest(subtest, parent) {
   await reportersSetup;
   getGlobalRoot().harness.bootstrapComplete = true;
-  await subtest.start();
+  if (!(parent instanceof Suite) || (parent.parent === null && parent.endTime !== null)) {
+    await subtest.start();
+  }
 }
 
 function runInParentContext(Factory) {
   function run(name, options, fn, overrides) {
     const parent = testResources.get(executionAsyncId()) || getGlobalRoot();
     const subtest = parent.createSubtest(Factory, name, options, fn, overrides);
-    if (!(parent instanceof Suite)) {
-      return startSubtest(subtest);
-    }
-    return PromiseResolve();
+    return startSubtest(subtest, parent);
   }
 
   const test = (name, options, fn) => {

diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js
@@ -16,7 +16,6 @@ const {
   PromisePrototypeThen,
   SafePromiseAll,
   SafePromiseAllReturnVoid,
-  SafePromiseAllSettledReturnVoid,
   PromiseResolve,
   SafeMap,
   SafeSet,
@@ -378,7 +377,9 @@ function runTestFile(path, root, inspectPort, filesWatcher, testNamePatterns) {
       throw err;
     }
   });
-  return subtest.start();
+  if (filesWatcher) {
+    return subtest.start();
+  }
 }
 
 function watchFiles(testFiles, root, inspectPort, signal, testNamePatterns) {
@@ -476,22 +477,27 @@ function run(options) {
     testFiles = ArrayPrototypeFilter(testFiles, (_, index) => index % shard.total === shard.index - 1);
   }
 
-  let postRun = () => root.postRun();
   let filesWatcher;
   if (watch) {
     filesWatcher = watchFiles(testFiles, root, inspectPort, signal, testNamePatterns);
-    postRun = undefined;
   }
+
   const runFiles = () => {
     root.harness.bootstrapComplete = true;
-    return SafePromiseAllSettledReturnVoid(testFiles, (path) => {
-      const subtest = runTestFile(path, root, inspectPort, filesWatcher, testNamePatterns);
-      filesWatcher?.runningSubtests.set(path, subtest);
-      return subtest;
-    });
+    root.buildPhaseFinished = false;
+    for (let i = 0; i < testFiles.length; i++) {
+      const path = testFiles[i];
+      const enqueued = runTestFile(path, root, inspectPort, filesWatcher, testNamePatterns);
+      filesWatcher?.runningSubtests.set(path, enqueued);
+    }
+    root.buildPhaseFinished = true;
+    if (filesWatcher) {
+      return root.processPendingSubtests();
+    }
+    return PromisePrototypeThen(root.run(), () => root.postRun());
   };
 
-  PromisePrototypeThen(PromisePrototypeThen(PromiseResolve(setup?.(root)), runFiles), postRun);
+  PromisePrototypeThen(PromiseResolve(setup?.(root)), runFiles);
 
   return root.reporter;
 }

diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js
@@ -198,6 +198,10 @@ class SuiteContext {
   get name() {
     return this.#suite.name;
   }
+
+  diagnostic(message) {
+    this.#suite.diagnostic(message);
+  }
 }
 
 class Test extends AsyncResource {
@@ -430,7 +434,7 @@ class Test extends AsyncResource {
 
     // If this test has already ended, attach this test to the root test so
     // that the error can be properly reported.
-    const preventAddingSubtests = this.finished || this.buildPhaseFinished;
+    const preventAddingSubtests = (this.finished || this.buildPhaseFinished) && parent.parent !== null;
     if (preventAddingSubtests) {
       while (parent.parent !== null) {
         parent = parent.parent;
@@ -940,7 +944,7 @@ class Suite extends Test {
         return;
       }
 
-      if (this.parent.hooks.before.length > 0) {
+      if (this.parent?.hooks.before.length > 0) {
         await this.parent.runHook('before', this.parent.getRunArgs());
       }
 
@@ -965,7 +969,9 @@ class Suite extends Test {
       stopPromise?.[SymbolDispose]();
     }
 
-    this.postRun();
+    if (this.parent !== null) {
+      this.postRun();
+    }
   }
 }
 

diff --git a/test/fixtures/test-runner/output/abort.js b/test/fixtures/test-runner/output/abort.js
@@ -2,7 +2,7 @@
 require('../../../common');
 const test = require('node:test');
 
-test('promise timeout signal', { signal: AbortSignal.timeout(1) }, async (t) => {
+test('promise timeout signal', { signal: AbortSignal.timeout(100) }, async (t) => {
   await Promise.all([
     t.test('ok 1', async () => {}),
     t.test('ok 2', () => {}),
@@ -22,7 +22,7 @@ test('promise abort signal', { signal: AbortSignal.abort() }, async (t) => {
   await t.test('should not appear', () => {});
 });
 
-test('callback timeout signal', { signal: AbortSignal.timeout(1) }, (t, done) => {
+test('callback timeout signal', { signal: AbortSignal.timeout(100) }, (t, done) => {
   t.test('ok 1', async () => {});
   t.test('ok 2', () => {});
   t.test('ok 3', { signal: t.signal }, async () => {});
@@ -40,7 +40,7 @@ test('callback abort signal', { signal: AbortSignal.abort() }, (t, done) => {
   t.test('should not appear', done);
 });
 
-// AbortSignal.timeout(1) doesn't prevent process from closing
+// AbortSignal.timeout doesn't prevent process from closing
 // thus we have to keep the process open to prevent cancelation
 // of the entire test tree
 setTimeout(() => {}, 1000);
diff --git a/test/fixtures/test-runner/output/abort.snapshot b/test/fixtures/test-runner/output/abort.snapshot
@@ -28,6 +28,8 @@ TAP version 13
       failureType: 'cancelledByParent'
       error: 'test did not finish before its parent and was cancelled'
       code: 'ERR_TEST_FAILURE'
+      stack: |-
+        async Promise.all (index 0)
       ...
     # Subtest: not ok 2
     not ok 6 - not ok 2
@@ -37,6 +39,8 @@ TAP version 13
       failureType: 'cancelledByParent'
       error: 'test did not finish before its parent and was cancelled'
       code: 'ERR_TEST_FAILURE'
+      stack: |-
+        async Promise.all (index 0)
       ...
     # Subtest: not ok 3
     not ok 7 - not ok 3
@@ -165,6 +169,8 @@ not ok 2 - promise abort signal
       failureType: 'cancelledByParent'
       error: 'test did not finish before its parent and was cancelled'
       code: 'ERR_TEST_FAILURE'
+      stack: |-
+        async Promise.all (index 2)
       ...
     # Subtest: not ok 2
     not ok 6 - not ok 2
@@ -174,6 +180,8 @@ not ok 2 - promise abort signal
       failureType: 'cancelledByParent'
       error: 'test did not finish before its parent and was cancelled'
       code: 'ERR_TEST_FAILURE'
+      stack: |-
+        async Promise.all (index 2)
       ...
     # Subtest: not ok 3
     not ok 7 - not ok 3

diff --git a/test/fixtures/test-runner/output/abort_hooks.snapshot b/test/fixtures/test-runner/output/abort_hooks.snapshot
@@ -1,3 +1,4 @@
+TAP version 13
 before
 2.1
 2.2
@@ -6,7 +7,6 @@ beforeEach
 4.1
 afterEach
 4.2
-TAP version 13
 # Subtest: 1 before describe
     # Subtest: test 1
     not ok 1 - test 1
@@ -16,6 +16,8 @@ TAP version 13
       failureType: 'cancelledByParent'
       error: 'test did not finish before its parent and was cancelled'
       code: 'ERR_TEST_FAILURE'
+      stack: |-
+        async Promise.all (index 0)
       ...
     # Subtest: test 2
     not ok 2 - test 2
@@ -25,6 +27,8 @@ TAP version 13
       failureType: 'cancelledByParent'
       error: 'test did not finish before its parent and was cancelled'
       code: 'ERR_TEST_FAILURE'
+      stack: |-
+        async Promise.all (index 0)
       ...
     1..2
 not ok 1 - 1 before describe
@@ -133,6 +137,8 @@ not ok 3 - 3 beforeEach describe
   failureType: 'subtestsFailed'
   error: '2 subtests failed'
   code: 'ERR_TEST_FAILURE'
+  stack: |-
+    async Promise.all (index 2)
   ...
 # Subtest: 4 afterEach describe
     # Subtest: test 1
@@ -186,6 +192,8 @@ not ok 4 - 4 afterEach describe
   failureType: 'subtestsFailed'
   error: '2 subtests failed'
   code: 'ERR_TEST_FAILURE'
+  stack: |-
+    async Promise.all (index 3)
   ...
 1..4
 # tests 8

diff --git a/test/fixtures/test-runner/output/abort_suite.js b/test/fixtures/test-runner/output/abort_suite.js
@@ -2,7 +2,7 @@
 require('../../../common');
 const { describe, it } = require('node:test');
 
-describe('describe timeout signal', { signal: AbortSignal.timeout(1) }, (t) => {
+describe('describe timeout signal', { signal: AbortSignal.timeout(100) }, (t) => {
   it('ok 1', async () => {});
   it('ok 2', () => {});
   it('ok 3', { signal: t.signal }, async () => {});
@@ -20,7 +20,7 @@ describe('describe abort signal', { signal: AbortSignal.abort() }, () => {
   it('should not appear', () => {});
 });
 
-// AbortSignal.timeout(1) doesn't prevent process from closing
+// AbortSignal.timeout doesn't prevent process from closing
 // thus we have to keep the process open to prevent cancelation
 // of the entire test tree
 setTimeout(() => {}, 1000);
diff --git a/test/fixtures/test-runner/output/abort_suite.snapshot b/test/fixtures/test-runner/output/abort_suite.snapshot
@@ -28,6 +28,8 @@ TAP version 13
       failureType: 'cancelledByParent'
       error: 'test did not finish before its parent and was cancelled'
       code: 'ERR_TEST_FAILURE'
+      stack: |-
+        async Promise.all (index 0)
       ...
     # Subtest: not ok 2
     not ok 6 - not ok 2
@@ -37,6 +39,8 @@ TAP version 13
       failureType: 'cancelledByParent'
       error: 'test did not finish before its parent and was cancelled'
       code: 'ERR_TEST_FAILURE'
+      stack: |-
+        async Promise.all (index 0)
       ...
     # Subtest: not ok 3
     not ok 7 - not ok 3

diff --git a/test/fixtures/test-runner/output/default_output.snapshot b/test/fixtures/test-runner/output/default_output.snapshot
@@ -8,6 +8,7 @@
   *[39m
   *[39m
   *[39m
+      at async Promise.all (index 0)
   *[39m
 
 [90m﹣ should skip [90m(*ms)[39m # SKIP[39m
@@ -47,6 +48,7 @@
   *[39m
   *[39m
   *[39m
+      at async Promise.all (index 0)
   *[39m
 
 *
-Original file line number
+Diff line change
@@ Expand Up / @@ -8,6 +8,7 @@ @@
       *[39m
       *[39m
       *[39m
+          at async Promise.all (index 0)
       *[39m
     [90m﹣ should skip [90m(*ms)[39m # SKIP[39m
@@ Expand Down Expand Up / @@ -47,6 +48,7 @@ @@
       *[39m
       *[39m
       *[39m
+          at async Promise.all (index 0)
       *[39m
     *
@@ Expand Down @@