From 4fd66354d093662be45684d4f8ee40d0e904619a Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Fri, 15 Mar 2024 15:35:45 +0200 Subject: [PATCH 1/3] test_runner: skip `--require` for test orchestration process --- lib/internal/process/pre_execution.js | 3 ++- test/fixtures/test-runner/print_pid.js | 3 +++ test/parallel/test-runner-cli.js | 24 ++++++++++++++++++++++++ 3 files changed, 29 insertions(+), 1 deletion(-) create mode 100644 test/fixtures/test-runner/print_pid.js diff --git a/lib/internal/process/pre_execution.js b/lib/internal/process/pre_execution.js index c4e39798f29dcc..cda8b5b34799e3 100644 --- a/lib/internal/process/pre_execution.js +++ b/lib/internal/process/pre_execution.js @@ -735,7 +735,8 @@ function runEmbedderPreload() { function loadPreloadModules() { // For user code, we preload modules if `-r` is passed const preloadModules = getOptionValue('--require'); - if (preloadModules && preloadModules.length > 0) { + const isOrcastrationProcess = getOptionValue('--test'); + if (preloadModules && preloadModules.length > 0 && !isOrcastrationProcess) { const { Module: { _preloadModules, diff --git a/test/fixtures/test-runner/print_pid.js b/test/fixtures/test-runner/print_pid.js new file mode 100644 index 00000000000000..a7c928959e38ee --- /dev/null +++ b/test/fixtures/test-runner/print_pid.js @@ -0,0 +1,3 @@ +'use strict'; + +console.log({ pid: process.pid }); diff --git a/test/parallel/test-runner-cli.js b/test/parallel/test-runner-cli.js index ab6078a4a05d74..b7c40845d80906 100644 --- a/test/parallel/test-runner-cli.js +++ b/test/parallel/test-runner-cli.js @@ -334,3 +334,27 @@ const testFixtures = fixtures.path('test-runner'); assert.match(stdout, /# fail 0/); assert.match(stdout, /# skipped 0/); } + +{ + // --require should only be applied to individual test processes, not the orchestrator + const args = ['--test', '--require', join(testFixtures, 'print_pid.js'), join(testFixtures, 'index.js')]; + const child = spawnSync(process.execPath, args, { cwd: testFixtures }); + + assert.strictEqual(child.status, 1); + assert.strictEqual(child.signal, null); + assert.strictEqual(child.stderr.toString(), ''); + assert.match(child.stdout.toString(), /pid: \d+/); + assert.doesNotMatch(child.stdout.toString(), new RegExp(`pid: ${child.pid}`)); +} + +{ + // --import should only be applied to individual test processes, not the orchestrator + const args = ['--test', '--require', join(testFixtures, 'print_pid.js'), join(testFixtures, 'index.js')]; + const child = spawnSync(process.execPath, args, { cwd: testFixtures }); + + assert.strictEqual(child.status, 1); + assert.strictEqual(child.signal, null); + assert.strictEqual(child.stderr.toString(), ''); + assert.match(child.stdout.toString(), /pid: \d+/); + assert.doesNotMatch(child.stdout.toString(), new RegExp(`pid: ${child.pid}`)); +} From ff52556b89b74776c3e44b5b944040aabce7fcb4 Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Mon, 1 Apr 2024 10:58:36 +0300 Subject: [PATCH 2/3] CR --- lib/internal/main/test_runner.js | 2 +- lib/internal/process/pre_execution.js | 15 ++++++++------- test/parallel/test-runner-cli.js | 6 ++++-- 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/lib/internal/main/test_runner.js b/lib/internal/main/test_runner.js index 4f3f067cc74242..3ba9fcea511423 100644 --- a/lib/internal/main/test_runner.js +++ b/lib/internal/main/test_runner.js @@ -22,7 +22,7 @@ let debug = require('internal/util/debuglog').debuglog('test_runner', (fn) => { debug = fn; }); -prepareMainThreadExecution(false); +prepareMainThreadExecution(false, true, false); markBootstrapComplete(); let concurrency = getOptionValue('--test-concurrency') || true; diff --git a/lib/internal/process/pre_execution.js b/lib/internal/process/pre_execution.js index cda8b5b34799e3..3133da9fa46ce8 100644 --- a/lib/internal/process/pre_execution.js +++ b/lib/internal/process/pre_execution.js @@ -50,10 +50,11 @@ const { }, } = require('internal/v8/startup_snapshot'); -function prepareMainThreadExecution(expandArgv1 = false, initializeModules = true) { +function prepareMainThreadExecution(expandArgv1 = false, initializeModules = true, preloadModules = initializeModules) { return prepareExecution({ expandArgv1, initializeModules, + preloadModules, isMainThread: true, }); } @@ -62,6 +63,7 @@ function prepareWorkerThreadExecution() { prepareExecution({ expandArgv1: false, initializeModules: false, // Will need to initialize it after policy setup + preloadModules: false, isMainThread: false, }); } @@ -95,7 +97,7 @@ function prepareShadowRealmExecution() { } function prepareExecution(options) { - const { expandArgv1, initializeModules, isMainThread } = options; + const { expandArgv1, initializeModules, isMainThread, preloadModules } = options; refreshRuntimeOptions(); reconnectZeroFillToggle(); @@ -157,7 +159,7 @@ function prepareExecution(options) { } if (initializeModules) { - setupUserModules(); + setupUserModules(false, preloadModules); } return mainEntry; @@ -188,7 +190,7 @@ function setupSymbolDisposePolyfill() { } } -function setupUserModules(forceDefaultLoader = false) { +function setupUserModules(forceDefaultLoader = false, preloadModules = true) { initializeCJSLoader(); initializeESMLoader(forceDefaultLoader); const { @@ -203,7 +205,7 @@ function setupUserModules(forceDefaultLoader = false) { // Do not enable preload modules if custom loaders are disabled. // For example, loader workers are responsible for doing this themselves. // And preload modules are not supported in ShadowRealm as well. - if (!forceDefaultLoader) { + if (!forceDefaultLoader && preloadModules) { loadPreloadModules(); } // Need to be done after --require setup. @@ -735,8 +737,7 @@ function runEmbedderPreload() { function loadPreloadModules() { // For user code, we preload modules if `-r` is passed const preloadModules = getOptionValue('--require'); - const isOrcastrationProcess = getOptionValue('--test'); - if (preloadModules && preloadModules.length > 0 && !isOrcastrationProcess) { + if (preloadModules && preloadModules.length > 0) { const { Module: { _preloadModules, diff --git a/test/parallel/test-runner-cli.js b/test/parallel/test-runner-cli.js index b7c40845d80906..7d64671bf28267 100644 --- a/test/parallel/test-runner-cli.js +++ b/test/parallel/test-runner-cli.js @@ -337,7 +337,8 @@ const testFixtures = fixtures.path('test-runner'); { // --require should only be applied to individual test processes, not the orchestrator - const args = ['--test', '--require', join(testFixtures, 'print_pid.js'), join(testFixtures, 'index.js')]; + const args = ['--test', '--require', join(testFixtures, 'print_pid.js'), + join(testFixtures, 'index.js'), join(testFixtures, 'default-behavior', 'index.test.js')]; const child = spawnSync(process.execPath, args, { cwd: testFixtures }); assert.strictEqual(child.status, 1); @@ -349,7 +350,8 @@ const testFixtures = fixtures.path('test-runner'); { // --import should only be applied to individual test processes, not the orchestrator - const args = ['--test', '--require', join(testFixtures, 'print_pid.js'), join(testFixtures, 'index.js')]; + const args = ['--test', '--import', fixtures.fileURL('test-runner/print_pid.js'), + join(testFixtures, 'index.js'), join(testFixtures, 'default-behavior', 'index.test.js')]; const child = spawnSync(process.execPath, args, { cwd: testFixtures }); assert.strictEqual(child.status, 1); From 42049a2ce5a452878508293eb0027ebcd7a7e15d Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Mon, 1 Apr 2024 22:01:41 +0300 Subject: [PATCH 3/3] fixup! CR --- .../test-runner/output/arbitrary-output-colored.js | 7 +++---- test/fixtures/test-runner/output/reset-color-depth.js | 5 ----- 2 files changed, 3 insertions(+), 9 deletions(-) delete mode 100644 test/fixtures/test-runner/output/reset-color-depth.js diff --git a/test/fixtures/test-runner/output/arbitrary-output-colored.js b/test/fixtures/test-runner/output/arbitrary-output-colored.js index af23e674cb361e..f3f4e00e06d9c5 100644 --- a/test/fixtures/test-runner/output/arbitrary-output-colored.js +++ b/test/fixtures/test-runner/output/arbitrary-output-colored.js @@ -5,8 +5,7 @@ const { spawn } = require('node:child_process'); const fixtures = require('../../../common/fixtures'); (async function run() { - const test = fixtures.path('test-runner/output/arbitrary-output-colored-1.js'); - const reset = fixtures.path('test-runner/output/reset-color-depth.js'); - await once(spawn(process.execPath, ['-r', reset, '--test', test], { stdio: 'inherit' }), 'exit'); - await once(spawn(process.execPath, ['-r', reset, '--test', '--test-reporter', 'tap', test], { stdio: 'inherit' }), 'exit'); + const test = fixtures.path('test-runner/output/arbitrary-output-colored-1.js'); + await once(spawn(process.execPath, ['--test', test], { stdio: 'inherit', env: { FORCE_COLOR: 1 } }), 'exit'); + await once(spawn(process.execPath, ['--test', '--test-reporter', 'tap', test], { stdio: 'inherit', env: { FORCE_COLOR: 1 } }), 'exit'); })().then(common.mustCall()); diff --git a/test/fixtures/test-runner/output/reset-color-depth.js b/test/fixtures/test-runner/output/reset-color-depth.js deleted file mode 100644 index 02c04b247ad4d9..00000000000000 --- a/test/fixtures/test-runner/output/reset-color-depth.js +++ /dev/null @@ -1,5 +0,0 @@ -'use strict'; - -// Make tests OS-independent by overriding stdio getColorDepth(). -process.stdout.getColorDepth = () => 8; -process.stderr.getColorDepth = () => 8;