diff --git a/doc/api/test.md b/doc/api/test.md index 67af2b8fc8d3a9..ec9ac4da09900b 100644 --- a/doc/api/test.md +++ b/doc/api/test.md @@ -520,6 +520,24 @@ each other in ways that are not possible when isolation is enabled. For example, if a test relies on global state, it is possible for that state to be modified by a test originating from another file. +#### Child process option inheritance + +When running tests in process isolation mode (the default), spawned child processes +inherit Node.js options from the parent process, including those specified in +[configuration files][]. However, certain flags are filtered out to enable proper +test runner functionality: + +* `--test` - Prevented to avoid recursive test execution +* `--experimental-test-coverage` - Managed by the test runner +* `--watch` - Watch mode is handled at the parent level +* `--experimental-default-config-file` - Config file loading is handled by the parent +* `--test-reporter` - Reporting is managed by the parent process +* `--test-reporter-destination` - Output destinations are controlled by the parent +* `--experimental-config-file` - Config file paths are managed by the parent + +All other Node.js options from command line arguments, environment variables, +and configuration files are inherited by the child processes. + ## Collecting code coverage > Stability: 1 - Experimental @@ -3950,6 +3968,7 @@ Can be used to abort test subtasks when the test has been aborted. [`suite()`]: #suitename-options-fn [`test()`]: #testname-options-fn [code coverage]: #collecting-code-coverage +[configuration files]: cli.md#--experimental-config-fileconfig [describe options]: #describename-options-fn [it options]: #testname-options-fn [running tests from the command line]: #running-tests-from-the-command-line diff --git a/lib/internal/options.js b/lib/internal/options.js index fef0d61d143335..be1dc6a842c526 100644 --- a/lib/internal/options.js +++ b/lib/internal/options.js @@ -11,6 +11,7 @@ const { const { getCLIOptionsValues, getCLIOptionsInfo, + getOptionsAsFlags, getEmbedderOptions: getEmbedderOptionsFromBinding, getEnvOptionsInputType, getNamespaceOptionsInputType, @@ -21,6 +22,7 @@ let warnOnAllowUnauthorized = true; let optionsDict; let cliInfo; let embedderOptions; +let optionsFlags; // getCLIOptionsValues() would serialize the option values from C++ land. // It would error if the values are queried before bootstrap is @@ -34,6 +36,10 @@ function getCLIOptionsInfoFromBinding() { return cliInfo ??= getCLIOptionsInfo(); } +function getOptionsAsFlagsFromBinding() { + return optionsFlags ??= getOptionsAsFlags(); +} + function getEmbedderOptions() { return embedderOptions ??= getEmbedderOptionsFromBinding(); } @@ -156,6 +162,7 @@ function getAllowUnauthorized() { module.exports = { getCLIOptionsInfo: getCLIOptionsInfoFromBinding, getOptionValue, + getOptionsAsFlagsFromBinding, getAllowUnauthorized, getEmbedderOptions, generateConfigJsonSchema, diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js index 958ad1e060fbbe..d737909851ae6c 100644 --- a/lib/internal/test_runner/runner.js +++ b/lib/internal/test_runner/runner.js @@ -35,7 +35,7 @@ const { spawn } = require('child_process'); const { finished } = require('internal/streams/end-of-stream'); const { resolve, sep, isAbsolute } = require('path'); const { DefaultDeserializer, DefaultSerializer } = require('v8'); -const { getOptionValue } = require('internal/options'); +const { getOptionValue, getOptionsAsFlagsFromBinding } = require('internal/options'); const { Interface } = require('internal/readline/interface'); const { deserializeError } = require('internal/error_serdes'); const { Buffer } = require('buffer'); @@ -150,38 +150,39 @@ function getRunArgs(path, { forceExit, execArgv, root: { timeout }, cwd }) { - const argv = ArrayPrototypeFilter(process.execArgv, filterExecArgv); + const processNodeOptions = getOptionsAsFlagsFromBinding(); + const runArgs = ArrayPrototypeFilter(processNodeOptions, filterExecArgv); if (forceExit === true) { - ArrayPrototypePush(argv, '--test-force-exit'); + ArrayPrototypePush(runArgs, '--test-force-exit'); } if (isUsingInspector()) { - ArrayPrototypePush(argv, `--inspect-port=${getInspectPort(inspectPort)}`); + ArrayPrototypePush(runArgs, `--inspect-port=${getInspectPort(inspectPort)}`); } if (testNamePatterns != null) { - ArrayPrototypeForEach(testNamePatterns, (pattern) => ArrayPrototypePush(argv, `--test-name-pattern=${pattern}`)); + ArrayPrototypeForEach(testNamePatterns, (pattern) => ArrayPrototypePush(runArgs, `--test-name-pattern=${pattern}`)); } if (testSkipPatterns != null) { - ArrayPrototypeForEach(testSkipPatterns, (pattern) => ArrayPrototypePush(argv, `--test-skip-pattern=${pattern}`)); + ArrayPrototypeForEach(testSkipPatterns, (pattern) => ArrayPrototypePush(runArgs, `--test-skip-pattern=${pattern}`)); } if (only === true) { - ArrayPrototypePush(argv, '--test-only'); + ArrayPrototypePush(runArgs, '--test-only'); } if (timeout != null) { - ArrayPrototypePush(argv, `--test-timeout=${timeout}`); + ArrayPrototypePush(runArgs, `--test-timeout=${timeout}`); } - ArrayPrototypePushApply(argv, execArgv); + ArrayPrototypePushApply(runArgs, execArgv); if (path === kIsolatedProcessName) { - ArrayPrototypePush(argv, '--test'); - ArrayPrototypePushApply(argv, ArrayPrototypeSlice(process.argv, 1)); + ArrayPrototypePush(runArgs, '--test'); + ArrayPrototypePushApply(runArgs, ArrayPrototypeSlice(process.argv, 1)); } else { - ArrayPrototypePush(argv, path); + ArrayPrototypePush(runArgs, path); } - ArrayPrototypePushApply(argv, suppliedArgs); + ArrayPrototypePushApply(runArgs, suppliedArgs); - return argv; + return runArgs; } const serializer = new DefaultSerializer(); diff --git a/src/node_options.cc b/src/node_options.cc index dd68166b1656a5..d37ef9427dc018 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -1867,6 +1867,117 @@ void GetNamespaceOptionsInputType(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(namespaces_map); } +// Return an array containing all currently active options as flag +// strings from all sources (command line, NODE_OPTIONS, config file) +void GetOptionsAsFlags(const FunctionCallbackInfo& args) { + Isolate* isolate = args.GetIsolate(); + Local context = isolate->GetCurrentContext(); + Environment* env = Environment::GetCurrent(context); + + if (!env->has_run_bootstrapping_code()) { + // No code because this is an assertion. + THROW_ERR_OPTIONS_BEFORE_BOOTSTRAPPING( + isolate, "Should not query options before bootstrapping is done"); + } + env->set_has_serialized_options(true); + + Mutex::ScopedLock lock(per_process::cli_options_mutex); + IterateCLIOptionsScope s(env); + + std::vector flags; + PerProcessOptions* opts = per_process::cli_options.get(); + + for (const auto& item : _ppop_instance.options_) { + const std::string& option_name = item.first; + const auto& option_info = item.second; + auto field = option_info.field; + + // TODO(pmarchini): Skip internal options for the moment as probably not + // required + if (option_name.empty() || option_name.starts_with('[')) { + continue; + } + + // Skip V8 options and NoOp options - only Node.js-specific options + if (option_info.type == kNoOp || option_info.type == kV8Option) { + continue; + } + + switch (option_info.type) { + case kBoolean: { + bool current_value = *_ppop_instance.Lookup(field, opts); + // For boolean options with default_is_true, we want the opposite logic + if (option_info.default_is_true) { + if (!current_value) { + // If default is true and current is false, add --no-* flag + flags.push_back("--no-" + option_name.substr(2)); + } + } else { + if (current_value) { + // If default is false and current is true, add --flag + flags.push_back(option_name); + } + } + break; + } + case kInteger: { + int64_t current_value = *_ppop_instance.Lookup(field, opts); + flags.push_back(option_name + "=" + std::to_string(current_value)); + break; + } + case kUInteger: { + uint64_t current_value = *_ppop_instance.Lookup(field, opts); + flags.push_back(option_name + "=" + std::to_string(current_value)); + break; + } + case kString: { + const std::string& current_value = + *_ppop_instance.Lookup(field, opts); + // Only include if not empty + if (!current_value.empty()) { + flags.push_back(option_name + "=" + current_value); + } + break; + } + case kStringList: { + const std::vector& current_values = + *_ppop_instance.Lookup(field, opts); + // Add each string in the list as a separate flag + for (const std::string& value : current_values) { + flags.push_back(option_name + "=" + value); + } + break; + } + case kHostPort: { + const HostPort& host_port = + *_ppop_instance.Lookup(field, opts); + // Only include if host is not empty or port is not default + if (!host_port.host().empty() || host_port.port() != 0) { + std::string host_port_str = host_port.host(); + if (host_port.port() != 0) { + if (!host_port_str.empty()) { + host_port_str += ":"; + } + host_port_str += std::to_string(host_port.port()); + } + if (!host_port_str.empty()) { + flags.push_back(option_name + "=" + host_port_str); + } + } + break; + } + default: + // Skip unknown types + break; + } + } + + Local result; + CHECK(ToV8Value(context, flags).ToLocal(&result)); + + args.GetReturnValue().Set(result); +} + void Initialize(Local target, Local unused, Local context, @@ -1877,6 +1988,8 @@ void Initialize(Local target, context, target, "getCLIOptionsValues", GetCLIOptionsValues); SetMethodNoSideEffect( context, target, "getCLIOptionsInfo", GetCLIOptionsInfo); + SetMethodNoSideEffect( + context, target, "getOptionsAsFlags", GetOptionsAsFlags); SetMethodNoSideEffect( context, target, "getEmbedderOptions", GetEmbedderOptions); SetMethodNoSideEffect( @@ -1909,6 +2022,7 @@ void Initialize(Local target, void RegisterExternalReferences(ExternalReferenceRegistry* registry) { registry->Register(GetCLIOptionsValues); registry->Register(GetCLIOptionsInfo); + registry->Register(GetOptionsAsFlags); registry->Register(GetEmbedderOptions); registry->Register(GetEnvOptionsInputType); registry->Register(GetNamespaceOptionsInputType); diff --git a/src/node_options.h b/src/node_options.h index 367f2842d86e58..59409e6926d353 100644 --- a/src/node_options.h +++ b/src/node_options.h @@ -649,6 +649,8 @@ class OptionsParser { friend std::vector MapAvailableNamespaces(); friend void GetEnvOptionsInputType( const v8::FunctionCallbackInfo& args); + friend void GetOptionsAsFlags( + const v8::FunctionCallbackInfo& args); }; using StringVector = std::vector; diff --git a/test/fixtures/options-as-flags/.test.env b/test/fixtures/options-as-flags/.test.env new file mode 100644 index 00000000000000..6c92e1684b2ed2 --- /dev/null +++ b/test/fixtures/options-as-flags/.test.env @@ -0,0 +1 @@ +NODE_OPTIONS=--v8-pool-size=8 diff --git a/test/fixtures/options-as-flags/fixture.cjs b/test/fixtures/options-as-flags/fixture.cjs new file mode 100644 index 00000000000000..e2a61b52c7b176 --- /dev/null +++ b/test/fixtures/options-as-flags/fixture.cjs @@ -0,0 +1,4 @@ +const { getOptionsAsFlagsFromBinding } = require('internal/options'); + +const flags = getOptionsAsFlagsFromBinding(); +console.log(JSON.stringify(flags.sort())); diff --git a/test/fixtures/options-as-flags/test-config.json b/test/fixtures/options-as-flags/test-config.json new file mode 100644 index 00000000000000..c80ffa4069fbf5 --- /dev/null +++ b/test/fixtures/options-as-flags/test-config.json @@ -0,0 +1,9 @@ +{ + "nodeOptions": { + "experimental-transform-types": true, + "max-http-header-size": 8192 + }, + "testRunner": { + "test-isolation": "none" + } +} diff --git a/test/fixtures/test-runner/flag-propagation/.env b/test/fixtures/test-runner/flag-propagation/.env new file mode 100644 index 00000000000000..e69de29bb2d1d6 diff --git a/test/fixtures/test-runner/flag-propagation/index.js b/test/fixtures/test-runner/flag-propagation/index.js new file mode 100644 index 00000000000000..77dc08acfe464d --- /dev/null +++ b/test/fixtures/test-runner/flag-propagation/index.js @@ -0,0 +1 @@ +// Empty file used by test/parallel/test-runner-flag-propagation.js \ No newline at end of file diff --git a/test/fixtures/test-runner/flag-propagation/node.config.json b/test/fixtures/test-runner/flag-propagation/node.config.json new file mode 100644 index 00000000000000..54bcbfef04a947 --- /dev/null +++ b/test/fixtures/test-runner/flag-propagation/node.config.json @@ -0,0 +1,5 @@ +{ + "nodeOptions": { + "max-http-header-size": 10 + } +} diff --git a/test/fixtures/test-runner/flag-propagation/runner.mjs b/test/fixtures/test-runner/flag-propagation/runner.mjs new file mode 100644 index 00000000000000..162c7c8566a858 --- /dev/null +++ b/test/fixtures/test-runner/flag-propagation/runner.mjs @@ -0,0 +1,33 @@ +import { run } from 'node:test'; +import { tap } from 'node:test/reporters'; +import { parseArgs } from 'node:util'; + +const options = { + flag: { + type: 'string', + default: '', + }, + expected: { + type: 'string', + default: '', + }, + description: { + type: 'string', + default: 'flag propagation test', + }, +}; + +const { values } = parseArgs({ args: process.argv.slice(2), options }); + +const argv = [ + `--flag=${values.flag}`, + `--expected=${values.expected}`, + `--description="${values.description}"`, +].filter(Boolean); + +run({ + files: ['./test.mjs'], + cwd: process.cwd(), + argv, + isolation: 'process', +}).compose(tap).pipe(process.stdout); diff --git a/test/fixtures/test-runner/flag-propagation/test.mjs b/test/fixtures/test-runner/flag-propagation/test.mjs new file mode 100644 index 00000000000000..c47f7053ffda2b --- /dev/null +++ b/test/fixtures/test-runner/flag-propagation/test.mjs @@ -0,0 +1,46 @@ +import { test } from 'node:test'; +import { deepStrictEqual } from 'node:assert'; +import internal from 'internal/options'; +import { parseArgs } from 'node:util'; + +const options = { + flag: { + type: 'string', + default: '', + }, + expected: { + type: 'string', + default: '', + }, + description: { + type: 'string', + default: 'flag propagation test', + }, +}; + + +const { values } = parseArgs({ args: process.argv.slice(2), options }); + +let { flag, expected, description } = values; + +test(description, () => { + const optionValue = internal.getOptionValue(flag); + const isArrayOption = Array.isArray(optionValue); + + if (isArrayOption) { + expected = [expected]; + } + + console.error(`testing flag: ${flag}, found value: ${optionValue}, expected: ${expected}`); + + const isNumber = !isNaN(Number(expected)); + const booleanValue = expected === 'true' || expected === 'false'; + if (booleanValue) { + deepStrictEqual(optionValue, expected === 'true'); + return; + } else if (isNumber) { + deepStrictEqual(Number(optionValue), Number(expected)); + } else{ + deepStrictEqual(optionValue, expected); + } +}); diff --git a/test/parallel/test-cli-options-as-flags.js b/test/parallel/test-cli-options-as-flags.js new file mode 100644 index 00000000000000..c9d92b69f72a45 --- /dev/null +++ b/test/parallel/test-cli-options-as-flags.js @@ -0,0 +1,111 @@ +'use strict'; + +const { + spawnPromisified, +} = require('../common'); +const fixtures = require('../common/fixtures'); +const { strictEqual } = require('node:assert'); +const { describe, it } = require('node:test'); +const path = require('node:path'); + +const fixtureFile = fixtures.path(path.join('options-as-flags', 'fixture.cjs')); +const configFile = fixtures.path(path.join('options-as-flags', 'test-config.json')); +const envFile = fixtures.path(path.join('options-as-flags', '.test.env')); + +describe('getOptionsAsFlagsFromBinding', () => { + it('should extract flags from command line arguments', async () => { + const result = await spawnPromisified(process.execPath, [ + '--no-warnings', + '--expose-internals', + '--stack-trace-limit=512', + fixtureFile, + ]); + + strictEqual(result.code, 0); + const flags = JSON.parse(result.stdout.trim()); + + strictEqual(flags.includes('--no-warnings'), true); + strictEqual(flags.includes('--stack-trace-limit=512'), true); + }); + + it('should extract flags from NODE_OPTIONS environment variable', async () => { + const result = await spawnPromisified(process.execPath, [ + '--no-warnings', + '--expose-internals', + fixtureFile, + ], { + env: { + ...process.env, + NODE_OPTIONS: '--stack-trace-limit=4096' + } + }); + + strictEqual(result.code, 0); + const flags = JSON.parse(result.stdout.trim()); + + // Should contain the flag from NODE_OPTIONS + strictEqual(flags.includes('--stack-trace-limit=4096'), true); + // Should also contain command line flags + strictEqual(flags.includes('--no-warnings'), true); + }); + + it('should extract flags from config file', async () => { + const result = await spawnPromisified(process.execPath, [ + '--no-warnings', + '--expose-internals', + '--experimental-config-file', + configFile, + fixtureFile, + ]); + + strictEqual(result.code, 0); + const flags = JSON.parse(result.stdout.trim()); + + // Should contain flags from config file + strictEqual(flags.includes('--experimental-transform-types'), true); + strictEqual(flags.includes('--max-http-header-size=8192'), true); + strictEqual(flags.includes('--test-isolation=none'), true); + // Should also contain command line flags + strictEqual(flags.includes('--no-warnings'), true); + }); + + it('should extract flags from config file and command line', async () => { + const result = await spawnPromisified(process.execPath, [ + '--no-warnings', + '--expose-internals', + '--stack-trace-limit=512', + '--experimental-config-file', + configFile, + fixtureFile, + ]); + + strictEqual(result.code, 0); + const flags = JSON.parse(result.stdout.trim()); + + // Should contain flags from command line arguments + strictEqual(flags.includes('--no-warnings'), true); + strictEqual(flags.includes('--stack-trace-limit=512'), true); + + // Should contain flags from config file + strictEqual(flags.includes('--experimental-transform-types'), true); + strictEqual(flags.includes('--max-http-header-size=8192'), true); + strictEqual(flags.includes('--test-isolation=none'), true); + }); + + it('should extract flags from .env file', async () => { + const result = await spawnPromisified(process.execPath, [ + '--no-warnings', + '--expose-internals', + `--env-file=${envFile}`, + fixtureFile, + ]); + + strictEqual(result.code, 0); + const flags = JSON.parse(result.stdout.trim()); + + // Should contain flags from .env file (NODE_OPTIONS) + strictEqual(flags.includes('--v8-pool-size=8'), true); + // Should also contain command line flags + strictEqual(flags.includes('--no-warnings'), true); + }); +}); diff --git a/test/parallel/test-runner-flag-propagation.js b/test/parallel/test-runner-flag-propagation.js new file mode 100644 index 00000000000000..d5190251ef8db7 --- /dev/null +++ b/test/parallel/test-runner-flag-propagation.js @@ -0,0 +1,128 @@ +'use strict'; + +require('../common'); +const fixtures = require('../common/fixtures.js'); +const tmpdir = require('../common/tmpdir'); +const assert = require('node:assert'); +const fs = require('node:fs'); +const { spawnSync } = require('node:child_process'); +const { describe, it } = require('node:test'); +const path = require('node:path'); + +const fixtureDir = fixtures.path('test-runner', 'flag-propagation'); +const runner = path.join(fixtureDir, 'runner.mjs'); + +describe('test runner flag propagation', () => { + describe('via command line', () => { + const flagPropagationTests = [ + ['--experimental-config-file', 'node.config.json', ''], + ['--experimental-default-config-file', '', false], + ['--env-file', '.env', '.env'], + ['--env-file-if-exists', '.env', '.env'], + ['--test-concurrency', '2', '2'], + ['--test-timeout', '5000', '5000'], + ['--test-coverage-branches', '100', '100'], + ['--test-coverage-functions', '100', '100'], + ['--test-coverage-lines', '100', '100'], + ['--experimental-test-coverage', '', false], + ['--test-coverage-exclude', 'test/**', 'test/**'], + ['--test-coverage-include', 'src/**', 'src/**'], + ['--test-update-snapshots', '', true], + ['--import', './index.js', './index.js'], + ['--require', './index.js', './index.js'], + ]; + + for (const [flagName, testValue, expectedValue] of flagPropagationTests) { + const testDescription = `should propagate ${flagName} to child tests as expected`; + + it(testDescription, () => { + const args = [ + '--test-reporter=tap', + '--no-warnings', + '--expose-internals', + // We need to pass the flag that will be propagated to the child test + testValue ? `${flagName}=${testValue}` : flagName, + // Use the runner fixture + runner, + // Pass parameters to the fixture + `--flag=${flagName}`, + `--expected=${expectedValue}`, + `--description="${testDescription}"`, + ].filter(Boolean); + + const child = spawnSync( + process.execPath, + args, + { + cwd: fixtureDir, + }, + ); + + assert.strictEqual(child.status, 0, `Flag propagation test failed for ${flagName}.`); + const stdout = child.stdout.toString(); + assert.match(stdout, /tests 1/, `Test should execute for ${flagName}`); + assert.match(stdout, /pass 1/, `Test should pass for ${flagName} propagation check`); + }); + } + }); + + describe('via config file', () => { + const configFilePropagationTests = [ + ['--test-concurrency', 2, 2, 'testRunner'], + ['--test-timeout', 5000, 5000, 'testRunner'], + ['--test-coverage-branches', 100, 100, 'testRunner'], + ['--test-coverage-functions', 100, 100, 'testRunner'], + ['--test-coverage-lines', 100, 100, 'testRunner'], + ['--experimental-test-coverage', true, false, 'testRunner'], + ['--test-coverage-exclude', 'test/**', 'test/**', 'testRunner'], + ['--test-coverage-include', 'src/**', 'src/**', 'testRunner'], + ['--test-update-snapshots', true, true, 'testRunner'], + ['--test-concurrency', 3, 3, 'testRunner'], + ['--test-timeout', 2500, 2500, 'testRunner'], + ['--test-coverage-branches', 90, 90, 'testRunner'], + ['--test-coverage-functions', 85, 85, 'testRunner'], + ]; + + for (const [flagName, configValue, expectedValue, namespace] of configFilePropagationTests) { + const testDescription = `should propagate ${flagName} from config file (${namespace}) to child tests`; + + it(testDescription, () => { + tmpdir.refresh(); + + // Create a temporary config file + const configFile = path.join(tmpdir.path, 'test-config.json'); + const configContent = { + [namespace]: { + [flagName.replace('--', '')]: configValue + } + }; + + fs.writeFileSync(configFile, JSON.stringify(configContent, null, 2)); + + const args = [ + '--test-reporter=tap', + '--no-warnings', + '--expose-internals', + `--experimental-config-file=${configFile}`, + runner, + `--flag=${flagName}`, + `--expected=${expectedValue}`, + `--description="${testDescription}"`, + ]; + + const child = spawnSync( + process.execPath, + args, + { + cwd: fixtureDir, + }, + ); + + assert.strictEqual(child.status, 0, `Config file propagation test failed for ${flagName}.`); + const stdout = child.stdout.toString(); + assert.match(stdout, /tests 1/, `Test should execute for config file ${flagName}`); + assert.match(stdout, /pass 1/, `Test should pass for config file ${flagName} propagation check`); + }); + } + }); +});