Skip to content

Commit 0d340fb

Browse files
committed
src: allow config file to use kDisallowedInEnvvar option flag
1 parent 1a5096c commit 0d340fb

File tree

8 files changed

+106
-17
lines changed

8 files changed

+106
-17
lines changed

doc/node-config-schema.json

+9
Original file line numberDiff line numberDiff line change
@@ -589,6 +589,15 @@
589589
"type": "object",
590590
"additionalProperties": false,
591591
"properties": {
592+
"test": {
593+
"type": "boolean"
594+
},
595+
"test-concurrency": {
596+
"type": "number"
597+
},
598+
"test-global-setup": {
599+
"type": "string"
600+
},
592601
"test-isolation": {
593602
"type": "string"
594603
}

lib/internal/test_runner/runner.js

+7-1
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,13 @@ let debug = require('internal/util/debuglog').debuglog('test_runner', (fn) => {
9898
});
9999

100100
const kIsolatedProcessName = Symbol('kIsolatedProcessName');
101-
const kFilterArgs = ['--test', '--experimental-test-coverage', '--watch'];
101+
const kFilterArgs = [
102+
'--test',
103+
'--experimental-test-coverage',
104+
'--watch',
105+
'--experimental-default-config-file',
106+
'-experimental-config-file',
107+
];
102108
const kFilterArgValues = ['--test-reporter', '--test-reporter-destination'];
103109
const kDiagnosticsFilterArgs = ['tests', 'suites', 'pass', 'fail', 'cancelled', 'skipped', 'todo', 'duration_ms'];
104110

src/node.cc

+12-1
Original file line numberDiff line numberDiff line change
@@ -956,9 +956,20 @@ static ExitCode InitializeNodeWithArgsInternal(
956956
#endif
957957

958958
if (!(flags & ProcessInitializationFlags::kDisableCLIOptions)) {
959-
const ExitCode exit_code =
959+
ExitCode exit_code =
960960
ProcessGlobalArgsInternal(argv, exec_argv, errors, kDisallowedInEnvvar);
961961
if (exit_code != ExitCode::kNoFailure) return exit_code;
962+
963+
// Parse options coming from the command line.
964+
// Append here the extra flag that are coming from config file
965+
std::vector<std::string> extra_argv =
966+
per_process::config_reader.AssignNodeNonEnvOptions();
967+
// [0] is expected to be the program name, fill it in from the real argv.
968+
extra_argv.insert(extra_argv.begin(), argv->at(0));
969+
// Parse the extra argv coming from the config file
970+
exit_code = ProcessGlobalArgsInternal(
971+
&extra_argv, nullptr, errors, kDisallowedInEnvvar);
972+
if (exit_code != ExitCode::kNoFailure) return exit_code;
962973
}
963974

964975
// Set the process.title immediately after processing argv if --title is set.

src/node_config_file.cc

+41-10
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ ParseResult ConfigReader::ProcessOptionValue(
4343
simdjson::ondemand::value& ondemand_value,
4444
options_parser::OptionType option_type,
4545
std::vector<std::string>* output,
46-
std::unordered_set<std::string>* unique_options) {
46+
std::unordered_set<std::string>* unique_options = nullptr) {
4747
switch (option_type) {
4848
case options_parser::OptionType::kBoolean: {
4949
bool result;
@@ -142,7 +142,9 @@ ParseResult ConfigReader::ProcessOptionValue(
142142
default:
143143
UNREACHABLE();
144144
}
145-
unique_options->insert(option_name);
145+
if (unique_options != nullptr) {
146+
unique_options->insert(option_name);
147+
}
146148
return ParseResult::Valid;
147149
}
148150

@@ -189,7 +191,14 @@ return ParseResult::Valid;
189191
ParseResult ConfigReader::ParseNamespaceOptions(
190192
simdjson::ondemand::object* options_object,
191193
const std::string& namespace_name) {
194+
// MapOptions could send also options non settable via nodeOptions
192195
auto options_map = options_parser::MapOptionsByNamespace(namespace_name);
196+
197+
if (!env_options_initialized_) {
198+
env_options_map_ = options_parser::MapEnvOptionsFlagInputType();
199+
env_options_initialized_ = true;
200+
}
201+
193202
simdjson::ondemand::value ondemand_value;
194203
std::string_view key;
195204

@@ -210,14 +219,32 @@ ParseResult ConfigReader::ParseNamespaceOptions(
210219
it->first.c_str());
211220
return ParseResult::InvalidContent;
212221
}
213-
ParseResult result = ProcessOptionValue(key,
214-
it->first,
215-
ondemand_value,
216-
it->second,
217-
&namespace_options_,
218-
&unique_namespace_options_);
219-
if (result != ParseResult::Valid) {
220-
return result;
222+
223+
bool is_allowed_in_envvar =
224+
env_options_map_.find(it->first) != env_options_map_.end();
225+
if (is_allowed_in_envvar) {
226+
// Process the option for env options
227+
ParseResult result = ProcessOptionValue(key,
228+
it->first,
229+
ondemand_value,
230+
it->second,
231+
&namespace_options_,
232+
&unique_namespace_options_);
233+
if (result != ParseResult::Valid) {
234+
return result;
235+
}
236+
} else {
237+
// Process the option for non-env options (don't add to
238+
// unique_namespace_options_)
239+
ParseResult result = ProcessOptionValue(key,
240+
it->first,
241+
ondemand_value,
242+
it->second,
243+
&namespace_non_env_options_,
244+
nullptr);
245+
if (result != ParseResult::Valid) {
246+
return result;
247+
}
221248
}
222249
} else {
223250
FPrintF(stderr,
@@ -337,6 +364,10 @@ std::string ConfigReader::AssignNodeOptions() {
337364
return acc;
338365
}
339366

367+
std::vector<std::string> ConfigReader::AssignNodeNonEnvOptions() {
368+
return namespace_non_env_options_;
369+
}
370+
340371
size_t ConfigReader::GetFlagsSize() {
341372
return node_options_.size() + namespace_options_.size();
342373
}

src/node_config_file.h

+7
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
#include <map>
77
#include <string>
8+
#include <unordered_map>
89
#include <unordered_set>
910
#include <variant>
1011
#include <vector>
@@ -32,6 +33,7 @@ class ConfigReader {
3233
const std::vector<std::string>& args);
3334

3435
std::string AssignNodeOptions();
36+
std::vector<std::string> AssignNodeNonEnvOptions();
3537

3638
size_t GetFlagsSize();
3739

@@ -56,6 +58,11 @@ class ConfigReader {
5658
std::vector<std::string> node_options_;
5759
std::unordered_set<std::string> unique_namespace_options_;
5860
std::vector<std::string> namespace_options_;
61+
std::vector<std::string> namespace_non_env_options_;
62+
63+
// Cache for fast lookup of environment options
64+
std::unordered_map<std::string, options_parser::OptionType> env_options_map_;
65+
bool env_options_initialized_ = false;
5966
};
6067

6168
} // namespace node

src/node_options.cc

+10-5
Original file line numberDiff line numberDiff line change
@@ -753,10 +753,15 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
753753
&EnvironmentOptions::experimental_default_config_file);
754754
AddOption("--test",
755755
"launch test runner on startup",
756-
&EnvironmentOptions::test_runner);
756+
&EnvironmentOptions::test_runner,
757+
kDisallowedInEnvvar,
758+
false,
759+
"test_runner");
757760
AddOption("--test-concurrency",
758761
"specify test runner concurrency",
759-
&EnvironmentOptions::test_runner_concurrency);
762+
&EnvironmentOptions::test_runner_concurrency,
763+
kDisallowedInEnvvar,
764+
"test_runner");
760765
AddOption("--test-force-exit",
761766
"force test runner to exit upon completion",
762767
&EnvironmentOptions::test_runner_force_exit);
@@ -827,7 +832,8 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
827832
AddOption("--test-global-setup",
828833
"specifies the path to the global setup file",
829834
&EnvironmentOptions::test_global_setup_path,
830-
kAllowedInEnvvar);
835+
kAllowedInEnvvar,
836+
"test_runner");
831837
AddOption("--test-udp-no-try-send", "", // For testing only.
832838
&EnvironmentOptions::test_udp_no_try_send);
833839
AddOption("--throw-deprecation",
@@ -1405,8 +1411,7 @@ MapOptionsByNamespace(std::string namespace_name) {
14051411
const auto& parser = _ppop_instance;
14061412
for (const auto& item : parser.options_) {
14071413
if (!item.first.empty() && !item.first.starts_with('[') &&
1408-
item.second.namespace_id == namespace_name &&
1409-
item.second.env_setting == kAllowedInEnvvar) {
1414+
item.second.namespace_id == namespace_name) {
14101415
type_map[item.first] = item.second.type;
14111416
}
14121417
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"test_runner": {
3+
"test-concurrency": 1
4+
}
5+
}

test/parallel/test-config-file.js

+15
Original file line numberDiff line numberDiff line change
@@ -451,4 +451,19 @@ describe('namespaced options', () => {
451451
strictEqual(result.stdout, '');
452452
strictEqual(result.code, 9);
453453
});
454+
455+
it('should allow setting kDisallowedInEnvvar in the config file if part of a namespace', async () => {
456+
// This test assumes that the --test-concurrency flag is configured as kDisallowedInEnvVar
457+
// and that it is part of at least one namespace.
458+
const result = await spawnPromisified(process.execPath, [
459+
'--no-warnings',
460+
'--expose-internals',
461+
'--experimental-config-file',
462+
fixtures.path('rc/namespace-with-disallowed-envvar.json'),
463+
'-p', 'require("internal/options").getOptionValue("--test-concurrency")',
464+
]);
465+
strictEqual(result.stderr, '');
466+
strictEqual(result.stdout, '1\n');
467+
strictEqual(result.code, 0);
468+
});
454469
});

0 commit comments

Comments
 (0)