diff --git a/lib/internal/test_runner/harness.js b/lib/internal/test_runner/harness.js index 9c372c115e90f2..3c56820cf4e247 100644 --- a/lib/internal/test_runner/harness.js +++ b/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(); diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js index e994b1aa40ecab..a4874d5caead91 100644 --- a/lib/internal/test_runner/runner.js +++ b/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); diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index e4ebbb2ee9238b..b79ff7a049ea6c 100644 --- a/lib/internal/test_runner/test.js +++ b/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; diff --git a/test/fixtures/test-runner/output/filtered-suite-delayed-build.js b/test/fixtures/test-runner/output/filtered-suite-delayed-build.js new file mode 100644 index 00000000000000..c6b7060c2b88b2 --- /dev/null +++ b/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()); +}); diff --git a/test/fixtures/test-runner/output/filtered-suite-delayed-build.snapshot b/test/fixtures/test-runner/output/filtered-suite-delayed-build.snapshot new file mode 100644 index 00000000000000..dbe3048dffdf12 --- /dev/null +++ b/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 * diff --git a/test/fixtures/test-runner/output/filtered-suite-order.mjs b/test/fixtures/test-runner/output/filtered-suite-order.mjs new file mode 100644 index 00000000000000..f7df0cb8e355a7 --- /dev/null +++ b/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); diff --git a/test/fixtures/test-runner/output/filtered-suite-order.snapshot b/test/fixtures/test-runner/output/filtered-suite-order.snapshot new file mode 100644 index 00000000000000..7a18df8c7d0aea --- /dev/null +++ b/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 * diff --git a/test/fixtures/test-runner/output/source_mapped_locations.snapshot b/test/fixtures/test-runner/output/source_mapped_locations.snapshot index 29b70fd0d08378..24c3ee8d113446 100644 --- a/test/fixtures/test-runner/output/source_mapped_locations.snapshot +++ b/test/fixtures/test-runner/output/source_mapped_locations.snapshot @@ -21,9 +21,6 @@ not ok 1 - fails * * * - * - * - * ... 1..1 # tests 1 diff --git a/test/parallel/test-runner-output.mjs b/test/parallel/test-runner-output.mjs index bd2db22ee6cc36..0125a8168e4464 100644 --- a/test/parallel/test-runner-output.mjs +++ b/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 },