Skip to content

Commit

Permalink
test: wrap assert.fail when passed to callback
Browse files Browse the repository at this point in the history
Currently there are many instances where assert.fail is directly passed
to a callback for error handling. Unfortunately this will swallow the
error as it is the third argument of assert.fail that sets the message
not the first.

This commit adds a new function to test/common.js that simply wraps
assert.fail and calls it with the provided message.

Tip of the hat to @Trott for pointing me in the direction of this.

PR-URL: #3453
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
  • Loading branch information
Myles Borins authored and rvagg committed Oct 26, 2015
1 parent 7added3 commit 60de9f8
Show file tree
Hide file tree
Showing 37 changed files with 81 additions and 77 deletions.
4 changes: 4 additions & 0 deletions test/common.js
Original file line number Diff line number Diff line change
Expand Up @@ -442,3 +442,7 @@ exports.fileExists = function(pathname) {
return false;
}
};

exports.fail = function(msg) {
assert.fail(null, null, msg);
};
2 changes: 1 addition & 1 deletion test/parallel/test-child-process-recv-handle.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ function master() {
});
proc.stdout.on('data', function(data) {
assert.equal(data, 'ok\r\n');
net.createServer(assert.fail).listen(common.PORT, function() {
net.createServer(common.fail).listen(common.PORT, function() {
handle = this._handle;
proc.send('one');
proc.send('two', handle);
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-child-process-spawn-typeerror.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const empty = common.fixturesDir + '/empty.js';

assert.throws(function() {
var child = spawn(invalidcmd, 'this is not an array');
child.on('error', assert.fail);
child.on('error', common.fail);
}, TypeError);

// verify that valid argument combinations do not throw
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-cluster-bind-privileged-port.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ if (cluster.isMaster) {
}));
}
else {
var s = net.createServer(assert.fail);
s.listen(42, assert.fail.bind(null, 'listen should have failed'));
var s = net.createServer(common.fail);
s.listen(42, common.fail.bind(null, 'listen should have failed'));
s.on('error', common.mustCall(function(err) {
assert.equal(err.code, 'EACCES');
process.disconnect();
Expand Down
6 changes: 3 additions & 3 deletions test/parallel/test-cluster-bind-twice.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ if (!id) {
else if (id === 'one') {
if (cluster.isMaster) return startWorker();

var server = http.createServer(assert.fail).listen(common.PORT, function() {
var server = http.createServer(common.fail).listen(common.PORT, function() {
process.send('READY');
});

Expand All @@ -84,12 +84,12 @@ else if (id === 'two') {
assert(ok);
});

var server = http.createServer(assert.fail);
var server = http.createServer(common.fail);
process.on('message', function(m) {
if (typeof m === 'object') return; // ignore system messages
if (m === 'QUIT') process.exit();
assert.equal(m, 'START');
server.listen(common.PORT, assert.fail);
server.listen(common.PORT, common.fail);
server.on('error', function(e) {
assert.equal(e.code, 'EADDRINUSE');
process.send(e.code);
Expand Down
8 changes: 4 additions & 4 deletions test/parallel/test-cluster-eaddrinuse.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ var net = require('net');
var id = '' + process.argv[2];

if (id === 'undefined') {
var server = net.createServer(assert.fail);
var server = net.createServer(common.fail);
server.listen(common.PORT, function() {
var worker = fork(__filename, ['worker']);
worker.on('message', function(msg) {
Expand All @@ -24,14 +24,14 @@ if (id === 'undefined') {
});
}
else if (id === 'worker') {
var server = net.createServer(assert.fail);
server.listen(common.PORT, assert.fail);
var server = net.createServer(common.fail);
server.listen(common.PORT, common.fail);
server.on('error', common.mustCall(function(e) {
assert(e.code, 'EADDRINUSE');
process.send('stop-listening');
process.once('message', function(msg) {
if (msg !== 'stopped-listening') return;
server = net.createServer(assert.fail);
server = net.createServer(common.fail);
server.listen(common.PORT, common.mustCall(function() {
server.close();
}));
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-cluster-net-listen.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,5 @@ if (cluster.isMaster) {
}
else {
// listen() without port should not trigger a libuv assert
net.createServer(assert.fail).listen(process.exit);
net.createServer(common.fail).listen(process.exit);
}
2 changes: 1 addition & 1 deletion test/parallel/test-cluster-rr-ref.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ if (cluster.isMaster) {
if (msg === 'done') this.kill();
});
} else {
const server = net.createServer(assert.fail);
const server = net.createServer(common.fail);
server.listen(common.PORT, function() {
server.unref();
server.ref();
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-cluster-setup-master-argv.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ var common = require('../common');
var assert = require('assert');
var cluster = require('cluster');

setTimeout(assert.fail.bind(assert, 'setup not emitted'), 1000).unref();
setTimeout(common.fail.bind(assert, 'setup not emitted'), 1000).unref();

cluster.on('setup', function() {
var clusterArgs = cluster.settings.args;
Expand Down
6 changes: 3 additions & 3 deletions test/parallel/test-cluster-shared-handle-bind-error.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ if (cluster.isMaster) {
// Master opens and binds the socket and shares it with the worker.
cluster.schedulingPolicy = cluster.SCHED_NONE;
// Hog the TCP port so that when the worker tries to bind, it'll fail.
net.createServer(assert.fail).listen(common.PORT, function() {
net.createServer(common.fail).listen(common.PORT, function() {
var server = this;
var worker = cluster.fork();
worker.on('exit', common.mustCall(function(exitCode) {
Expand All @@ -18,8 +18,8 @@ if (cluster.isMaster) {
});
}
else {
var s = net.createServer(assert.fail);
s.listen(common.PORT, assert.fail.bind(null, 'listen should have failed'));
var s = net.createServer(common.fail);
s.listen(common.PORT, common.fail.bind(null, 'listen should have failed'));
s.on('error', common.mustCall(function(err) {
assert.equal(err.code, 'EADDRINUSE');
process.disconnect();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ if (cluster.isMaster) {
}));
}
else {
var s = net.createServer(assert.fail);
s.listen(42, assert.fail.bind(null, 'listen should have failed'));
var s = net.createServer(common.fail);
s.listen(42, common.fail.bind(null, 'listen should have failed'));
s.on('error', common.mustCall(function(err) {
assert.equal(err.code, 'EACCES');
process.disconnect();
Expand Down
8 changes: 4 additions & 4 deletions test/parallel/test-crypto-pbkdf2.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,28 +62,28 @@ assert.throws(function() {

// Should not work with Infinity key length
assert.throws(function() {
crypto.pbkdf2('password', 'salt', 1, Infinity, assert.fail);
crypto.pbkdf2('password', 'salt', 1, Infinity, common.fail);
}, function(err) {
return err instanceof Error && err.message === 'Bad key length';
});

// Should not work with negative Infinity key length
assert.throws(function() {
crypto.pbkdf2('password', 'salt', 1, -Infinity, assert.fail);
crypto.pbkdf2('password', 'salt', 1, -Infinity, common.fail);
}, function(err) {
return err instanceof Error && err.message === 'Bad key length';
});

// Should not work with NaN key length
assert.throws(function() {
crypto.pbkdf2('password', 'salt', 1, NaN, assert.fail);
crypto.pbkdf2('password', 'salt', 1, NaN, common.fail);
}, function(err) {
return err instanceof Error && err.message === 'Bad key length';
});

// Should not work with negative key length
assert.throws(function() {
crypto.pbkdf2('password', 'salt', 1, -1, assert.fail);
crypto.pbkdf2('password', 'salt', 1, -1, common.fail);
}, function(err) {
return err instanceof Error && err.message === 'Bad key length';
});
4 changes: 2 additions & 2 deletions test/parallel/test-dgram-error-message-address.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ var dgram = require('dgram');
// IPv4 Test
var socket_ipv4 = dgram.createSocket('udp4');

socket_ipv4.on('listening', assert.fail);
socket_ipv4.on('listening', common.fail);

socket_ipv4.on('error', common.mustCall(function(e) {
assert.equal(e.message, 'bind EADDRNOTAVAIL 1.1.1.1:' + common.PORT);
Expand All @@ -22,7 +22,7 @@ socket_ipv4.bind(common.PORT, '1.1.1.1');
var socket_ipv6 = dgram.createSocket('udp6');
var family_ipv6 = 'IPv6';

socket_ipv6.on('listening', assert.fail);
socket_ipv6.on('listening', common.fail);

socket_ipv6.on('error', common.mustCall(function(e) {
// EAFNOSUPPORT or EPROTONOSUPPORT means IPv6 is disabled on this system.
Expand Down
12 changes: 6 additions & 6 deletions test/parallel/test-dgram-oob-buffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,22 +19,22 @@ socket.send(buf, 3, 1, common.PORT, '127.0.0.1', ok);
socket.send(buf, 4, 0, common.PORT, '127.0.0.1', ok);

assert.throws(function() {
socket.send(buf, 0, 5, common.PORT, '127.0.0.1', assert.fail);
socket.send(buf, 0, 5, common.PORT, '127.0.0.1', common.fail);
});
assert.throws(function() {
socket.send(buf, 2, 3, common.PORT, '127.0.0.1', assert.fail);
socket.send(buf, 2, 3, common.PORT, '127.0.0.1', common.fail);
});
assert.throws(function() {
socket.send(buf, 4, 4, common.PORT, '127.0.0.1', assert.fail);
socket.send(buf, 4, 4, common.PORT, '127.0.0.1', common.fail);
});
assert.throws(function() {
socket.send('abc', 4, 1, common.PORT, '127.0.0.1', assert.fail);
socket.send('abc', 4, 1, common.PORT, '127.0.0.1', common.fail);
});
assert.throws(function() {
socket.send('abc', 0, 4, common.PORT, '127.0.0.1', assert.fail);
socket.send('abc', 0, 4, common.PORT, '127.0.0.1', common.fail);
});
assert.throws(function() {
socket.send('abc', -1, 2, common.PORT, '127.0.0.1', assert.fail);
socket.send('abc', -1, 2, common.PORT, '127.0.0.1', common.fail);
});

socket.close(); // FIXME should not be necessary
2 changes: 1 addition & 1 deletion test/parallel/test-event-emitter-remove-listeners.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ assert.deepEqual([], e1.listeners('hello'));

var e2 = new events.EventEmitter();
e2.on('hello', listener1);
e2.on('removeListener', assert.fail);
e2.on('removeListener', common.fail);
e2.removeListener('hello', listener2);
assert.deepEqual([listener1], e2.listeners('hello'));

Expand Down
6 changes: 3 additions & 3 deletions test/parallel/test-fs-null-bytes.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,10 @@ check(fs.symlink, fs.symlinkSync, 'foo\u0000bar', 'foobar');
check(fs.symlink, fs.symlinkSync, 'foobar', 'foo\u0000bar');
check(fs.truncate, fs.truncateSync, 'foo\u0000bar');
check(fs.unlink, fs.unlinkSync, 'foo\u0000bar');
check(null, fs.unwatchFile, 'foo\u0000bar', assert.fail);
check(null, fs.unwatchFile, 'foo\u0000bar', common.fail);
check(fs.utimes, fs.utimesSync, 'foo\u0000bar', 0, 0);
check(null, fs.watch, 'foo\u0000bar', assert.fail);
check(null, fs.watchFile, 'foo\u0000bar', assert.fail);
check(null, fs.watch, 'foo\u0000bar', common.fail);
check(null, fs.watchFile, 'foo\u0000bar', common.fail);
check(fs.writeFile, fs.writeFileSync, 'foo\u0000bar');

// an 'error' for exists means that it doesn't exist.
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-fs-read-stream-err.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,5 +39,5 @@ fs.read = function() {
};

stream.on('data', function(buf) {
stream.on('data', assert.fail); // no more 'data' events should follow
stream.on('data', common.fail); // no more 'data' events should follow
});
2 changes: 1 addition & 1 deletion test/parallel/test-http-client-unescaped-path.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,5 @@ var http = require('http');

assert.throws(function() {
// Path with spaces in it should throw.
http.get({ path: 'bad path' }, assert.fail);
http.get({ path: 'bad path' }, common.fail);
}, /contains unescaped characters/);
2 changes: 1 addition & 1 deletion test/parallel/test-http-set-trailers.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ server.on('listening', function() {
c.on('connect', function() {
outstanding_reqs++;
c.write('GET / HTTP/1.1\r\n\r\n');
tid = setTimeout(assert.fail, 2000, 'Couldn\'t find last chunk.');
tid = setTimeout(common.fail, 2000, 'Couldn\'t find last chunk.');
});

c.on('data', function(chunk) {
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-https-timeout-server-2.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ var options = {
cert: fs.readFileSync(common.fixturesDir + '/keys/agent1-cert.pem')
};

var server = https.createServer(options, assert.fail);
var server = https.createServer(options, common.fail);

server.on('secureConnection', function(cleartext) {
var s = cleartext.setTimeout(50, function() {
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-https-timeout-server.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ var options = {
handshakeTimeout: 50
};

var server = https.createServer(options, assert.fail);
var server = https.createServer(options, common.fail);

server.on('clientError', function(err, conn) {
// Don't hesitate to update the asserts if the internal structure of
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-listen-fd-ebadf.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ process.on('exit', function() {
assert.equal(gotError, 2);
});

net.createServer(assert.fail).listen({fd:2}).on('error', onError);
net.createServer(assert.fail).listen({fd:42}).on('error', onError);
net.createServer(common.fail).listen({fd:2}).on('error', onError);
net.createServer(common.fail).listen({fd:42}).on('error', onError);

function onError(ex) {
assert.equal(ex.code, 'EINVAL');
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-net-better-error-messages-listen-path.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ var common = require('../common');
var assert = require('assert');
var net = require('net');
var fp = '/blah/fadfa';
var server = net.createServer(assert.fail);
server.listen(fp, assert.fail);
var server = net.createServer(common.fail);
server.listen(fp, common.fail);
server.on('error', common.mustCall(function(e) {
assert.equal(e.address, fp);
}));
4 changes: 2 additions & 2 deletions test/parallel/test-net-better-error-messages-listen.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ var common = require('../common');
var assert = require('assert');
var net = require('net');

var server = net.createServer(assert.fail);
server.listen(1, '1.1.1.1', assert.fail);
var server = net.createServer(common.fail);
server.listen(1, '1.1.1.1', common.fail);
server.on('error', common.mustCall(function(e) {
assert.equal(e.address, '1.1.1.1');
assert.equal(e.port, 1);
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-net-better-error-messages-path.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ var assert = require('assert');
var fp = '/tmp/fadagagsdfgsdf';
var c = net.connect(fp);

c.on('connect', assert.fail);
c.on('connect', common.fail);

c.on('error', common.mustCall(function(e) {
assert.equal(e.code, 'ENOENT');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ var assert = require('assert');

var c = net.createConnection(common.PORT, '...');

c.on('connect', assert.fail);
c.on('connect', common.fail);

c.on('error', common.mustCall(function(e) {
assert.equal(e.code, 'ENOTFOUND');
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-net-better-error-messages-port.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ var assert = require('assert');

var c = net.createConnection(common.PORT);

c.on('connect', assert.fail);
c.on('connect', common.fail);

c.on('error', common.mustCall(function(e) {
assert.equal(e.code, 'ECONNREFUSED');
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-net-dns-lookup-skip.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ function check(addressType) {

var address = addressType === 4 ? '127.0.0.1' : '::1';
server.listen(common.PORT, address, function() {
net.connect(common.PORT, address).on('lookup', assert.fail);
net.connect(common.PORT, address).on('lookup', common.fail);
});
}

Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-net-listen-fd0.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ process.on('exit', function() {
});

// this should fail with an async EINVAL error, not throw an exception
net.createServer(assert.fail).listen({fd:0}).on('error', function(e) {
net.createServer(common.fail).listen({fd:0}).on('error', function(e) {
switch (e.code) {
case 'EINVAL':
case 'ENOTSOCK':
Expand Down
Loading

0 comments on commit 60de9f8

Please sign in to comment.