From b851d9f823ae5c83fb364ac92056cab8dc66ea05 Mon Sep 17 00:00:00 2001 From: Pietro Marchini Date: Mon, 21 Jul 2025 17:54:27 +0200 Subject: [PATCH 1/7] src: add internal GetOptionsAsFlags --- lib/internal/options.js | 7 ++ src/node_options.cc | 115 ++++++++++++++++++ src/node_options.h | 2 + test/fixtures/options-as-flags/.test.env | 1 + test/fixtures/options-as-flags/fixture.cjs | 4 + .../options-as-flags/test-config.json | 9 ++ test/parallel/test-cli-options-as-flags.js | 111 +++++++++++++++++ 7 files changed, 249 insertions(+) create mode 100644 test/fixtures/options-as-flags/.test.env create mode 100644 test/fixtures/options-as-flags/fixture.cjs create mode 100644 test/fixtures/options-as-flags/test-config.json create mode 100644 test/parallel/test-cli-options-as-flags.js 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/src/node_options.cc b/src/node_options.cc index dd68166b1656a5..7732d9423ce663 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -21,6 +21,7 @@ #include #include +using v8::Array; using v8::Boolean; using v8::Context; using v8::FunctionCallbackInfo; @@ -1867,6 +1868,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 +1989,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 +2023,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..3f47cf0445f907 --- /dev/null +++ b/test/fixtures/options-as-flags/.test.env @@ -0,0 +1 @@ +NODE_OPTIONS=--secure-heap=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/parallel/test-cli-options-as-flags.js b/test/parallel/test-cli-options-as-flags.js new file mode 100644 index 00000000000000..08aa723f2c0a3d --- /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('--secure-heap=8'), true); + // Should also contain command line flags + strictEqual(flags.includes('--no-warnings'), true); + }); +}); From 0f0cccf6961e5c7298e9ad2351f4f092e1b60167 Mon Sep 17 00:00:00 2001 From: Pietro Marchini Date: Tue, 22 Jul 2025 09:15:35 +0200 Subject: [PATCH 2/7] fixup! src: add internal GetOptionsAsFlags --- src/node_options.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/src/node_options.cc b/src/node_options.cc index 7732d9423ce663..d37ef9427dc018 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -21,7 +21,6 @@ #include #include -using v8::Array; using v8::Boolean; using v8::Context; using v8::FunctionCallbackInfo; From 7b51777cc7d979828807a9acc70d7795aa4e0df2 Mon Sep 17 00:00:00 2001 From: Pietro Marchini Date: Tue, 29 Jul 2025 09:13:07 +0200 Subject: [PATCH 3/7] test_runner: replace process.execArgv with getOptionsAsFlagsFromBinding --- lib/internal/test_runner/runner.js | 29 ++++---- .../test-runner/flag-propagation/.env | 0 .../flag-propagation/node.config.json | 5 ++ .../test-runner/flag-propagation/runner.mjs | 33 +++++++++ .../test-runner/flag-propagation/test.mjs | 40 +++++++++++ test/parallel/test-runner-flag-propagation.js | 67 +++++++++++++++++++ 6 files changed, 160 insertions(+), 14 deletions(-) create mode 100644 test/fixtures/test-runner/flag-propagation/.env create mode 100644 test/fixtures/test-runner/flag-propagation/node.config.json create mode 100644 test/fixtures/test-runner/flag-propagation/runner.mjs create mode 100644 test/fixtures/test-runner/flag-propagation/test.mjs create mode 100644 test/parallel/test-runner-flag-propagation.js 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/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/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..b1a6e50a3972b8 --- /dev/null +++ b/test/fixtures/test-runner/flag-propagation/test.mjs @@ -0,0 +1,40 @@ +import { test } from 'node:test'; +import { strictEqual } 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 }); + +const { flag, expected, description } = values; + +test(description, () => { + const optionValue = internal.getOptionValue(flag); + console.error(`testing flag: ${flag}, found value: ${optionValue}, expected: ${expected}`); + + const isNumber = !isNaN(Number(expected)); + const booleanValue = expected === 'true' || expected === 'false'; + if (booleanValue) { + strictEqual(optionValue, expected === 'true'); + return; + } else if (isNumber) { + strictEqual(Number(optionValue), Number(expected)); + } else{ + strictEqual(optionValue, expected); + } +}); diff --git a/test/parallel/test-runner-flag-propagation.js b/test/parallel/test-runner-flag-propagation.js new file mode 100644 index 00000000000000..f855e213f263cc --- /dev/null +++ b/test/parallel/test-runner-flag-propagation.js @@ -0,0 +1,67 @@ +'use strict'; + +require('../common'); +const fixtures = require('../common/fixtures.js'); +const assert = require('assert'); +const { spawnSync } = require('child_process'); +const { describe, it } = require('node:test'); +const path = require('path'); + +// Test flag propagation to child test processes +// This validates that certain flags are/aren't propagated to child test processes +// based on the specification in the Node.js documentation +describe('test runner flag propagation', () => { + 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-force-exit', '', true], // <-- this test fails as is forces exit + // ['--test-only', '', true], // <-- this needs to be investigated + ['--test-timeout', '5000', '5000'], + ['--test-coverage-branches', '100', '100'], + ['--test-coverage-functions', '100', '100'], + ['--test-coverage-lines', '100', '100'], + // ['--test-coverage-exclude', 'test/**', 'test/**'], + // ['--test-coverage-include', 'src/**', 'src/**'], + ['--test-update-snapshots', '', true], + ]; + + // Path to the static fixture + const fixtureDir = fixtures.path('test-runner', 'flag-propagation'); + const runner = path.join(fixtureDir, 'runner.mjs'); + + 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`); + }); + } +}); From 3095abc036ccbe4f387ee218708f5c1dd79e990b Mon Sep 17 00:00:00 2001 From: Pietro Marchini Date: Wed, 30 Jul 2025 09:09:26 +0200 Subject: [PATCH 4/7] test: add flags --- .../test-runner/flag-propagation/index.js | 1 + .../test-runner/flag-propagation/test.mjs | 16 +++++++++++----- test/parallel/test-runner-flag-propagation.js | 7 +++++-- 3 files changed, 17 insertions(+), 7 deletions(-) create mode 100644 test/fixtures/test-runner/flag-propagation/index.js 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/test.mjs b/test/fixtures/test-runner/flag-propagation/test.mjs index b1a6e50a3972b8..c47f7053ffda2b 100644 --- a/test/fixtures/test-runner/flag-propagation/test.mjs +++ b/test/fixtures/test-runner/flag-propagation/test.mjs @@ -1,5 +1,5 @@ import { test } from 'node:test'; -import { strictEqual } from 'node:assert'; +import { deepStrictEqual } from 'node:assert'; import internal from 'internal/options'; import { parseArgs } from 'node:util'; @@ -21,20 +21,26 @@ const options = { const { values } = parseArgs({ args: process.argv.slice(2), options }); -const { flag, expected, description } = values; +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) { - strictEqual(optionValue, expected === 'true'); + deepStrictEqual(optionValue, expected === 'true'); return; } else if (isNumber) { - strictEqual(Number(optionValue), Number(expected)); + deepStrictEqual(Number(optionValue), Number(expected)); } else{ - strictEqual(optionValue, expected); + deepStrictEqual(optionValue, expected); } }); diff --git a/test/parallel/test-runner-flag-propagation.js b/test/parallel/test-runner-flag-propagation.js index f855e213f263cc..70d6845f374d8a 100644 --- a/test/parallel/test-runner-flag-propagation.js +++ b/test/parallel/test-runner-flag-propagation.js @@ -23,9 +23,12 @@ describe('test runner flag propagation', () => { ['--test-coverage-branches', '100', '100'], ['--test-coverage-functions', '100', '100'], ['--test-coverage-lines', '100', '100'], - // ['--test-coverage-exclude', 'test/**', 'test/**'], - // ['--test-coverage-include', 'src/**', 'src/**'], + ['--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'], ]; // Path to the static fixture From efb6a55680c2c2b08da892afba4b146038fc84d6 Mon Sep 17 00:00:00 2001 From: Pietro Marchini Date: Wed, 6 Aug 2025 17:21:19 +0200 Subject: [PATCH 5/7] test: cover options via config file --- test/parallel/test-runner-flag-propagation.js | 172 ++++++++++++------ 1 file changed, 115 insertions(+), 57 deletions(-) diff --git a/test/parallel/test-runner-flag-propagation.js b/test/parallel/test-runner-flag-propagation.js index 70d6845f374d8a..d5190251ef8db7 100644 --- a/test/parallel/test-runner-flag-propagation.js +++ b/test/parallel/test-runner-flag-propagation.js @@ -2,69 +2,127 @@ require('../common'); const fixtures = require('../common/fixtures.js'); -const assert = require('assert'); -const { spawnSync } = require('child_process'); +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('path'); +const path = require('node:path'); + +const fixtureDir = fixtures.path('test-runner', 'flag-propagation'); +const runner = path.join(fixtureDir, 'runner.mjs'); -// Test flag propagation to child test processes -// This validates that certain flags are/aren't propagated to child test processes -// based on the specification in the Node.js documentation describe('test runner flag propagation', () => { - 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-force-exit', '', true], // <-- this test fails as is forces exit - // ['--test-only', '', true], // <-- this needs to be investigated - ['--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'], - ]; + 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(); - // Path to the static fixture - const fixtureDir = fixtures.path('test-runner', 'flag-propagation'); - const runner = path.join(fixtureDir, 'runner.mjs'); + // Create a temporary config file + const configFile = path.join(tmpdir.path, 'test-config.json'); + const configContent = { + [namespace]: { + [flagName.replace('--', '')]: configValue + } + }; - for (const [flagName, testValue, expectedValue] of flagPropagationTests) { - const testDescription = `should propagate ${flagName} to child tests as expected`; + fs.writeFileSync(configFile, JSON.stringify(configContent, null, 2)); - 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 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, - }, - ); + 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`); - }); - } + 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`); + }); + } + }); }); From c59932ffe545afc921f2532b15fed803b44d4ab6 Mon Sep 17 00:00:00 2001 From: Pietro Marchini Date: Tue, 12 Aug 2025 14:23:48 +0200 Subject: [PATCH 6/7] doc: add section on child process option inheritance in test runner --- doc/api/test.md | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) 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 From afc38503f7e6c64f471af54c639260658147118f Mon Sep 17 00:00:00 2001 From: Pietro Marchini Date: Thu, 14 Aug 2025 11:55:45 +0200 Subject: [PATCH 7/7] fixup! doc: add section on child process option inheritance in test runner --- test/fixtures/options-as-flags/.test.env | 2 +- test/parallel/test-cli-options-as-flags.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/fixtures/options-as-flags/.test.env b/test/fixtures/options-as-flags/.test.env index 3f47cf0445f907..6c92e1684b2ed2 100644 --- a/test/fixtures/options-as-flags/.test.env +++ b/test/fixtures/options-as-flags/.test.env @@ -1 +1 @@ -NODE_OPTIONS=--secure-heap=8 +NODE_OPTIONS=--v8-pool-size=8 diff --git a/test/parallel/test-cli-options-as-flags.js b/test/parallel/test-cli-options-as-flags.js index 08aa723f2c0a3d..c9d92b69f72a45 100644 --- a/test/parallel/test-cli-options-as-flags.js +++ b/test/parallel/test-cli-options-as-flags.js @@ -104,7 +104,7 @@ describe('getOptionsAsFlagsFromBinding', () => { const flags = JSON.parse(result.stdout.trim()); // Should contain flags from .env file (NODE_OPTIONS) - strictEqual(flags.includes('--secure-heap=8'), true); + strictEqual(flags.includes('--v8-pool-size=8'), true); // Should also contain command line flags strictEqual(flags.includes('--no-warnings'), true); });