test: migrate test runner message tests to snapshot

PR-URL: #47392
Reviewed-By: Geoffrey Booth <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>

.github/CODEOWNERS

@@ -135,7 +135,6 @@
 
 # Test runner
 
-/test/message/test_runner_* @nodejs/test_runner
 /test/parallel/test-runner-* @nodejs/test_runner
 /doc/api/test.md @nodejs/test_runner
 /lib/test.js @nodejs/test_runner

test/common/README.md

@@ -642,6 +642,12 @@ environment variables.
 If set, `NODE_COMMON_PORT`'s value overrides the `common.PORT` default value of
 12346\.
 
+### `NODE_REGENERATE_SNAPSHOTS`
+
+If set, test snapshots for a the current test are regenerated.
+for example `NODE_REGENERATE_SNAPSHOTS=1 out/Release/node test/parallel/test-runner-output.mjs`
+will update all the test runner output snapshots.
+
 ### `NODE_SKIP_FLAG_CHECK`
 
 If set, command line arguments passed to individual tests are not validated.

test/common/assertSnapshot.js

@@ -0,0 +1,53 @@
+'use strict';
+const common = require('.');
+const path = require('node:path');
+const fs = require('node:fs/promises');
+const assert = require('node:assert/strict');
+
+
+const stackFramesRegexp = /(\s+)((.+?)\s+\()?(?:\(?(.+?):(\d+)(?::(\d+))?)\)?(\s+\{)?(\n|$)/g;
+const windowNewlineRegexp = /\r/g;
+
+function replaceStackTrace(str) {
+  return str.replace(stackFramesRegexp, '$1*$7\n');
+}
+
+function replaceWindowsLineEndings(str) {
+  return str.replace(windowNewlineRegexp, '');
+}
+
+function transform(...args) {
+  return (str) => args.reduce((acc, fn) => fn(acc), str);
+}
+
+function getSnapshotPath(filename) {
+  const { name, dir } = path.parse(filename);
+  return path.resolve(dir, `${name}.snapshot`);
+}
+
+async function assertSnapshot(actual, filename = process.argv[1]) {
+  const snapshot = getSnapshotPath(filename);
+  if (process.env.NODE_REGENERATE_SNAPSHOTS) {
+    await fs.writeFile(snapshot, actual);
+  } else {
+    const expected = await fs.readFile(snapshot, 'utf8');
+    assert.strictEqual(actual, replaceWindowsLineEndings(expected));
+  }
+}
+
+async function spawnAndAssert(filename, transform = (x) => x) {
+  // TODO: Add an option to this function to alternatively or additionally compare stderr.
+  // For now, tests that want to check stderr or both stdout and stderr can use spawnPromisified.
+  const flags = common.parseTestFlags(filename);
+  const { stdout } = await common.spawnPromisified(process.execPath, [...flags, filename]);
+  await assertSnapshot(transform(stdout), filename);
+}
+
+module.exports = {
+  assertSnapshot,
+  getSnapshotPath,
+  replaceStackTrace,
+  replaceWindowsLineEndings,
+  spawnAndAssert,
+  transform,
+};

test/common/index.js

@@ -61,6 +61,31 @@ const hasOpenSSL3 = hasCrypto &&
 
 const hasQuic = hasCrypto && !!process.config.variables.openssl_quic;
 
+function parseTestFlags(filename = process.argv[1]) {
+  // The copyright notice is relatively big and the flags could come afterwards.
+  const bytesToRead = 1500;
+  const buffer = Buffer.allocUnsafe(bytesToRead);
+  const fd = fs.openSync(filename, 'r');
+  const bytesRead = fs.readSync(fd, buffer, 0, bytesToRead);
+  fs.closeSync(fd);
+  const source = buffer.toString('utf8', 0, bytesRead);
+
+  const flagStart = source.indexOf('// Flags: --') + 10;
+
+  if (flagStart === 9) {
+    return [];
+  }
+  let flagEnd = source.indexOf('\n', flagStart);
+  // Normalize different EOL.
+  if (source[flagEnd - 1] === '\r') {
+    flagEnd--;
+  }
+  return source
+    .substring(flagStart, flagEnd)
+    .replace(/_/g, '-')
+    .split(' ');
+}
+
 // Check for flags. Skip this for workers (both, the `cluster` module and
 // `worker_threads`) and child processes.
 // If the binary was built without-ssl then the crypto flags are
@@ -71,44 +96,25 @@ if (process.argv.length === 2 &&
     hasCrypto &&
     require('cluster').isPrimary &&
     fs.existsSync(process.argv[1])) {
-  // The copyright notice is relatively big and the flags could come afterwards.
-  const bytesToRead = 1500;
-  const buffer = Buffer.allocUnsafe(bytesToRead);
-  const fd = fs.openSync(process.argv[1], 'r');
-  const bytesRead = fs.readSync(fd, buffer, 0, bytesToRead);
-  fs.closeSync(fd);
-  const source = buffer.toString('utf8', 0, bytesRead);
-
-  const flagStart = source.indexOf('// Flags: --') + 10;
-  if (flagStart !== 9) {
-    let flagEnd = source.indexOf('\n', flagStart);
-    // Normalize different EOL.
-    if (source[flagEnd - 1] === '\r') {
-      flagEnd--;
-    }
-    const flags = source
-      .substring(flagStart, flagEnd)
-      .replace(/_/g, '-')
-      .split(' ');
-    const args = process.execArgv.map((arg) => arg.replace(/_/g, '-'));
-    for (const flag of flags) {
-      if (!args.includes(flag) &&
-          // If the binary is build without `intl` the inspect option is
-          // invalid. The test itself should handle this case.
-          (process.features.inspector || !flag.startsWith('--inspect'))) {
-        console.log(
-          'NOTE: The test started as a child_process using these flags:',
-          inspect(flags),
-          'Use NODE_SKIP_FLAG_CHECK to run the test with the original flags.',
-        );
-        const args = [...flags, ...process.execArgv, ...process.argv.slice(1)];
-        const options = { encoding: 'utf8', stdio: 'inherit' };
-        const result = spawnSync(process.execPath, args, options);
-        if (result.signal) {
-          process.kill(0, result.signal);
-        } else {
-          process.exit(result.status);
-        }
+  const flags = parseTestFlags();
+  const args = process.execArgv.map((arg) => arg.replace(/_/g, '-'));
+  for (const flag of flags) {
+    if (!args.includes(flag) &&
+        // If the binary is build without `intl` the inspect option is
+        // invalid. The test itself should handle this case.
+        (process.features.inspector || !flag.startsWith('--inspect'))) {
+      console.log(
+        'NOTE: The test started as a child_process using these flags:',
+        inspect(flags),
+        'Use NODE_SKIP_FLAG_CHECK to run the test with the original flags.',
+      );
+      const args = [...flags, ...process.execArgv, ...process.argv.slice(1)];
+      const options = { encoding: 'utf8', stdio: 'inherit' };
+      const result = spawnSync(process.execPath, args, options);
+      if (result.signal) {
+        process.kill(0, result.signal);
+      } else {
+        process.exit(result.status);
       }
     }
   }
@@ -929,6 +935,7 @@ const common = {
   mustSucceed,
   nodeProcessAborted,
   PIPE,
+  parseTestFlags,
   platformTimeout,
   printSkipMessage,
   pwdCommand,

test/common/index.mjs

@@ -37,6 +37,7 @@ const {
   getCallSite,
   mustNotCall,
   mustNotMutateObjectDeep,
+  parseTestFlags,
   printSkipMessage,
   skip,
   nodeProcessAborted,
@@ -88,6 +89,7 @@ export {
   getCallSite,
   mustNotCall,
   mustNotMutateObjectDeep,
+  parseTestFlags,
   printSkipMessage,
   skip,
   nodeProcessAborted,

test/message/test_runner_abort.js → test/fixtures/test-runner/output/abort.js

@@ -1,6 +1,6 @@
 // Flags: --no-warnings
 'use strict';
-require('../common');
+require('../../../common');
 const test = require('node:test');
 
 test('promise timeout signal', { signal: AbortSignal.timeout(1) }, async (t) => {

test/message/test_runner_abort.out → test/fixtures/test-runner/output/abort.snapshot

@@ -31,15 +31,15 @@ TAP version 13
     # Subtest: not ok 2
     not ok 6 - not ok 2
       ---
-      duration_ms: *
+      duration_ms: ZERO
       failureType: 'cancelledByParent'
       error: 'test did not finish before its parent and was cancelled'
       code: 'ERR_TEST_FAILURE'
       ...
     # Subtest: not ok 3
     not ok 7 - not ok 3
       ---
-      duration_ms: *
+      duration_ms: ZERO
       failureType: 'testAborted'
       error: 'This operation was aborted'
       code: 20
@@ -59,7 +59,7 @@ TAP version 13
     # Subtest: not ok 4
     not ok 8 - not ok 4
       ---
-      duration_ms: *
+      duration_ms: ZERO
       failureType: 'testAborted'
       error: 'This operation was aborted'
       code: 20
@@ -79,7 +79,7 @@ TAP version 13
     # Subtest: not ok 5
     not ok 9 - not ok 5
       ---
-      duration_ms: *
+      duration_ms: ZERO
       failureType: 'testAborted'
       error: 'This operation was aborted'
       code: 20
@@ -161,15 +161,15 @@ not ok 2 - promise abort signal
     # Subtest: not ok 2
     not ok 6 - not ok 2
       ---
-      duration_ms: *
+      duration_ms: ZERO
       failureType: 'cancelledByParent'
       error: 'test did not finish before its parent and was cancelled'
       code: 'ERR_TEST_FAILURE'
       ...
     # Subtest: not ok 3
     not ok 7 - not ok 3
       ---
-      duration_ms: *
+      duration_ms: ZERO
       failureType: 'testAborted'
       error: 'This operation was aborted'
       code: 20
@@ -189,7 +189,7 @@ not ok 2 - promise abort signal
     # Subtest: not ok 4
     not ok 8 - not ok 4
       ---
-      duration_ms: *
+      duration_ms: ZERO
       failureType: 'testAborted'
       error: 'This operation was aborted'
       code: 20
@@ -209,7 +209,7 @@ not ok 2 - promise abort signal
     # Subtest: not ok 5
     not ok 9 - not ok 5
       ---
-      duration_ms: *
+      duration_ms: ZERO
       failureType: 'testAborted'
       error: 'This operation was aborted'
       code: 20

test/message/test_runner_abort_suite.js → test/fixtures/test-runner/output/abort_suite.js

@@ -1,6 +1,6 @@
 // Flags: --no-warnings
 'use strict';
-require('../common');
+require('../../../common');
 const { describe, it } = require('node:test');
 
 describe('describe timeout signal', { signal: AbortSignal.timeout(1) }, (t) => {

test/message/test_runner_abort_suite.out → test/fixtures/test-runner/output/abort_suite.snapshot

@@ -31,31 +31,31 @@ TAP version 13
     # Subtest: not ok 2
     not ok 6 - not ok 2
       ---
-      duration_ms: *
+      duration_ms: ZERO
       failureType: 'cancelledByParent'
       error: 'test did not finish before its parent and was cancelled'
       code: 'ERR_TEST_FAILURE'
       ...
     # Subtest: not ok 3
     not ok 7 - not ok 3
       ---
-      duration_ms: *
+      duration_ms: ZERO
       failureType: 'cancelledByParent'
       error: 'test did not finish before its parent and was cancelled'
       code: 'ERR_TEST_FAILURE'
       ...
     # Subtest: not ok 4
     not ok 8 - not ok 4
       ---
-      duration_ms: *
+      duration_ms: ZERO
       failureType: 'cancelledByParent'
       error: 'test did not finish before its parent and was cancelled'
       code: 'ERR_TEST_FAILURE'
       ...
     # Subtest: not ok 5
     not ok 9 - not ok 5
       ---
-      duration_ms: *
+      duration_ms: ZERO
       failureType: 'cancelledByParent'
       error: 'test did not finish before its parent and was cancelled'
       code: 'ERR_TEST_FAILURE'

test/message/test_runner_describe_it.js → test/fixtures/test-runner/output/describe_it.js

@@ -1,6 +1,6 @@
 // Flags: --no-warnings
 'use strict';
-require('../common');
+require('../../../common');
 const assert = require('node:assert');
 const { describe, it, test } = require('node:test');
 const util = require('util');

test/message/test_runner_describe_it.out → test/fixtures/test-runner/output/describe_it.snapshot

@@ -196,10 +196,10 @@ ok 21 - immediate resolve pass
         *
         *
         *
+        new Promise (<anonymous>)
         *
         *
-        *
-        *
+        Array.map (<anonymous>)
       ...
     # Subtest: mixing describe/it and test should work
     ok 2 - mixing describe/it and test should work
@@ -469,10 +469,10 @@ not ok 53 - custom inspect symbol that throws fail
         *
         *
         *
+        new Promise (<anonymous>)
         *
         *
-        *
-        *
+        Array.map (<anonymous>)
       ...
     # Subtest: sync throw fails at second
     not ok 2 - sync throw fails at second
@@ -491,7 +491,7 @@ not ok 53 - custom inspect symbol that throws fail
         *
         *
         *
-        *
+        async Promise.all (index 0)
       ...
     1..2
 not ok 54 - subtest sync throw fails
@@ -506,7 +506,7 @@ not ok 54 - subtest sync throw fails
     # Subtest: should not run
     not ok 1 - should not run
       ---
-      duration_ms: *
+      duration_ms: ZERO
       failureType: 'cancelledByParent'
       error: 'test did not finish before its parent and was cancelled'
       code: 'ERR_TEST_FAILURE'
@@ -535,7 +535,7 @@ not ok 55 - describe sync throw fails
     # Subtest: should not run
     not ok 1 - should not run
       ---
-      duration_ms: *
+      duration_ms: ZERO
       failureType: 'cancelledByParent'
       error: 'test did not finish before its parent and was cancelled'
       code: 'ERR_TEST_FAILURE'

test/message/test_runner_describe_nested.js → test/fixtures/test-runner/output/describe_nested.js

@@ -1,6 +1,6 @@
 // Flags: --no-warnings
 'use strict';
-require('../common');
+require('../../../common');
 const { describe, it } = require('node:test');
 
 describe('nested - no tests', () => {

test/fixtures/test-runner/output/dot_reporter.js

@@ -0,0 +1,8 @@
+// Flags: --no-warnings
+'use strict';
+require('../../../common');
+const fixtures = require('../../../common/fixtures');
+const spawn = require('node:child_process').spawn;
+
+spawn(process.execPath,
+      ['--no-warnings', '--test-reporter', 'dot', fixtures.path('test-runner/output/output.js')], { stdio: 'inherit' });