Skip to content

Commit e517792

Browse files
pmarchinitargos
authored andcommitted
test: add parseTestMetadata support
PR-URL: #59503 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
1 parent b962560 commit e517792

File tree

4 files changed

+76
-19
lines changed

4 files changed

+76
-19
lines changed

test/common/assertSnapshot.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ async function spawnAndAssert(filename, transform = (x) => x, { tty = false, ...
8484
test({ skip: 'Skipping pseudo-tty tests, as pseudo terminals are not available on Windows.' });
8585
return;
8686
}
87-
let flags = common.parseTestFlags(filename);
87+
let { flags } = common.parseTestMetadata(filename);
8888
if (options.flags) {
8989
flags = [...options.flags, ...flags];
9090
}

test/common/index.js

Lines changed: 47 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,16 @@ const hasSQLite = Boolean(process.versions.sqlite);
5858

5959
const hasQuic = hasCrypto && !!process.config.variables.node_quic;
6060

61-
function parseTestFlags(filename = process.argv[1]) {
62-
// The copyright notice is relatively big and the flags could come afterwards.
61+
/**
62+
* Parse test metadata from the specified file.
63+
* @param {string} filename - The name of the file to parse.
64+
* @returns {{
65+
* flags: string[],
66+
* envs: Record<string, string>
67+
* }} An object containing the parsed flags and environment variables.
68+
*/
69+
function parseTestMetadata(filename = process.argv[1]) {
70+
// The copyright notice is relatively big and the metadata could come afterwards.
6371
const bytesToRead = 1500;
6472
const buffer = Buffer.allocUnsafe(bytesToRead);
6573
const fd = fs.openSync(filename, 'r');
@@ -68,19 +76,33 @@ function parseTestFlags(filename = process.argv[1]) {
6876
const source = buffer.toString('utf8', 0, bytesRead);
6977

7078
const flagStart = source.search(/\/\/ Flags:\s+--/) + 10;
71-
72-
if (flagStart === 9) {
73-
return [];
79+
let flags = [];
80+
if (flagStart !== 9) {
81+
let flagEnd = source.indexOf('\n', flagStart);
82+
if (source[flagEnd - 1] === '\r') {
83+
flagEnd--;
84+
}
85+
flags = source
86+
.substring(flagStart, flagEnd)
87+
.split(/\s+/)
88+
.filter(Boolean);
7489
}
75-
let flagEnd = source.indexOf('\n', flagStart);
76-
// Normalize different EOL.
77-
if (source[flagEnd - 1] === '\r') {
78-
flagEnd--;
90+
91+
const envStart = source.search(/\/\/ Env:\s+/) + 8;
92+
let envs = {};
93+
if (envStart !== 7) {
94+
let envEnd = source.indexOf('\n', envStart);
95+
if (source[envEnd - 1] === '\r') {
96+
envEnd--;
97+
}
98+
const envArray = source
99+
.substring(envStart, envEnd)
100+
.split(/\s+/)
101+
.filter(Boolean);
102+
envs = Object.fromEntries(envArray.map((env) => env.split('=')));
79103
}
80-
return source
81-
.substring(flagStart, flagEnd)
82-
.split(/\s+/)
83-
.filter(Boolean);
104+
105+
return { flags, envs };
84106
}
85107

86108
// Check for flags. Skip this for workers (both, the `cluster` module and
@@ -93,7 +115,7 @@ if (process.argv.length === 2 &&
93115
hasCrypto &&
94116
require('cluster').isPrimary &&
95117
fs.existsSync(process.argv[1])) {
96-
const flags = parseTestFlags();
118+
const { flags, envs } = parseTestMetadata();
97119
for (const flag of flags) {
98120
if (!process.execArgv.includes(flag) &&
99121
// If the binary is build without `intl` the inspect option is
@@ -102,11 +124,20 @@ if (process.argv.length === 2 &&
102124
console.log(
103125
'NOTE: The test started as a child_process using these flags:',
104126
inspect(flags),
127+
'And these environment variables:',
128+
inspect(envs),
105129
'Use NODE_SKIP_FLAG_CHECK to run the test with the original flags.',
106130
);
107131
const { spawnSync } = require('child_process');
108132
const args = [...flags, ...process.execArgv, ...process.argv.slice(1)];
109-
const options = { encoding: 'utf8', stdio: 'inherit' };
133+
const options = {
134+
encoding: 'utf8',
135+
stdio: 'inherit',
136+
env: {
137+
...process.env,
138+
...envs,
139+
},
140+
};
110141
const result = spawnSync(process.execPath, args, options);
111142
if (result.signal) {
112143
process.kill(0, result.signal);
@@ -912,7 +943,7 @@ const common = {
912943
mustSucceed,
913944
nodeProcessAborted,
914945
PIPE,
915-
parseTestFlags,
946+
parseTestMetadata,
916947
platformTimeout,
917948
printSkipMessage,
918949
pwdCommand,

test/common/index.mjs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ const {
3636
mustNotMutateObjectDeep,
3737
mustSucceed,
3838
nodeProcessAborted,
39-
parseTestFlags,
39+
parseTestMetadata,
4040
PIPE,
4141
platformTimeout,
4242
printSkipMessage,
@@ -86,7 +86,7 @@ export {
8686
mustNotMutateObjectDeep,
8787
mustSucceed,
8888
nodeProcessAborted,
89-
parseTestFlags,
89+
parseTestMetadata,
9090
PIPE,
9191
platformTimeout,
9292
printSkipMessage,

test/parallel/test-parse-test-envs.js

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
'use strict';
2+
3+
// Env: A_SET_ENV_VAR=A_SET_ENV_VAR_VALUE B_SET_ENV_VAR=B_SET_ENV_VAR_VALUE
4+
// Flags: --test-isolation=none --expose-internals
5+
6+
require('../common');
7+
const assert = require('node:assert');
8+
const { describe, it } = require('node:test');
9+
10+
11+
// This test verifies that a test file that requires 'common' can set environment variables
12+
// via comments in the test file, similar to how we set flags via comments.
13+
// Ref: https://github.com/nodejs/node/issues/58179
14+
describe('env var via comment', () => {
15+
it('should set env var A_SET_ENV_VAR', () => {
16+
assert.strictEqual(process.env.A_SET_ENV_VAR, 'A_SET_ENV_VAR_VALUE');
17+
});
18+
it('should set env var B_SET_ENV_VAR', () => {
19+
assert.strictEqual(process.env.B_SET_ENV_VAR, 'B_SET_ENV_VAR_VALUE');
20+
});
21+
22+
it('should interop with flags', () => {
23+
const flag = require('internal/options').getOptionValue('--test-isolation');
24+
assert.strictEqual(flag, 'none');
25+
});
26+
});

0 commit comments

Comments
 (0)