test: use spawn and spawnPromisified instead of exec

PR-URL: #48991
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Debadree Chatterjee <[email protected]>

Author: aduh95

test/parallel/test-preload.js

@@ -11,12 +11,8 @@ const childProcess = require('child_process');
 
 const nodeBinary = process.argv[0];
 
-const preloadOption = (preloads) => {
-  let option = '';
-  preloads.forEach(function(preload, index) {
-    option += `-r "${preload}" `;
-  });
-  return option;
+const preloadOption = (...preloads) => {
+  return preloads.flatMap((preload) => ['-r', preload]);
 };
 
 const fixtureA = fixtures.path('printA.js');
@@ -34,75 +30,52 @@ assert(!module.isPreloading);
 
 // Test that module.isPreloading is set in preloaded module
 // Test preloading a single module works
-childProcess.exec(
-  `"${nodeBinary}" ${preloadOption([fixtureIsPreloading])} "${fixtureB}"`,
-  function(err, stdout, stderr) {
-    assert.ifError(err);
-    assert.strictEqual(stdout, 'B\n');
-  });
+common.spawnPromisified(nodeBinary, [...preloadOption(fixtureIsPreloading), fixtureB]).then(common.mustCall(
+  ({ stdout }) => {
+    assert.strictEqual(stdout.trim(), 'B');
+  }
+));
 
 // Test preloading a single module works
-childProcess.exec(`"${nodeBinary}" ${preloadOption([fixtureA])} "${fixtureB}"`,
-                  function(err, stdout, stderr) {
-                    assert.ifError(err);
-                    assert.strictEqual(stdout, 'A\nB\n');
-                  });
+common.spawnPromisified(nodeBinary, [...preloadOption(fixtureA), fixtureB]).then(common.mustCall(
+  ({ stdout }) => {
+    assert.strictEqual(stdout, 'A\nB\n');
+  }));
 
 // Test preloading multiple modules works
-childProcess.exec(
-  `"${nodeBinary}" ${preloadOption([fixtureA, fixtureB])} "${fixtureC}"`,
-  function(err, stdout, stderr) {
-    assert.ifError(err);
+common.spawnPromisified(nodeBinary, [...preloadOption(fixtureA, fixtureB), fixtureC]).then(common.mustCall(
+  ({ stdout }) => {
     assert.strictEqual(stdout, 'A\nB\nC\n');
   }
-);
+));
 
 // Test that preloading a throwing module aborts
-childProcess.exec(
-  `"${nodeBinary}" ${preloadOption([fixtureA, fixtureThrows])} "${fixtureB}"`,
-  function(err, stdout, stderr) {
-    if (err) {
-      assert.strictEqual(stdout, 'A\n');
-    } else {
-      throw new Error('Preload should have failed');
-    }
+common.spawnPromisified(nodeBinary, [...preloadOption(fixtureA, fixtureThrows), fixtureB]).then(common.mustCall(
+  ({ code, stdout }) => {
+    assert.strictEqual(stdout, 'A\n');
+    assert.strictEqual(code, 1);
   }
-);
+));
 
 // Test that preload can be used with --eval
-childProcess.exec(
-  `"${nodeBinary}" ${preloadOption([fixtureA])}-e "console.log('hello');"`,
-  function(err, stdout, stderr) {
-    assert.ifError(err);
+common.spawnPromisified(nodeBinary, [...preloadOption(fixtureA), '-e', 'console.log("hello");']).then(common.mustCall(
+  ({ stdout }) => {
     assert.strictEqual(stdout, 'A\nhello\n');
   }
-);
+));
 
 // Test that preload can be used with --frozen-intrinsics
-childProcess.exec(
-  `"${nodeBinary}" --frozen-intrinsics ${
-    preloadOption([fixtureE])
-  } ${
-    fixtureF
-  }`,
-  function(err, stdout) {
-    assert.ifError(err);
-    assert.strictEqual(stdout, 'smoosh\n');
-  }
-);
-childProcess.exec(
-  `"${
-    nodeBinary
-  }" --frozen-intrinsics ${
-    preloadOption([fixtureE])
-  } ${
-    fixtureG
-  } ${fixtureF}`,
-  function(err, stdout) {
-    assert.ifError(err);
+common.spawnPromisified(nodeBinary, ['--frozen-intrinsics', ...preloadOption(fixtureE), fixtureF]).then(common.mustCall(
+  ({ stdout }) => {
     assert.strictEqual(stdout, 'smoosh\n');
   }
-);
+));
+common.spawnPromisified(nodeBinary, ['--frozen-intrinsics', ...preloadOption(fixtureE), fixtureG, fixtureF])
+  .then(common.mustCall(
+    ({ stdout }) => {
+      assert.strictEqual(stdout, 'smoosh\n');
+    }
+  ));
 
 // Test that preload can be used with stdin
 const stdinProc = childProcess.spawn(
@@ -143,61 +116,62 @@ replProc.on('close', function(code) {
 
 // Test that preload placement at other points in the cmdline
 // also test that duplicated preload only gets loaded once
-childProcess.exec(
-  `"${nodeBinary}" ${preloadOption([fixtureA])}-e "console.log('hello');" ${
-    preloadOption([fixtureA, fixtureB])}`,
-  function(err, stdout, stderr) {
-    assert.ifError(err);
+common.spawnPromisified(nodeBinary, [
+  ...preloadOption(fixtureA),
+  '-e', 'console.log("hello");',
+  ...preloadOption(fixtureA, fixtureB),
+]).then(common.mustCall(
+  ({ stdout }) => {
     assert.strictEqual(stdout, 'A\nB\nhello\n');
   }
-);
+));
 
 // Test that preload works with -i
-const interactive = childProcess.exec(
-  `"${nodeBinary}" ${preloadOption([fixtureD])}-i`,
-  common.mustSucceed((stdout, stderr) => {
-    assert.ok(stdout.endsWith("> 'test'\n> "));
-  })
-);
+const interactive = childProcess.spawn(nodeBinary, [...preloadOption(fixtureD), '-i']);
+
+{
+  const stdout = [];
+  interactive.stdout.on('data', (chunk) => {
+    stdout.push(chunk);
+  });
+  interactive.on('close', common.mustCall(() => {
+    assert.match(Buffer.concat(stdout).toString('utf8'), /> 'test'\r?\n> $/);
+  }));
+}
 
 interactive.stdin.write('a\n');
 interactive.stdin.write('process.exit()\n');
 
-childProcess.exec(
-  `"${nodeBinary}" --require "${fixtures.path('cluster-preload.js')}" "${
-    fixtures.path('cluster-preload-test.js')}"`,
-  function(err, stdout, stderr) {
-    assert.ifError(err);
+common.spawnPromisified(nodeBinary,
+                        ['--require', fixtures.path('cluster-preload.js'), fixtures.path('cluster-preload-test.js')])
+  .then(common.mustCall(({ stdout }) => {
     assert.match(stdout, /worker terminated with code 43/);
-  }
-);
+  }));
 
 // Test that preloading with a relative path works
-childProcess.exec(
-  `"${nodeBinary}" ${preloadOption(['./printA.js'])} "${fixtureB}"`,
-  { cwd: fixtures.fixturesDir },
-  common.mustSucceed((stdout, stderr) => {
+common.spawnPromisified(nodeBinary,
+                        [...preloadOption('./printA.js'), fixtureB],
+                        { cwd: fixtures.fixturesDir }).then(common.mustCall(
+  ({ stdout }) => {
     assert.strictEqual(stdout, 'A\nB\n');
   })
 );
 if (common.isWindows) {
   // https://github.com/nodejs/node/issues/21918
-  childProcess.exec(
-    `"${nodeBinary}" ${preloadOption(['.\\printA.js'])} "${fixtureB}"`,
-    { cwd: fixtures.fixturesDir },
-    common.mustSucceed((stdout, stderr) => {
+  common.spawnPromisified(nodeBinary,
+                          [...preloadOption('.\\printA.js'), fixtureB],
+                          { cwd: fixtures.fixturesDir }).then(common.mustCall(
+    ({ stdout }) => {
       assert.strictEqual(stdout, 'A\nB\n');
     })
   );
 }
 
 // https://github.com/nodejs/node/issues/1691
-childProcess.exec(
-  `"${nodeBinary}" --require ` +
-     `"${fixtures.path('cluster-preload.js')}" cluster-preload-test.js`,
-  { cwd: fixtures.fixturesDir },
-  function(err, stdout, stderr) {
-    assert.ifError(err);
+common.spawnPromisified(nodeBinary,
+                        ['--require', fixtures.path('cluster-preload.js'), 'cluster-preload-test.js'],
+                        { cwd: fixtures.fixturesDir }).then(common.mustCall(
+  ({ stdout }) => {
     assert.match(stdout, /worker terminated with code 43/);
   }
-);
+));

test/parallel/test-timers-immediate-promisified.js

@@ -4,15 +4,13 @@ const common = require('../common');
 const assert = require('assert');
 const timers = require('timers');
 const { promisify } = require('util');
-const child_process = require('child_process');
 
 const { getEventListeners } = require('events');
 const { NodeEventTarget } = require('internal/event_target');
 
 const timerPromises = require('timers/promises');
 
 const setPromiseImmediate = promisify(timers.setImmediate);
-const exec = promisify(child_process.exec);
 
 assert.strictEqual(setPromiseImmediate, timerPromises.setImmediate);
 
@@ -91,9 +89,9 @@ process.on('multipleResolves', common.mustNotCall());
 }
 
 {
-  exec(`${process.execPath} -pe "const assert = require('assert');` +
+  common.spawnPromisified(process.execPath, ['-pe', "const assert = require('assert');" +
     'require(\'timers/promises\').setImmediate(null, { ref: false }).' +
-    'then(assert.fail)"').then(common.mustCall(({ stderr }) => {
+    'then(assert.fail)']).then(common.mustCall(({ stderr }) => {
     assert.strictEqual(stderr, '');
   }));
 }

test/parallel/test-timers-interval-promisified.js

@@ -4,15 +4,13 @@ const common = require('../common');
 const assert = require('assert');
 const timers = require('timers');
 const { promisify } = require('util');
-const child_process = require('child_process');
 
 const { getEventListeners } = require('events');
 const { NodeEventTarget } = require('internal/event_target');
 
 const timerPromises = require('timers/promises');
 
 const setPromiseTimeout = promisify(timers.setTimeout);
-const exec = promisify(child_process.exec);
 
 const { setInterval } = timerPromises;
 
@@ -154,11 +152,11 @@ process.on('multipleResolves', common.mustNotCall());
 }
 
 {
-  exec(`${process.execPath} -pe "const assert = require('assert');` +
+  common.spawnPromisified(process.execPath, ['-pe', "const assert = require('assert');" +
     'const interval = require(\'timers/promises\')' +
     '.setInterval(1000, null, { ref: false });' +
     'interval[Symbol.asyncIterator]().next()' +
-    '.then(assert.fail)"').then(common.mustCall(({ stderr }) => {
+    '.then(assert.fail)']).then(common.mustCall(({ stderr }) => {
     assert.strictEqual(stderr, '');
   }));
 }

test/parallel/test-timers-timeout-promisified.js

@@ -4,15 +4,13 @@ const common = require('../common');
 const assert = require('assert');
 const timers = require('timers');
 const { promisify } = require('util');
-const child_process = require('child_process');
 
 const { getEventListeners } = require('events');
 const { NodeEventTarget } = require('internal/event_target');
 
 const timerPromises = require('timers/promises');
 
 const setPromiseTimeout = promisify(timers.setTimeout);
-const exec = promisify(child_process.exec);
 
 assert.strictEqual(setPromiseTimeout, timerPromises.setTimeout);
 
@@ -91,9 +89,9 @@ process.on('multipleResolves', common.mustNotCall());
 }
 
 {
-  exec(`${process.execPath} -pe "const assert = require('assert');` +
+  common.spawnPromisified(process.execPath, ['-pe', 'const assert = require(\'assert\');' +
     'require(\'timers/promises\').setTimeout(1000, null, { ref: false }).' +
-    'then(assert.fail)"').then(common.mustCall(({ stderr }) => {
+    'then(assert.fail)']).then(common.mustCall(({ stderr }) => {
     assert.strictEqual(stderr, '');
   }));
 }

test/parallel/test-url-parse-invalid-input.js

@@ -1,7 +1,6 @@
 'use strict';
 const common = require('../common');
 const assert = require('assert');
-const childProcess = require('child_process');
 const url = require('url');
 
 // https://github.com/joyent/node/issues/568
@@ -82,13 +81,12 @@ if (common.hasIntl) {
     'git+ssh://[email protected]:npm/npm',
   ];
   badURLs.forEach((badURL) => {
-    childProcess.exec(`${process.execPath} -e "url.parse('${badURL}')"`,
-                      common.mustCall((err, stdout, stderr) => {
-                        assert.strictEqual(err, null);
-                        assert.strictEqual(stdout, '');
-                        assert.match(stderr, /\[DEP0170\] DeprecationWarning:/);
-                      })
-    );
+    common.spawnPromisified(process.execPath, ['-e', `url.parse(${JSON.stringify(badURL)})`])
+      .then(common.mustCall(({ code, stdout, stderr }) => {
+        assert.strictEqual(code, 0);
+        assert.strictEqual(stdout, '');
+        assert.match(stderr, /\[DEP0170\] DeprecationWarning:/);
+      }));
   });
 
   // Warning should only happen once per process.

test/sequential/test-cli-syntax-require.js

@@ -2,7 +2,7 @@
 
 const common = require('../common');
 const assert = require('assert');
-const { exec } = require('child_process');
+const { spawn } = require('child_process');
 const fixtures = require('../common/fixtures');
 
 const node = process.execPath;
@@ -17,20 +17,25 @@ const syntaxErrorRE = /^SyntaxError: \b/m;
     const preloadFile = fixtures.path('no-wrapper.js');
     const file = fixtures.path('syntax', 'illegal_if_not_wrapped.js');
     const args = [requireFlag, preloadFile, checkFlag, file];
-    const cmd = [node, ...args].join(' ');
-    exec(cmd, common.mustCall((err, stdout, stderr) => {
-      assert.strictEqual(err instanceof Error, true);
-      assert.strictEqual(err.code, 1,
-                         `code ${err.code} !== 1 for error:\n\n${err}`);
+    const cp = spawn(node, args);
 
-      // No stdout should be produced
-      assert.strictEqual(stdout, '');
+    // No stdout should be produced
+    cp.stdout.on('data', common.mustNotCall('stdout data event'));
 
+    const stderrBuffer = [];
+    cp.stderr.on('data', (chunk) => stderrBuffer.push(chunk));
+
+    cp.on('exit', common.mustCall((code, signal) => {
+      assert.strictEqual(code, 1);
+      assert.strictEqual(signal, null);
+
+      const stderr = Buffer.concat(stderrBuffer).toString('utf-8');
       // stderr should have a syntax error message
       assert.match(stderr, syntaxErrorRE);
 
       // stderr should include the filename
       assert(stderr.startsWith(file), `${stderr} starts with ${file}`);
     }));
+
   });
 });