test: use mustCall() for simple flow tracking

Many of the tests use variables to track when callback functions
are invoked or events are emitted. These variables are then
asserted on process exit. This commit replaces this pattern in
straightforward cases with common.mustCall(). This makes the
tests easier to reason about, leads to a net reduction in lines
of code, and uncovered a few bugs in tests. This commit also
replaces some callbacks that should never be called with
common.fail().

PR-URL: #7753
Reviewed-By: Wyatt Preul <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>

Conflicts:
	test/parallel/test-file-read-noexist.js

Author: cjihrig

test/addons/async-hello-world/test.js

@@ -1,17 +1,10 @@
 'use strict';
-require('../../common');
+const common = require('../../common');
 var assert = require('assert');
 var binding = require('./build/Release/binding');
-var called = false;
 
-process.on('exit', function() {
-  assert(called);
-});
-
-binding(5, function(err, val) {
+binding(5, common.mustCall(function(err, val) {
   assert.equal(null, err);
   assert.equal(10, val);
-  process.nextTick(function() {
-    called = true;
-  });
-});
+  process.nextTick(common.mustCall(function() {}));
+}));

test/internet/test-http-dns-fail.js

@@ -4,11 +4,10 @@
  * should trigger the error event after each attempt.
  */
 
-require('../common');
+const common = require('../common');
 var assert = require('assert');
 var http = require('http');
 
-var resDespiteError = false;
 var hadError = 0;
 
 function httpreq(count) {
@@ -19,9 +18,7 @@ function httpreq(count) {
     port: 80,
     path: '/',
     method: 'GET'
-  }, function(res) {
-    resDespiteError = true;
-  });
+  }, common.fail);
 
   req.on('error', function(e) {
     console.log(e.message);
@@ -37,6 +34,5 @@ httpreq(0);
 
 
 process.on('exit', function() {
-  assert.equal(false, resDespiteError);
   assert.equal(2, hadError);
 });

test/internet/test-http-https-default-ports.js

@@ -1,6 +1,5 @@
 'use strict';
 var common = require('../common');
-var assert = require('assert');
 
 if (!common.hasCrypto) {
   common.skip('missing crypto');
@@ -9,21 +8,11 @@ if (!common.hasCrypto) {
 var https = require('https');
 
 var http = require('http');
-var gotHttpsResp = false;
-var gotHttpResp = false;
 
-process.on('exit', function() {
-  assert(gotHttpsResp);
-  assert(gotHttpResp);
-  console.log('ok');
-});
-
-https.get('https://www.google.com/', function(res) {
-  gotHttpsResp = true;
+https.get('https://www.google.com/', common.mustCall(function(res) {
   res.resume();
-});
+}));
 
-http.get('http://www.google.com/', function(res) {
-  gotHttpResp = true;
+http.get('http://www.google.com/', common.mustCall(function(res) {
   res.resume();
-});
+}));

test/internet/test-net-connect-timeout.js

@@ -3,16 +3,12 @@
 // https://groups.google.com/forum/#!topic/nodejs/UE0ZbfLt6t8
 // https://groups.google.com/forum/#!topic/nodejs-dev/jR7-5UDqXkw
 
-require('../common');
+const common = require('../common');
 var net = require('net');
 var assert = require('assert');
 
 var start = new Date();
 
-var gotTimeout = false;
-
-var gotConnect = false;
-
 var T = 100;
 
 // 192.0.2.1 is part of subnet assigned as "TEST-NET" in RFC 5737.
@@ -23,22 +19,11 @@ var socket = net.createConnection(9999, '192.0.2.1');
 
 socket.setTimeout(T);
 
-socket.on('timeout', function() {
+socket.on('timeout', common.mustCall(function() {
   console.error('timeout');
-  gotTimeout = true;
   var now = new Date();
   assert.ok(now - start < T + 500);
   socket.destroy();
-});
-
-socket.on('connect', function() {
-  console.error('connect');
-  gotConnect = true;
-  socket.destroy();
-});
-
+}));
 
-process.on('exit', function() {
-  assert.ok(gotTimeout);
-  assert.ok(!gotConnect);
-});
+socket.on('connect', common.fail);

test/internet/test-net-connect-unref.js

@@ -1,25 +1,14 @@
 'use strict';
-require('../common');
-var assert = require('assert');
+const common = require('../common');
 var net = require('net');
 
-var client, killed = false, ended = false;
+var client;
 var TIMEOUT = 10 * 1000;
 
 client = net.createConnection(53, '8.8.8.8', function() {
   client.unref();
 });
 
-client.on('close', function() {
-  ended = true;
-});
-
-setTimeout(function() {
-  killed = true;
-  client.end();
-}, TIMEOUT).unref();
+client.on('close', common.fail);
 
-process.on('exit', function() {
-  assert.strictEqual(killed, false, 'A client should have connected');
-  assert.strictEqual(ended, false, 'A client should stay connected');
-});
+setTimeout(common.fail, TIMEOUT).unref();

test/parallel/test-child-process-buffering.js

@@ -2,10 +2,6 @@
 var common = require('../common');
 var assert = require('assert');
 
-var pwd_called = false;
-var childClosed = false;
-var childExited = false;
-
 function pwd(callback) {
   var output = '';
   var child = common.spawnPwd();
@@ -16,17 +12,14 @@ function pwd(callback) {
     output += s;
   });
 
-  child.on('exit', function(c) {
+  child.on('exit', common.mustCall(function(c) {
     console.log('exit: ' + c);
     assert.equal(0, c);
-    childExited = true;
-  });
+  }));
 
-  child.on('close', function() {
+  child.on('close', common.mustCall(function() {
     callback(output);
-    pwd_called = true;
-    childClosed = true;
-  });
+  }));
 }
 
 
@@ -35,9 +28,3 @@ pwd(function(result) {
   assert.equal(true, result.length > 1);
   assert.equal('\n', result[result.length - 1]);
 });
-
-process.on('exit', function() {
-  assert.equal(true, pwd_called);
-  assert.equal(true, childExited);
-  assert.equal(true, childClosed);
-});

test/parallel/test-child-process-disconnect.js

@@ -48,21 +48,16 @@ if (process.argv[2] === 'child') {
   var child = fork(process.argv[1], ['child']);
 
   var childFlag = false;
-  var childSelfTerminate = false;
-  var parentEmit = false;
   var parentFlag = false;
 
   // when calling .disconnect the event should emit
   // and the disconnected flag should be true.
-  child.on('disconnect', function() {
-    parentEmit = true;
+  child.on('disconnect', common.mustCall(function() {
     parentFlag = child.connected;
-  });
+  }));
 
   // the process should also self terminate without using signals
-  child.on('exit', function() {
-    childSelfTerminate = true;
-  });
+  child.on('exit', common.mustCall(function() {}));
 
   // when child is listening
   child.on('message', function(obj) {
@@ -91,8 +86,5 @@ if (process.argv[2] === 'child') {
   process.on('exit', function() {
     assert.equal(childFlag, false);
     assert.equal(parentFlag, false);
-
-    assert.ok(childSelfTerminate);
-    assert.ok(parentEmit);
   });
 }

test/parallel/test-child-process-exec-buffer.js

@@ -1,33 +1,22 @@
 'use strict';
-require('../common');
+const common = require('../common');
 var assert = require('assert');
 var exec = require('child_process').exec;
 var os = require('os');
-
-var success_count = 0;
-
 var str = 'hello';
 
 // default encoding
-exec('echo ' + str, function(err, stdout, stderr) {
+exec('echo ' + str, common.mustCall(function(err, stdout, stderr) {
   assert.ok('string', typeof stdout, 'Expected stdout to be a string');
   assert.ok('string', typeof stderr, 'Expected stderr to be a string');
   assert.equal(str + os.EOL, stdout);
-
-  success_count++;
-});
+}));
 
 // no encoding (Buffers expected)
 exec('echo ' + str, {
   encoding: null
-}, function(err, stdout, stderr) {
+}, common.mustCall(function(err, stdout, stderr) {
   assert.ok(stdout instanceof Buffer, 'Expected stdout to be a Buffer');
   assert.ok(stderr instanceof Buffer, 'Expected stderr to be a Buffer');
   assert.equal(str + os.EOL, stdout.toString());
-
-  success_count++;
-});
-
-process.on('exit', function() {
-  assert.equal(2, success_count);
-});
+}));

test/parallel/test-child-process-exec-cwd.js

@@ -3,9 +3,6 @@ const common = require('../common');
 var assert = require('assert');
 var exec = require('child_process').exec;
 
-var success_count = 0;
-var error_count = 0;
-
 var pwdcommand, dir;
 
 if (common.isWindows) {
@@ -16,21 +13,7 @@ if (common.isWindows) {
   dir = '/dev';
 }
 
-exec(pwdcommand, {cwd: dir}, function(err, stdout, stderr) {
-  if (err) {
-    error_count++;
-    console.log('error!: ' + err.code);
-    console.log('stdout: ' + JSON.stringify(stdout));
-    console.log('stderr: ' + JSON.stringify(stderr));
-    assert.equal(false, err.killed);
-  } else {
-    success_count++;
-    console.log(stdout);
-    assert.ok(stdout.indexOf(dir) == 0);
-  }
-});
-
-process.on('exit', function() {
-  assert.equal(1, success_count);
-  assert.equal(0, error_count);
-});
+exec(pwdcommand, {cwd: dir}, common.mustCall(function(err, stdout, stderr) {
+  assert.ifError(err);
+  assert.ok(stdout.indexOf(dir) == 0);
+}));

test/parallel/test-child-process-exec-error.js

@@ -4,17 +4,10 @@ var assert = require('assert');
 var child_process = require('child_process');
 
 function test(fun, code) {
-  var errors = 0;
-
-  fun('does-not-exist', function(err) {
+  fun('does-not-exist', common.mustCall(function(err) {
     assert.equal(err.code, code);
     assert(/does\-not\-exist/.test(err.cmd));
-    errors++;
-  });
-
-  process.on('exit', function() {
-    assert.equal(errors, 1);
-  });
+  }));
 }
 
 if (common.isWindows) {

test/parallel/test-child-process-exit-code.js

@@ -4,29 +4,18 @@ var assert = require('assert');
 var spawn = require('child_process').spawn;
 var path = require('path');
 
-var exits = 0;
-
 var exitScript = path.join(common.fixturesDir, 'exit.js');
 var exitChild = spawn(process.argv[0], [exitScript, 23]);
-exitChild.on('exit', function(code, signal) {
+exitChild.on('exit', common.mustCall(function(code, signal) {
   assert.strictEqual(code, 23);
   assert.strictEqual(signal, null);
-
-  exits++;
-});
+}));
 
 
 var errorScript = path.join(common.fixturesDir,
                             'child_process_should_emit_error.js');
 var errorChild = spawn(process.argv[0], [errorScript]);
-errorChild.on('exit', function(code, signal) {
+errorChild.on('exit', common.mustCall(function(code, signal) {
   assert.ok(code !== 0);
   assert.strictEqual(signal, null);
-
-  exits++;
-});
-
-
-process.on('exit', function() {
-  assert.equal(2, exits);
-});
+}));