test: add parseTestMetadata support

PR-URL: #59503
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>

Author: pmarchini

test/common/assertSnapshot.js

@@ -84,7 +84,7 @@ async function spawnAndAssert(filename, transform = (x) => x, { tty = false, ...
     test({ skip: 'Skipping pseudo-tty tests, as pseudo terminals are not available on Windows.' });
     return;
   }
-  let flags = common.parseTestFlags(filename);
+  let { flags } = common.parseTestMetadata(filename);
   if (options.flags) {
     flags = [...options.flags, ...flags];
   }

test/common/index.js

@@ -58,8 +58,16 @@ const hasSQLite = Boolean(process.versions.sqlite);
 
 const hasQuic = hasCrypto && !!process.config.variables.node_quic;
 
-function parseTestFlags(filename = process.argv[1]) {
-  // The copyright notice is relatively big and the flags could come afterwards.
+/**
+ * Parse test metadata from the specified file.
+ * @param {string} filename - The name of the file to parse.
+ * @returns {{
+ *   flags: string[],
+ *   envs: Record<string, string>
+ * }} An object containing the parsed flags and environment variables.
+ */
+function parseTestMetadata(filename = process.argv[1]) {
+  // The copyright notice is relatively big and the metadata could come afterwards.
   const bytesToRead = 1500;
   const buffer = Buffer.allocUnsafe(bytesToRead);
   const fd = fs.openSync(filename, 'r');
@@ -68,19 +76,33 @@ function parseTestFlags(filename = process.argv[1]) {
   const source = buffer.toString('utf8', 0, bytesRead);
 
   const flagStart = source.search(/\/\/ Flags:\s+--/) + 10;
-
-  if (flagStart === 9) {
-    return [];
+  let flags = [];
+  if (flagStart !== 9) {
+    let flagEnd = source.indexOf('\n', flagStart);
+    if (source[flagEnd - 1] === '\r') {
+      flagEnd--;
+    }
+    flags = source
+      .substring(flagStart, flagEnd)
+      .split(/\s+/)
+      .filter(Boolean);
   }
-  let flagEnd = source.indexOf('\n', flagStart);
-  // Normalize different EOL.
-  if (source[flagEnd - 1] === '\r') {
-    flagEnd--;
+
+  const envStart = source.search(/\/\/ Env:\s+/) + 8;
+  let envs = {};
+  if (envStart !== 7) {
+    let envEnd = source.indexOf('\n', envStart);
+    if (source[envEnd - 1] === '\r') {
+      envEnd--;
+    }
+    const envArray = source
+      .substring(envStart, envEnd)
+      .split(/\s+/)
+      .filter(Boolean);
+    envs = Object.fromEntries(envArray.map((env) => env.split('=')));
   }
-  return source
-    .substring(flagStart, flagEnd)
-    .split(/\s+/)
-    .filter(Boolean);
+
+  return { flags, envs };
 }
 
 // Check for flags. Skip this for workers (both, the `cluster` module and
@@ -93,7 +115,7 @@ if (process.argv.length === 2 &&
     hasCrypto &&
     require('cluster').isPrimary &&
     fs.existsSync(process.argv[1])) {
-  const flags = parseTestFlags();
+  const { flags, envs } = parseTestMetadata();
   for (const flag of flags) {
     if (!process.execArgv.includes(flag) &&
         // If the binary is build without `intl` the inspect option is
@@ -102,11 +124,20 @@ if (process.argv.length === 2 &&
       console.log(
         'NOTE: The test started as a child_process using these flags:',
         inspect(flags),
+        'And these environment variables:',
+        inspect(envs),
         'Use NODE_SKIP_FLAG_CHECK to run the test with the original flags.',
       );
       const { spawnSync } = require('child_process');
       const args = [...flags, ...process.execArgv, ...process.argv.slice(1)];
-      const options = { encoding: 'utf8', stdio: 'inherit' };
+      const options = {
+        encoding: 'utf8',
+        stdio: 'inherit',
+        env: {
+          ...process.env,
+          ...envs,
+        },
+      };
       const result = spawnSync(process.execPath, args, options);
       if (result.signal) {
         process.kill(0, result.signal);
@@ -912,7 +943,7 @@ const common = {
   mustSucceed,
   nodeProcessAborted,
   PIPE,
-  parseTestFlags,
+  parseTestMetadata,
   platformTimeout,
   printSkipMessage,
   pwdCommand,

test/common/index.mjs

@@ -36,7 +36,7 @@ const {
   mustNotMutateObjectDeep,
   mustSucceed,
   nodeProcessAborted,
-  parseTestFlags,
+  parseTestMetadata,
   PIPE,
   platformTimeout,
   printSkipMessage,
@@ -86,7 +86,7 @@ export {
   mustNotMutateObjectDeep,
   mustSucceed,
   nodeProcessAborted,
-  parseTestFlags,
+  parseTestMetadata,
   PIPE,
   platformTimeout,
   printSkipMessage,

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

@@ -0,0 +1,26 @@
+'use strict';
+
+// Env: A_SET_ENV_VAR=A_SET_ENV_VAR_VALUE B_SET_ENV_VAR=B_SET_ENV_VAR_VALUE
+// Flags: --test-isolation=none --expose-internals
+
+require('../common');
+const assert = require('node:assert');
+const { describe, it } = require('node:test');
+
+
+// This test verifies that a test file that requires 'common' can set environment variables
+// via comments in the test file, similar to how we set flags via comments.
+// Ref: https://github.com/nodejs/node/issues/58179
+describe('env var via comment', () => {
+  it('should set env var A_SET_ENV_VAR', () => {
+    assert.strictEqual(process.env.A_SET_ENV_VAR, 'A_SET_ENV_VAR_VALUE');
+  });
+  it('should set env var B_SET_ENV_VAR', () => {
+    assert.strictEqual(process.env.B_SET_ENV_VAR, 'B_SET_ENV_VAR_VALUE');
+  });
+
+  it('should interop with flags', () => {
+    const flag = require('internal/options').getOptionValue('--test-isolation');
+    assert.strictEqual(flag, 'none');
+  });
+});