Skip to content

Commit

Permalink
test: use mustCall() for simple flow tracking
Browse files Browse the repository at this point in the history
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 <wpreul@gmail.com>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
  • Loading branch information
cjihrig authored and MylesBorins committed Feb 1, 2017
1 parent 79d4986 commit 19907c2
Show file tree
Hide file tree
Showing 212 changed files with 1,095 additions and 2,859 deletions.
15 changes: 4 additions & 11 deletions test/addons/async-hello-world/test.js
Original file line number Diff line number Diff line change
@@ -1,17 +1,10 @@
'use strict';
var common = require('../../common');
const common = require('../../common');
var assert = require('assert');
const binding = require(`./build/${common.buildType}/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() {}));
}));
8 changes: 2 additions & 6 deletions test/internet/test-http-dns-fail.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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);
Expand All @@ -37,6 +34,5 @@ httpreq(0);


process.on('exit', function() {
assert.equal(false, resDespiteError);
assert.equal(2, hadError);
});
19 changes: 4 additions & 15 deletions test/internet/test-http-https-default-ports.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
'use strict';
var common = require('../common');
var assert = require('assert');

if (!common.hasCrypto) {
common.skip('missing crypto');
Expand All @@ -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();
});
}));
23 changes: 4 additions & 19 deletions test/internet/test-net-connect-timeout.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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);
19 changes: 4 additions & 15 deletions test/internet/test-net-connect-unref.js
Original file line number Diff line number Diff line change
@@ -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();
21 changes: 4 additions & 17 deletions test/parallel/test-child-process-buffering.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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;
});
}));
}


Expand All @@ -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);
});
14 changes: 3 additions & 11 deletions test/parallel/test-child-process-disconnect.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
});
}
21 changes: 5 additions & 16 deletions test/parallel/test-child-process-exec-buffer.js
Original file line number Diff line number Diff line change
@@ -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);
});
}));
25 changes: 4 additions & 21 deletions test/parallel/test-child-process-exec-cwd.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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);
}));
11 changes: 2 additions & 9 deletions test/parallel/test-child-process-exec-error.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
19 changes: 4 additions & 15 deletions test/parallel/test-child-process-exit-code.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
}));
Loading

0 comments on commit 19907c2

Please sign in to comment.