From 8d6cd539747345cb9643697298e2ceb96e75c004 Mon Sep 17 00:00:00 2001 From: cjihrig Date: Wed, 24 Jul 2024 00:04:13 -0400 Subject: [PATCH 1/4] test_runner: remove redundant bootstrap boolean The test runner bootstrap process awaits a Promise and then sets a boolean flag. This commit consolidates the Promise and boolean into a single value. This commit also ensures that the globalRoot test is always assigned in createTestTree() in order to better consolidate the CLI/run() and non-CLI configuration. --- lib/internal/test_runner/harness.js | 39 ++++++++++++++--------------- lib/internal/test_runner/runner.js | 1 - 2 files changed, 19 insertions(+), 21 deletions(-) diff --git a/lib/internal/test_runner/harness.js b/lib/internal/test_runner/harness.js index e595f2ed1dff61..ac52307cc38be5 100644 --- a/lib/internal/test_runner/harness.js +++ b/lib/internal/test_runner/harness.js @@ -28,18 +28,20 @@ const { } = require('internal/test_runner/utils'); const { queueMicrotask } = require('internal/process/task_queues'); const { bigint: hrtime } = process.hrtime; - +const resolvedPromise = PromiseResolve(); const testResources = new SafeMap(); +let globalRoot; testResources.set(reporterScope.asyncId(), reporterScope); function createTestTree(options = kEmptyObject) { - return setup(new Test({ __proto__: null, ...options, name: '' })); + globalRoot = setup(new Test({ __proto__: null, ...options, name: '' })); + return globalRoot; } function createProcessEventHandler(eventName, rootTest) { return (err) => { - if (!rootTest.harness.bootstrapComplete) { + if (rootTest.harness.bootstrapPromise) { // Something went wrong during the asynchronous portion of bootstrapping // the test runner. Since the test runner is not setup properly, we can't // do anything but throw the error. @@ -196,7 +198,7 @@ function setup(root) { root.harness = { __proto__: null, allowTestsToRun: false, - bootstrapComplete: false, + bootstrapPromise: resolvedPromise, watching: false, coverage: FunctionPrototypeBind(collectCoverage, null, root, coverage), resetCounters() { @@ -222,33 +224,30 @@ function setup(root) { return root; } -let globalRoot; -let asyncBootstrap; function lazyBootstrapRoot() { if (!globalRoot) { - globalRoot = createTestTree({ __proto__: null, entryFile: process.argv?.[1] }); + // This is where the test runner is bootstrapped when node:test is used + // without the --test flag or the run() API. + createTestTree({ __proto__: null, entryFile: process.argv?.[1] }); globalRoot.reporter.on('test:fail', (data) => { if (data.todo === undefined || data.todo === false) { process.exitCode = kGenericUserError; } }); - asyncBootstrap = setupTestReporters(globalRoot.reporter); + globalRoot.harness.bootstrapPromise = setupTestReporters(globalRoot.reporter); } return globalRoot; } -async function startSubtest(subtest) { - if (asyncBootstrap) { +async function startSubtestAfterBootstrap(subtest) { + if (subtest.root.harness.bootstrapPromise) { // Only incur the overhead of awaiting the Promise once. - await asyncBootstrap; - asyncBootstrap = undefined; - if (!subtest.root.harness.bootstrapComplete) { - subtest.root.harness.bootstrapComplete = true; - queueMicrotask(() => { - subtest.root.harness.allowTestsToRun = true; - subtest.root.processPendingSubtests(); - }); - } + await subtest.root.harness.bootstrapPromise; + subtest.root.harness.bootstrapPromise = null; + queueMicrotask(() => { + subtest.root.harness.allowTestsToRun = true; + subtest.root.processPendingSubtests(); + }); } await subtest.start(); @@ -262,7 +261,7 @@ function runInParentContext(Factory) { return PromiseResolve(); } - return startSubtest(subtest); + return startSubtestAfterBootstrap(subtest); } const test = (name, options, fn) => { diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js index 31a253c776a81e..4c46e910800535 100644 --- a/lib/internal/test_runner/runner.js +++ b/lib/internal/test_runner/runner.js @@ -596,7 +596,6 @@ function run(options = kEmptyObject) { teardown = undefined; } const runFiles = () => { - root.harness.bootstrapComplete = true; root.harness.allowTestsToRun = true; return SafePromiseAllSettledReturnVoid(testFiles, (path) => { const subtest = runTestFile(path, filesWatcher, opts); From 2d775dbd0b4f1c91797938907ab62a417e4bac29 Mon Sep 17 00:00:00 2001 From: cjihrig Date: Wed, 24 Jul 2024 10:23:19 -0400 Subject: [PATCH 2/4] hmm --- lib/internal/test_runner/runner.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js index 4c46e910800535..7443502848f930 100644 --- a/lib/internal/test_runner/runner.js +++ b/lib/internal/test_runner/runner.js @@ -596,6 +596,7 @@ function run(options = kEmptyObject) { teardown = undefined; } const runFiles = () => { + root.harness.bootstrapPromise = null; root.harness.allowTestsToRun = true; return SafePromiseAllSettledReturnVoid(testFiles, (path) => { const subtest = runTestFile(path, filesWatcher, opts); From 383ec7f5688f3f5118f2c41b7fe9c0768a839089 Mon Sep 17 00:00:00 2001 From: cjihrig Date: Thu, 25 Jul 2024 10:03:44 -0400 Subject: [PATCH 3/4] ugh --- test/parallel/test-runner-run-watch.mjs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/test/parallel/test-runner-run-watch.mjs b/test/parallel/test-runner-run-watch.mjs index 0445637bb6123c..faf0810ca9ad20 100644 --- a/test/parallel/test-runner-run-watch.mjs +++ b/test/parallel/test-runner-run-watch.mjs @@ -27,13 +27,19 @@ test('test has ran');`, let fixturePaths; -function refresh() { +async function refresh() { + const { rm, readdir } = await import('node:fs/promises'); + console.log('refreshing', tmpdir.path); + await rm(tmpdir.path, { maxRetries: 3, recursive: true, force: true }); + tmpdir.refresh(); fixturePaths = Object.keys(fixtureContent) .reduce((acc, file) => ({ ...acc, [file]: tmpdir.resolve(file) }), {}); Object.entries(fixtureContent) .forEach(([file, content]) => writeFileSync(fixturePaths[file], content)); + + console.log('contents of %s:', tmpdir.path, await readdir(tmpdir.path)); } const runner = join(import.meta.dirname, '..', 'fixtures', 'test-runner-watch.mjs'); From 7ac0989161fd5fda60eb6e5bcdaaf70cfbac7fec Mon Sep 17 00:00:00 2001 From: cjihrig Date: Thu, 25 Jul 2024 12:36:23 -0400 Subject: [PATCH 4/4] Revert "ugh" This reverts commit 383ec7f5688f3f5118f2c41b7fe9c0768a839089. --- test/parallel/test-runner-run-watch.mjs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/test/parallel/test-runner-run-watch.mjs b/test/parallel/test-runner-run-watch.mjs index faf0810ca9ad20..0445637bb6123c 100644 --- a/test/parallel/test-runner-run-watch.mjs +++ b/test/parallel/test-runner-run-watch.mjs @@ -27,19 +27,13 @@ test('test has ran');`, let fixturePaths; -async function refresh() { - const { rm, readdir } = await import('node:fs/promises'); - console.log('refreshing', tmpdir.path); - await rm(tmpdir.path, { maxRetries: 3, recursive: true, force: true }); - +function refresh() { tmpdir.refresh(); fixturePaths = Object.keys(fixtureContent) .reduce((acc, file) => ({ ...acc, [file]: tmpdir.resolve(file) }), {}); Object.entries(fixtureContent) .forEach(([file, content]) => writeFileSync(fixturePaths[file], content)); - - console.log('contents of %s:', tmpdir.path, await readdir(tmpdir.path)); } const runner = join(import.meta.dirname, '..', 'fixtures', 'test-runner-watch.mjs');