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]>

Author: cjihrig

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

@@ -1,17 +1,10 @@
 'use strict';
-varcommon=require('../../common');
+constcommon=require('../../common');
 varassert=require('assert');
 constbinding=require(`./build/${common.buildType}/binding`);
-varcalled=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');
+constcommon=require('../common');
 varassert=require('assert');
 varhttp=require('http');
 
-varresDespiteError=false;
 varhadError=0;
 
 functionhttpreq(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';
 varcommon=require('../common');
-varassert=require('assert');
 
 if(!common.hasCrypto){
 common.skip('missing crypto');
@@ -9,21 +8,11 @@ if (!common.hasCrypto){
 varhttps=require('https');
 
 varhttp=require('http');
-vargotHttpsResp=false;
-vargotHttpResp=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');
+constcommon=require('../common');
 varnet=require('net');
 varassert=require('assert');
 
 varstart=newDate();
 
-vargotTimeout=false;
-
-vargotConnect=false;
-
 varT=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;
 varnow=newDate();
 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');
-varassert=require('assert');
+constcommon=require('../common');
 varnet=require('net');
 
-varclient,killed=false,ended=false;
+varclient;
 varTIMEOUT=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 @@
 varcommon=require('../common');
 varassert=require('assert');
 
-varpwd_called=false;
-varchildClosed=false;
-varchildExited=false;
-
 functionpwd(callback){
 varoutput='';
 varchild=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'){
 varchild=fork(process.argv[1],['child']);
 
 varchildFlag=false;
-varchildSelfTerminate=false;
-varparentEmit=false;
 varparentFlag=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');
+constcommon=require('../common');
 varassert=require('assert');
 varexec=require('child_process').exec;
 varos=require('os');
-
-varsuccess_count=0;
-
 varstr='hello';
 
 // default encoding
-exec('echo '+str,function(err,stdout,stderr){
+exec('echo '+str,common.mustCall(function(err,stdout,stderr){
 assert.ok('string',typeofstdout,'Expected stdout to be a string');
 assert.ok('string',typeofstderr,'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(stdoutinstanceofBuffer,'Expected stdout to be a Buffer');
 assert.ok(stderrinstanceofBuffer,'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');
 varassert=require('assert');
 varexec=require('child_process').exec;
 
-varsuccess_count=0;
-varerror_count=0;
-
 varpwdcommand,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');
 varchild_process=require('child_process');
 
 functiontest(fun,code){
-varerrors=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){