test: refactoring / cleanup on child-process tests

PR-URL: #32078 Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
nodejs · Mar 25, 2020 · 07e4131 · 07e4131
1 parent dfcc3e8
commit 07e4131
Show file tree

Hide file tree

Showing 17 changed files with 309 additions and 261 deletions.
diff --git a/test/fixtures/child-process-spawn-node.js b/test/fixtures/child-process-spawn-node.js
@@ -1,7 +1,8 @@
 const assert = require('assert');
+const debug = require('util').debuglog('test');
 
 function onmessage(m) {
-  console.log('CHILD got message:', m);
+  debug('CHILD got message:', m);
   assert.ok(m.hello);
   process.removeListener('message', onmessage);
 }

diff --git a/test/parallel/test-bash-completion.js b/test/parallel/test-bash-completion.js
@@ -2,14 +2,15 @@
 require('../common');
 const assert = require('assert');
 const child_process = require('child_process');
-const { inspect } = require('util');
+const { debuglog, inspect } = require('util');
+const debug = debuglog('test');
 
 const p = child_process.spawnSync(
   process.execPath, [ '--completion-bash' ]);
 assert.ifError(p.error);
 
 const output = p.stdout.toString().trim().replace(/\r/g, '');
-console.log(output);
+debug(output);
 
 const prefix = `_node_complete() {
   local cur_word options

diff --git a/test/parallel/test-child-process-default-options.js b/test/parallel/test-child-process-default-options.js
@@ -20,15 +20,16 @@
 // USE OR OTHER DEALINGS IN THE SOFTWARE.
 
 'use strict';
-const common = require('../common');
+const { isWindows } = require('../common');
 const assert = require('assert');
 
 const spawn = require('child_process').spawn;
+const debug = require('util').debuglog('test');
 
 process.env.HELLO = 'WORLD';
 
 let child;
-if (common.isWindows) {
+if (isWindows) {
   child = spawn('cmd.exe', ['/c', 'set'], {});
 } else {
   child = spawn('/usr/bin/env', [], {});
@@ -39,7 +40,7 @@ let response = '';
 child.stdout.setEncoding('utf8');
 
 child.stdout.on('data', function(chunk) {
-  console.log(`stdout: ${chunk}`);
+  debug(`stdout: ${chunk}`);
   response += chunk;
 });
 

diff --git a/test/parallel/test-child-process-double-pipe.js b/test/parallel/test-child-process-double-pipe.js
@@ -20,17 +20,22 @@
 // USE OR OTHER DEALINGS IN THE SOFTWARE.
 
 'use strict';
-const common = require('../common');
+const {
+  isWindows,
+  mustCall,
+  mustCallAtLeast,
+} = require('../common');
 const assert = require('assert');
 const os = require('os');
 const spawn = require('child_process').spawn;
+const debug = require('util').debuglog('test');
 
 // We're trying to reproduce:
 // $ echo "hello\nnode\nand\nworld" | grep o | sed s/o/a/
 
 let grep, sed, echo;
 
-if (common.isWindows) {
+if (isWindows) {
   grep = spawn('grep', ['--binary', 'o']);
   sed = spawn('sed', ['--binary', 's/o/O/']);
   echo = spawn('cmd.exe',
@@ -54,62 +59,66 @@ if (common.isWindows) {
 
 
 // pipe echo | grep
-echo.stdout.on('data', function(data) {
-  console.error(`grep stdin write ${data.length}`);
+echo.stdout.on('data', mustCallAtLeast((data) => {
+  debug(`grep stdin write ${data.length}`);
   if (!grep.stdin.write(data)) {
     echo.stdout.pause();
   }
-});
+}));
 
-grep.stdin.on('drain', function(data) {
+// TODO(@jasnell): This does not appear to ever be
+// emitted. It's not clear if it is necessary.
+grep.stdin.on('drain', (data) => {
   echo.stdout.resume();
 });
 
 // Propagate end from echo to grep
-echo.stdout.on('end', function(code) {
+echo.stdout.on('end', mustCall((code) => {
   grep.stdin.end();
-});
+}));
 
-echo.on('exit', function() {
-  console.error('echo exit');
-});
+echo.on('exit', mustCall(() => {
+  debug('echo exit');
+}));
 
-grep.on('exit', function() {
-  console.error('grep exit');
-});
+grep.on('exit', mustCall(() => {
+  debug('grep exit');
+}));
 
-sed.on('exit', function() {
-  console.error('sed exit');
-});
+sed.on('exit', mustCall(() => {
+  debug('sed exit');
+}));
 
 
 // pipe grep | sed
-grep.stdout.on('data', function(data) {
-  console.error(`grep stdout ${data.length}`);
+grep.stdout.on('data', mustCallAtLeast((data) => {
+  debug(`grep stdout ${data.length}`);
   if (!sed.stdin.write(data)) {
     grep.stdout.pause();
   }
-});
+}));
 
-sed.stdin.on('drain', function(data) {
+// TODO(@jasnell): This does not appear to ever be
+// emitted. It's not clear if it is necessary.
+sed.stdin.on('drain', (data) => {
   grep.stdout.resume();
 });
 
 // Propagate end from grep to sed
-grep.stdout.on('end', function(code) {
-  console.error('grep stdout end');
+grep.stdout.on('end', mustCall((code) => {
+  debug('grep stdout end');
   sed.stdin.end();
-});
+}));
 
 
 let result = '';
 
 // print sed's output
-sed.stdout.on('data', function(data) {
+sed.stdout.on('data', mustCallAtLeast((data) => {
   result += data.toString('utf8', 0, data.length);
-  console.log(data);
-});
+  debug(data);
+}));
 
-sed.stdout.on('end', function(code) {
+sed.stdout.on('end', mustCall((code) => {
   assert.strictEqual(result, `hellO${os.EOL}nOde${os.EOL}wOrld${os.EOL}`);
-});
+}));
diff --git a/test/parallel/test-child-process-env.js b/test/parallel/test-child-process-env.js
@@ -20,9 +20,14 @@
 // USE OR OTHER DEALINGS IN THE SOFTWARE.
 
 'use strict';
-const common = require('../common');
+const {
+  isWindows,
+  mustCall,
+  mustCallAtLeast,
+} = require('../common');
 const assert = require('assert');
 const os = require('os');
+const debug = require('util').debuglog('test');
 
 const spawn = require('child_process').spawn;
 
@@ -38,7 +43,7 @@ Object.setPrototypeOf(env, {
 });
 
 let child;
-if (common.isWindows) {
+if (isWindows) {
   child = spawn('cmd.exe', ['/c', 'set'], { env });
 } else {
   child = spawn('/usr/bin/env', [], { env });
@@ -49,15 +54,15 @@ let response = '';
 
 child.stdout.setEncoding('utf8');
 
-child.stdout.on('data', (chunk) => {
-  console.log(`stdout: ${chunk}`);
+child.stdout.on('data', mustCallAtLeast((chunk) => {
+  debug(`stdout: ${chunk}`);
   response += chunk;
-});
+}));
 
-process.on('exit', () => {
+child.stdout.on('end', mustCall(() => {
   assert.ok(response.includes('HELLO=WORLD'));
   assert.ok(response.includes('FOO=BAR'));
   assert.ok(!response.includes('UNDEFINED=undefined'));
   assert.ok(response.includes('NULL=null'));
   assert.ok(response.includes(`EMPTY=${os.EOL}`));
-});
+}));
diff --git a/test/parallel/test-child-process-exec-env.js b/test/parallel/test-child-process-exec-env.js
@@ -20,9 +20,11 @@
 // USE OR OTHER DEALINGS IN THE SOFTWARE.
 
 'use strict';
-const common = require('../common');
+const { isWindows } = require('../common');
 const assert = require('assert');
 const exec = require('child_process').exec;
+const debug = require('util').debuglog('test');
+
 let success_count = 0;
 let error_count = 0;
 let response = '';
@@ -31,17 +33,17 @@ let child;
 function after(err, stdout, stderr) {
   if (err) {
     error_count++;
-    console.log(`error!: ${err.code}`);
-    console.log(`stdout: ${JSON.stringify(stdout)}`);
-    console.log(`stderr: ${JSON.stringify(stderr)}`);
+    debug(`error!: ${err.code}`);
+    debug(`stdout: ${JSON.stringify(stdout)}`);
+    debug(`stderr: ${JSON.stringify(stderr)}`);
     assert.strictEqual(err.killed, false);
   } else {
     success_count++;
     assert.notStrictEqual(stdout, '');
   }
 }
 
-if (!common.isWindows) {
+if (!isWindows) {
   child = exec('/usr/bin/env', { env: { 'HELLO': 'WORLD' } }, after);
 } else {
   child = exec('set',
@@ -55,7 +57,7 @@ child.stdout.on('data', function(chunk) {
 });
 
 process.on('exit', function() {
-  console.log('response: ', response);
+  debug('response: ', response);
   assert.strictEqual(success_count, 1);
   assert.strictEqual(error_count, 0);
   assert.ok(response.includes('HELLO=WORLD'));

diff --git a/test/parallel/test-child-process-fork-getconnections.js b/test/parallel/test-child-process-fork-getconnections.js
@@ -20,7 +20,7 @@
 // USE OR OTHER DEALINGS IN THE SOFTWARE.
 
 'use strict';
-require('../common');
+const common = require('../common');
 const assert = require('assert');
 const fork = require('child_process').fork;
 const net = require('net');
@@ -29,7 +29,7 @@ const count = 12;
 if (process.argv[2] === 'child') {
   const sockets = [];
 
-  process.on('message', function(m, socket) {
+  process.on('message', common.mustCall((m, socket) => {
     function sendClosed(id) {
       process.send({ id: id, status: 'closed' });
     }
@@ -52,41 +52,39 @@ if (process.argv[2] === 'child') {
         sockets[m.id].destroy();
       }
     }
-  });
+  }));
 
 } else {
   const child = fork(process.argv[1], ['child']);
 
-  child.on('exit', function(code, signal) {
+  child.on('exit', common.mustCall((code, signal) => {
     if (!subprocessKilled) {
       assert.fail('subprocess died unexpectedly! ' +
                   `code: ${code} signal: ${signal}`);
     }
-  });
+  }));
 
   const server = net.createServer();
   const sockets = [];
-  let sent = 0;
 
-  server.on('connection', function(socket) {
+  server.on('connection', common.mustCall((socket) => {
     child.send({ cmd: 'new' }, socket);
     sockets.push(socket);
 
     if (sockets.length === count) {
       closeSockets(0);
     }
-  });
+  }, count));
 
-  let disconnected = 0;
-  server.on('listening', function() {
+  const onClose = common.mustCall(count);
+
+  server.on('listening', common.mustCall(() => {
     let j = count;
     while (j--) {
       const client = net.connect(server.address().port, '127.0.0.1');
-      client.on('close', function() {
-        disconnected += 1;
-      });
+      client.on('close', onClose);
     }
-  });
+  }));
 
   let subprocessKilled = false;
   function closeSockets(i) {
@@ -97,29 +95,18 @@ if (process.argv[2] === 'child') {
       return;
     }
 
-    child.once('message', function(m) {
+    child.once('message', common.mustCall((m) => {
       assert.strictEqual(m.status, 'closed');
-      server.getConnections(function(err, num) {
+      server.getConnections(common.mustCall((err, num) => {
         assert.ifError(err);
         assert.strictEqual(num, count - (i + 1));
         closeSockets(i + 1);
-      });
-    });
-    sent++;
+      }));
+    }));
     child.send({ id: i, cmd: 'close' });
   }
 
-  let closeEmitted = false;
-  server.on('close', function() {
-    closeEmitted = true;
-  });
+  server.on('close', common.mustCall());
 
   server.listen(0, '127.0.0.1');
-
-  process.on('exit', function() {
-    assert.strictEqual(sent, count);
-    assert.strictEqual(disconnected, count);
-    assert.ok(closeEmitted);
-    console.log('ok');
-  });
 }