test: add spawnSyncAndExit() and spawnSyncAndExitWithoutError()

Replaces expectSyncExit() and expectSyncExitWithoutError(). Since we usually just check the child process right after its spawned, these shorthands also takes care of the spawning. This makes the tests more concise. PR-URL: #49200 Reviewed-By: LiviaMedeiros <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
nodejs · Sep 10, 2023 · 4a85f70 · 4a85f70
1 parent 9610008
commit 4a85f70
Show file tree

Hide file tree

Showing 5 changed files with 66 additions and 73 deletions.
diff --git a/test/common/README.md b/test/common/README.md
@@ -40,17 +40,16 @@ The `benchmark` module is used by tests to run benchmarks.
 
 The `child_process` module is used by tests that launch child processes.
 
-### `expectSyncExit(child, options)`
+### `spawnSyncAndExit(command[, args][, spawnOptions], expectations)`
 
-Checks if a _synchronous_ child process runs in the way expected. If it does
-not, print the stdout and stderr output from the child process and additional
-information about it to the stderr of the current process before throwing
-and error. This helps gathering more information about test failures
-coming from child processes.
+Spawns a child process synchronously using [`child_process.spawnSync()`][] and
+check if it runs in the way expected. If it does not, print the stdout and
+stderr output from the child process and additional information about it to
+the stderr of the current process before throwing and error. This helps
+gathering more information about test failures coming from child processes.
 
-* `child` [\<ChildProcess>][<ChildProcess>]: a `ChildProcess` instance
-  returned by `child_process.spawnSync()`.
-* `options` [\<Object>][<Object>]
+* `command`, `args`, `spawnOptions` See [`child_process.spawnSync()`][]
+* `expectations` [\<Object>][<Object>]
   * `status` [\<number>][<number>] Expected `child.status`
   * `signal` [\<string>][<string>] | `null` Expected `child.signal`
   * `stderr` [\<string>][<string>] | [\<RegExp>][<RegExp>] |
@@ -65,8 +64,13 @@ coming from child processes.
   * `trim` [\<boolean>][<boolean>] Optional. Whether this method should trim
     out the whitespace characters when checking `stderr` and `stdout` outputs.
     Defaults to `false`.
+* return [\<Object>][<Object>]
+  * `child` [\<ChildProcess>][<ChildProcess>] The child process returned by
+    [`child_process.spawnSync()`][].
+  * `stderr` [\<string>][<string>] The output from the child process to stderr.
+  * `stdout` [\<string>][<string>] The output from the child process to stdout.
 
-### `expectSyncExitWithoutError(child[, options])`
+### `spawnSyncAndExitWithoutError(command[, args][, spawnOptions], expectations)`
 
 Similar to `expectSyncExit()` with the `status` expected to be 0 and
 `signal` expected to be `null`. Any other optional options are passed
@@ -1160,6 +1164,7 @@ See [the WPT tests README][] for details.
 [<number>]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Data_structures#Number_type
 [<string>]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Data_structures#String_type
 [Web Platform Tests]: https://github.com/web-platform-tests/wpt
+[`child_process.spawnSync()`]: ../../doc/api/child_process.md#child_processspawnsynccommand-args-options
 [`hijackstdio.hijackStdErr()`]: #hijackstderrlistener
 [`hijackstdio.hijackStdOut()`]: #hijackstdoutlistener
 [internationalization]: ../../doc/api/intl.md

diff --git a/test/common/child_process.js b/test/common/child_process.js
@@ -1,6 +1,7 @@
 'use strict';
 
 const assert = require('assert');
+const { spawnSync, execFileSync } = require('child_process');
 const common = require('./');
 const util = require('util');
 
@@ -14,14 +15,13 @@ function cleanupStaleProcess(filename) {
   process.once('beforeExit', () => {
     const basename = filename.replace(/.*[/\\]/g, '');
     try {
-      require('child_process')
-        .execFileSync(`${process.env.SystemRoot}\\System32\\wbem\\WMIC.exe`, [
-          'process',
-          'where',
-          `commandline like '%${basename}%child'`,
-          'delete',
-          '/nointeractive',
-        ]);
+      execFileSync(`${process.env.SystemRoot}\\System32\\wbem\\WMIC.exe`, [
+        'process',
+        'where',
+        `commandline like '%${basename}%child'`,
+        'delete',
+        '/nointeractive',
+      ]);
     } catch {
       // Ignore failures, there might not be any stale process to clean up.
     }
@@ -111,11 +111,21 @@ function expectSyncExit(child, {
   return { child, stderr: stderrStr, stdout: stdoutStr };
 }
 
-function expectSyncExitWithoutError(child, options) {
+function spawnSyncAndExit(...args) {
+  const spawnArgs = args.slice(0, args.length - 1);
+  const expectations = args[args.length - 1];
+  const child = spawnSync(...spawnArgs);
+  return expectSyncExit(child, expectations);
+}
+
+function spawnSyncAndExitWithoutError(...args) {
+  const spawnArgs = args.slice(0, args.length);
+  const expectations = args[args.length - 1];
+  const child = spawnSync(...spawnArgs);
   return expectSyncExit(child, {
     status: 0,
     signal: null,
-    ...options,
+    ...expectations,
   });
 }
 
@@ -124,6 +134,6 @@ module.exports = {
   logAfterTime,
   kExpiringChildRunTime,
   kExpiringParentTimer,
-  expectSyncExit,
-  expectSyncExitWithoutError,
+  spawnSyncAndExit,
+  spawnSyncAndExitWithoutError,
 };
diff --git a/test/parallel/test-snapshot-api.js b/test/parallel/test-snapshot-api.js
@@ -4,10 +4,9 @@
 
 require('../common');
 const assert = require('assert');
-const { spawnSync } = require('child_process');
 const tmpdir = require('../common/tmpdir');
 const fixtures = require('../common/fixtures');
-const { expectSyncExitWithoutError } = require('../common/child_process');
+const { spawnSyncAndExitWithoutError } = require('../common/child_process');
 const fs = require('fs');
 
 const v8 = require('v8');
@@ -29,22 +28,20 @@ const entry = fixtures.path('snapshot', 'v8-startup-snapshot-api.js');
     fs.writeFileSync(tmpdir.resolve(book), content, 'utf8');
   }
   fs.copyFileSync(entry, tmpdir.resolve('entry.js'));
-  const child = spawnSync(process.execPath, [
+  spawnSyncAndExitWithoutError(process.execPath, [
     '--snapshot-blob',
     blobPath,
     '--build-snapshot',
     'entry.js',
   ], {
     cwd: tmpdir.path
   });
-
-  expectSyncExitWithoutError(child);
   const stats = fs.statSync(tmpdir.resolve('snapshot.blob'));
   assert(stats.isFile());
 }
 
 {
-  const child = spawnSync(process.execPath, [
+  spawnSyncAndExitWithoutError(process.execPath, [
     '--snapshot-blob',
     blobPath,
     'book1',
@@ -54,9 +51,7 @@ const entry = fixtures.path('snapshot', 'v8-startup-snapshot-api.js');
       ...process.env,
       BOOK_LANG: 'en_US',
     }
-  });
-
-  expectSyncExitWithoutError(child, {
+  }, {
     stderr: 'Reading book1.en_US.txt',
     stdout: 'This is book1.en_US.txt',
     trim: true

diff --git a/test/parallel/test-snapshot-basic.js b/test/parallel/test-snapshot-basic.js
@@ -5,10 +5,12 @@
 
 require('../common');
 const assert = require('assert');
-const { spawnSync } = require('child_process');
 const tmpdir = require('../common/tmpdir');
 const fixtures = require('../common/fixtures');
-const { expectSyncExitWithoutError, expectSyncExit } = require('../common/child_process');
+const {
+  spawnSyncAndExitWithoutError,
+  spawnSyncAndExit,
+} = require('../common/child_process');
 const fs = require('fs');
 
 tmpdir.refresh();
@@ -18,14 +20,12 @@ if (!process.config.variables.node_use_node_snapshot) {
   // Check that Node.js built without an embedded snapshot
   // exits with 9 when node:embedded_snapshot_main is specified
   // as snapshot entry point.
-  const child = spawnSync(process.execPath, [
+  spawnSyncAndExit(process.execPath, [
     '--build-snapshot',
     snapshotScript,
   ], {
     cwd: tmpdir.path
-  });
-
-  expectSyncExit(child, {
+  }, {
     status: 9,
     signal: null,
     stderr: /Node\.js was built without embedded snapshot/
@@ -37,13 +37,12 @@ if (!process.config.variables.node_use_node_snapshot) {
 // By default, the snapshot blob path is cwd/snapshot.blob.
 {
   // Create the snapshot.
-  const child = spawnSync(process.execPath, [
+  spawnSyncAndExitWithoutError(process.execPath, [
     '--build-snapshot',
     snapshotScript,
   ], {
     cwd: tmpdir.path
   });
-  expectSyncExitWithoutError(child);
   const stats = fs.statSync(tmpdir.resolve('snapshot.blob'));
   assert(stats.isFile());
 }
@@ -52,46 +51,41 @@ tmpdir.refresh();
 const blobPath = tmpdir.resolve('my-snapshot.blob');
 {
   // Create the snapshot.
-  const child = spawnSync(process.execPath, [
+  spawnSyncAndExitWithoutError(process.execPath, [
     '--snapshot-blob',
     blobPath,
     '--build-snapshot',
     snapshotScript,
   ], {
     cwd: tmpdir.path
   });
-  expectSyncExitWithoutError(child);
   const stats = fs.statSync(blobPath);
   assert(stats.isFile());
 }
 
 {
   // Check --help.
-  const child = spawnSync(process.execPath, [
+  spawnSyncAndExitWithoutError(process.execPath, [
     '--snapshot-blob',
     blobPath,
     '--help',
   ], {
     cwd: tmpdir.path
+  }, {
+    stdout: /--help/
   });
-  expectSyncExitWithoutError(child);
-
-  assert(child.stdout.toString().includes('--help'));
 }
 
 {
   // Check -c.
-  const child = spawnSync(process.execPath, [
+  spawnSyncAndExitWithoutError(process.execPath, [
     '--snapshot-blob',
     blobPath,
     '-c',
     fixtures.path('snapshot', 'marked.js'),
   ], {
     cwd: tmpdir.path
-  });
-
-  // Check that it is a noop.
-  expectSyncExitWithoutError(child, {
+  }, {
     stderr: '',
     stdout: '',
     trim: true

diff --git a/test/parallel/test-snapshot-warning.js b/test/parallel/test-snapshot-warning.js
@@ -7,10 +7,9 @@
 require('../common');
 
 const assert = require('assert');
-const { spawnSync } = require('child_process');
 const tmpdir = require('../common/tmpdir');
 const fixtures = require('../common/fixtures');
-const { expectSyncExitWithoutError } = require('../common/child_process');
+const { spawnSyncAndExitWithoutError } = require('../common/child_process');
 const fs = require('fs');
 
 const warningScript = fixtures.path('snapshot', 'warning.js');
@@ -20,48 +19,44 @@ const empty = fixtures.path('empty.js');
 tmpdir.refresh();
 {
   console.log('\n# Check snapshot scripts that do not emit warnings.');
-  let child = spawnSync(process.execPath, [
+  spawnSyncAndExitWithoutError(process.execPath, [
     '--snapshot-blob',
     blobPath,
     '--build-snapshot',
     empty,
   ], {
     cwd: tmpdir.path
   });
-  expectSyncExitWithoutError(child);
   const stats = fs.statSync(blobPath);
   assert(stats.isFile());
 
-  child = spawnSync(process.execPath, [
+  spawnSyncAndExitWithoutError(process.execPath, [
     '--snapshot-blob',
     blobPath,
     warningScript,
   ], {
     cwd: tmpdir.path
-  });
-  expectSyncExitWithoutError(child, {
+  }, {
     stderr(output) {
       const match = output.match(/Warning: test warning/g);
       assert.strictEqual(match.length, 1);
       return true;
     }
   });
-
 }
 
 tmpdir.refresh();
 {
   console.log('\n# Check snapshot scripts that emit ' +
               'warnings and --trace-warnings hint.');
-  let child = spawnSync(process.execPath, [
+  spawnSyncAndExitWithoutError(process.execPath, [
     '--snapshot-blob',
     blobPath,
     '--build-snapshot',
     warningScript,
   ], {
     cwd: tmpdir.path
-  });
-  expectSyncExitWithoutError(child, {
+  }, {
     stderr(output) {
       let match = output.match(/Warning: test warning/g);
       assert.strictEqual(match.length, 1);
@@ -73,15 +68,13 @@ tmpdir.refresh();
   const stats = fs.statSync(blobPath);
   assert(stats.isFile());
 
-  child = spawnSync(process.execPath, [
+  spawnSyncAndExitWithoutError(process.execPath, [
     '--snapshot-blob',
     blobPath,
     warningScript,
   ], {
     cwd: tmpdir.path
-  });
-
-  expectSyncExitWithoutError(child, {
+  }, {
     stderr(output) {
       // Warnings should not be handled more than once.
       let match = output.match(/Warning: test warning/g);
@@ -99,7 +92,7 @@ tmpdir.refresh();
   const warningFile1 = tmpdir.resolve('warnings.txt');
   const warningFile2 = tmpdir.resolve('warnings2.txt');
 
-  let child = spawnSync(process.execPath, [
+  spawnSyncAndExitWithoutError(process.execPath, [
     '--snapshot-blob',
     blobPath,
     '--redirect-warnings',
@@ -108,9 +101,7 @@ tmpdir.refresh();
     warningScript,
   ], {
     cwd: tmpdir.path
-  });
-
-  expectSyncExitWithoutError(child, {
+  }, {
     stderr(output) {
       assert.doesNotMatch(output, /Warning: test warning/);
     }
@@ -129,17 +120,15 @@ tmpdir.refresh();
     maxRetries: 3, recursive: false, force: true
   });
 
-  child = spawnSync(process.execPath, [
+  spawnSyncAndExitWithoutError(process.execPath, [
     '--snapshot-blob',
     blobPath,
     '--redirect-warnings',
     warningFile2,
     warningScript,
   ], {
     cwd: tmpdir.path
-  });
-
-  expectSyncExitWithoutError(child, {
+  }, {
     stderr(output) {
       assert.doesNotMatch(output, /Warning: test warning/);
       return true;