test_runner: finish build phase before running tests

lib/internal/test_runner/harness.js

@@ -1,9 +1,11 @@
 'use strict';
 const {
   ArrayPrototypeForEach,
+  ArrayPrototypePush,
   FunctionPrototypeBind,
   PromiseResolve,
   SafeMap,
+  SafePromiseAllReturnVoid,
 } = primordials;
 const { getCallerLocation } = internalBinding('util');
 const {
@@ -24,6 +26,7 @@ const {
   shouldColorizeTestFiles,
 } = require('internal/test_runner/utils');
 const { queueMicrotask } = require('internal/process/task_queues');
+const { createDeferredPromise } = require('internal/util');
 const { bigint: hrtime } = process.hrtime;
 const resolvedPromise = PromiseResolve();
 const testResources = new SafeMap();
@@ -32,9 +35,12 @@ let globalRoot;
 testResources.set(reporterScope.asyncId(), reporterScope);
 
 function createTestTree(rootTestOptions, globalOptions) {
+  const buildPhaseDeferred = createDeferredPromise();
   const harness = {
     __proto__: null,
-    allowTestsToRun: false,
+    buildPromise: buildPhaseDeferred.promise,
+    buildSuites: [],
+    isWaitingForBuildPhase: false,
     bootstrapPromise: resolvedPromise,
     watching: false,
     config: globalOptions,
@@ -56,6 +62,13 @@ function createTestTree(rootTestOptions, globalOptions) {
     shouldColorizeTestFiles: shouldColorizeTestFiles(globalOptions.destinations),
     teardown: null,
     snapshotManager: null,
+    async waitForBuildPhase() {
+      if (harness.buildSuites.length > 0) {
+        await SafePromiseAllReturnVoid(harness.buildSuites);
+      }
+
+      buildPhaseDeferred.resolve();
+    },
   };
 
   harness.resetCounters();
@@ -243,14 +256,25 @@ function lazyBootstrapRoot() {
 }
 
 async function startSubtestAfterBootstrap(subtest) {
-  if (subtest.root.harness.bootstrapPromise) {
-    // Only incur the overhead of awaiting the Promise once.
-    await subtest.root.harness.bootstrapPromise;
-    subtest.root.harness.bootstrapPromise = null;
-    queueMicrotask(() => {
-      subtest.root.harness.allowTestsToRun = true;
-      subtest.root.processPendingSubtests();
-    });
+  if (subtest.root.harness.buildPromise) {
+    if (subtest.root.harness.bootstrapPromise) {
+      await subtest.root.harness.bootstrapPromise;
+      subtest.root.harness.bootstrapPromise = null;
+    }
+
+    if (subtest.buildSuite) {
+      ArrayPrototypePush(subtest.root.harness.buildSuites, subtest.buildSuite);
+    }
+
+    if (!subtest.root.harness.isWaitingForBuildPhase) {
+      subtest.root.harness.isWaitingForBuildPhase = true;
+      queueMicrotask(() => {
+        subtest.root.harness.waitForBuildPhase();
+      });
+    }
+
+    await subtest.root.harness.buildPromise;
+    subtest.root.harness.buildPromise = null;
   }
 
   await subtest.start();

lib/internal/test_runner/runner.js

@@ -610,7 +610,7 @@ function run(options = kEmptyObject) {
   }
   const runFiles = () => {
     root.harness.bootstrapPromise = null;
-    root.harness.allowTestsToRun = true;
+    root.harness.buildPromise = null;
     return SafePromiseAllSettledReturnVoid(testFiles, (path) => {
       const subtest = runTestFile(path, filesWatcher, opts);
       filesWatcher?.runningSubtests.set(path, subtest);

lib/internal/test_runner/test.js

@@ -766,7 +766,7 @@ class Test extends AsyncResource {
     // it. Otherwise, return a Promise to the caller and mark the test as
     // pending for later execution.
     this.reporter.enqueue(this.nesting, this.loc, this.name);
-    if (!this.root.harness.allowTestsToRun || !this.parent.hasConcurrency()) {
+    if (this.root.harness.buildPromise || !this.parent.hasConcurrency()) {
       const deferred = createDeferredPromise();
 
       deferred.test = this;

test/fixtures/test-runner/output/filtered-suite-delayed-build.js

@@ -0,0 +1,16 @@
+// Flags: --test-name-pattern=enabled
+'use strict';
+const common = require('../../../common');
+const { suite, test } = require('node:test');
+
+suite('async suite', async () => {
+  await 1;
+  test('enabled 1', common.mustCall());
+  await 1;
+  test('not run', common.mustNotCall());
+  await 1;
+});
+
+suite('sync suite', () => {
+  test('enabled 2', common.mustCall());
+});

test/fixtures/test-runner/output/filtered-suite-delayed-build.snapshot

@@ -0,0 +1,34 @@
+TAP version 13
+# Subtest: async suite
+    # Subtest: enabled 1
+    ok 1 - enabled 1
+      ---
+      duration_ms: *
+      ...
+    1..1
+ok 1 - async suite
+  ---
+  duration_ms: *
+  type: 'suite'
+  ...
+# Subtest: sync suite
+    # Subtest: enabled 2
+    ok 1 - enabled 2
+      ---
+      duration_ms: *
+      ...
+    1..1
+ok 2 - sync suite
+  ---
+  duration_ms: *
+  type: 'suite'
+  ...
+1..2
+# tests 2
+# suites 2
+# pass 2
+# fail 0
+# cancelled 0
+# skipped 0
+# todo 0
+# duration_ms *

test/fixtures/test-runner/output/filtered-suite-order.mjs

@@ -0,0 +1,49 @@
+// Flags: --test-only
+import { describe, test, after } from 'node:test';
+
+after(() => { console.log('with global after()'); });
+await Promise.resolve();
+
+console.log('Execution order was:');
+const ll = (t) => { console.log(` * ${t.fullName}`) };
+
+describe('A', () => {
+  test.only('A', ll);
+  test('B', ll);
+  describe.only('C', () => {
+    test.only('A', ll);
+    test('B', ll);
+  });
+  describe('D', () => {
+    test.only('A', ll);
+    test('B', ll);
+  });
+});
+describe.only('B', () => {
+  test('A', ll);
+  test('B', ll);
+  describe('C', () => {
+    test('A', ll);
+  });
+});
+describe('C', () => {
+  test.only('A', ll);
+  test('B', ll);
+  describe.only('C', () => {
+    test('A', ll);
+    test('B', ll);
+  });
+  describe('D', () => {
+    test('A', ll);
+    test.only('B', ll);
+  });
+});
+describe('D', () => {
+  test('A', ll);
+  test.only('B', ll);
+});
+describe.only('E', () => {
+  test('A', ll);
+  test('B', ll);
+});
+test.only('F', ll);

test/fixtures/test-runner/output/filtered-suite-order.snapshot

@@ -0,0 +1,166 @@
+Execution order was:
+ * A > A
+ * A > C > A
+ * A > D > A
+ * B > A
+ * B > B
+ * B > C > A
+ * C > A
+ * C > C > A
+ * C > C > B
+ * C > D > B
+ * D > B
+ * E > A
+ * E > B
+ * F
+with global after()
+TAP version 13
+# Subtest: A
+    # Subtest: A
+    ok 1 - A
+      ---
+      duration_ms: *
+      ...
+    # Subtest: C
+        # Subtest: A
+        ok 1 - A
+          ---
+          duration_ms: *
+          ...
+        1..1
+    ok 2 - C
+      ---
+      duration_ms: *
+      type: 'suite'
+      ...
+    # Subtest: D
+        # Subtest: A
+        ok 1 - A
+          ---
+          duration_ms: *
+          ...
+        1..1
+    ok 3 - D
+      ---
+      duration_ms: *
+      type: 'suite'
+      ...
+    1..3
+ok 1 - A
+  ---
+  duration_ms: *
+  type: 'suite'
+  ...
+# Subtest: B
+    # Subtest: A
+    ok 1 - A
+      ---
+      duration_ms: *
+      ...
+    # Subtest: B
+    ok 2 - B
+      ---
+      duration_ms: *
+      ...
+    # Subtest: C
+        # Subtest: A
+        ok 1 - A
+          ---
+          duration_ms: *
+          ...
+        1..1
+    ok 3 - C
+      ---
+      duration_ms: *
+      type: 'suite'
+      ...
+    1..3
+ok 2 - B
+  ---
+  duration_ms: *
+  type: 'suite'
+  ...
+# Subtest: C
+    # Subtest: A
+    ok 1 - A
+      ---
+      duration_ms: *
+      ...
+    # Subtest: C
+        # Subtest: A
+        ok 1 - A
+          ---
+          duration_ms: *
+          ...
+        # Subtest: B
+        ok 2 - B
+          ---
+          duration_ms: *
+          ...
+        1..2
+    ok 2 - C
+      ---
+      duration_ms: *
+      type: 'suite'
+      ...
+    # Subtest: D
+        # Subtest: B
+        ok 1 - B
+          ---
+          duration_ms: *
+          ...
+        1..1
+    ok 3 - D
+      ---
+      duration_ms: *
+      type: 'suite'
+      ...
+    1..3
+ok 3 - C
+  ---
+  duration_ms: *
+  type: 'suite'
+  ...
+# Subtest: D
+    # Subtest: B
+    ok 1 - B
+      ---
+      duration_ms: *
+      ...
+    1..1
+ok 4 - D
+  ---
+  duration_ms: *
+  type: 'suite'
+  ...
+# Subtest: E
+    # Subtest: A
+    ok 1 - A
+      ---
+      duration_ms: *
+      ...
+    # Subtest: B
+    ok 2 - B
+      ---
+      duration_ms: *
+      ...
+    1..2
+ok 5 - E
+  ---
+  duration_ms: *
+  type: 'suite'
+  ...
+# Subtest: F
+ok 6 - F
+  ---
+  duration_ms: *
+  ...
+1..6
+# tests 14
+# suites 10
+# pass 14
+# fail 0
+# cancelled 0
+# skipped 0
+# todo 0
+# duration_ms *

test/fixtures/test-runner/output/source_mapped_locations.snapshot

@@ -21,9 +21,6 @@ not ok 1 - fails
     *
     *
     *
-    *
-    *
-    *
   ...
 1..1
 # tests 1

test/parallel/test-runner-output.mjs

@@ -101,6 +101,8 @@ const tests = [
   { name: 'test-runner/output/eval_dot.js', transform: specTransform },
   { name: 'test-runner/output/eval_spec.js', transform: specTransform },
   { name: 'test-runner/output/eval_tap.js' },
+  { name: 'test-runner/output/filtered-suite-delayed-build.js' },
+  { name: 'test-runner/output/filtered-suite-order.mjs' },
   { name: 'test-runner/output/filtered-suite-throws.js' },
   { name: 'test-runner/output/hooks.js' },
   { name: 'test-runner/output/hooks_spec_reporter.js', transform: specTransform },