test: wrap assert.fail when passed to callback

Myles Borins · Trott · commit 28e9a022df3e · 2015-10-24T14:42:41.000-07:00
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: nodejs#3453 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
diff --git a/test/common.js b/test/common.js
@@ -442,3 +442,7 @@ exports.fileExists = function(pathname) {
     return false;
   }
 };
+
+exports.fail = function(msg) {
+  assert.fail(null, null, msg);
+};
diff --git a/test/parallel/test-child-process-recv-handle.js b/test/parallel/test-child-process-recv-handle.js
@@ -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);
diff --git a/test/parallel/test-child-process-spawn-typeerror.js b/test/parallel/test-child-process-spawn-typeerror.js
@@ -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
diff --git a/test/parallel/test-cluster-bind-privileged-port.js b/test/parallel/test-cluster-bind-privileged-port.js
@@ -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();
diff --git a/test/parallel/test-cluster-bind-twice.js b/test/parallel/test-cluster-bind-twice.js
@@ -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');
   });
 
@@ -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);
diff --git a/test/parallel/test-cluster-eaddrinuse.js b/test/parallel/test-cluster-eaddrinuse.js
@@ -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) {
@@ -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();
       }));
diff --git a/test/parallel/test-cluster-net-listen.js b/test/parallel/test-cluster-net-listen.js
@@ -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);
 }
diff --git a/test/parallel/test-cluster-rr-ref.js b/test/parallel/test-cluster-rr-ref.js
@@ -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();
diff --git a/test/parallel/test-cluster-setup-master-argv.js b/test/parallel/test-cluster-setup-master-argv.js
@@ -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;
diff --git a/test/parallel/test-cluster-shared-handle-bind-error.js b/test/parallel/test-cluster-shared-handle-bind-error.js
@@ -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) {
@@ -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();
diff --git a/test/parallel/test-cluster-shared-handle-bind-privileged-port.js b/test/parallel/test-cluster-shared-handle-bind-privileged-port.js
@@ -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();
diff --git a/test/parallel/test-crypto-pbkdf2.js b/test/parallel/test-crypto-pbkdf2.js
@@ -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';
 });
diff --git a/test/parallel/test-dgram-error-message-address.js b/test/parallel/test-dgram-error-message-address.js
@@ -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);
@@ -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.
diff --git a/test/parallel/test-dgram-oob-buffer.js b/test/parallel/test-dgram-oob-buffer.js
@@ -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
diff --git a/test/parallel/test-event-emitter-remove-listeners.js b/test/parallel/test-event-emitter-remove-listeners.js
@@ -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'));
 
diff --git a/test/parallel/test-fs-null-bytes.js b/test/parallel/test-fs-null-bytes.js
@@ -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.
diff --git a/test/parallel/test-fs-read-stream-err.js b/test/parallel/test-fs-read-stream-err.js
@@ -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
 });
diff --git a/test/parallel/test-http-client-unescaped-path.js b/test/parallel/test-http-client-unescaped-path.js
@@ -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/);
diff --git a/test/parallel/test-http-set-trailers.js b/test/parallel/test-http-set-trailers.js
@@ -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) {
diff --git a/test/parallel/test-https-timeout-server-2.js b/test/parallel/test-https-timeout-server-2.js
@@ -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() {
diff --git a/test/parallel/test-https-timeout-server.js b/test/parallel/test-https-timeout-server.js
@@ -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
diff --git a/test/parallel/test-listen-fd-ebadf.js b/test/parallel/test-listen-fd-ebadf.js
@@ -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');
diff --git a/test/parallel/test-net-better-error-messages-listen-path.js b/test/parallel/test-net-better-error-messages-listen-path.js
@@ -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);
 }));
diff --git a/test/parallel/test-net-better-error-messages-listen.js b/test/parallel/test-net-better-error-messages-listen.js
@@ -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);
diff --git a/test/parallel/test-net-better-error-messages-path.js b/test/parallel/test-net-better-error-messages-path.js
@@ -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');
diff --git a/test/parallel/test-net-better-error-messages-port-hostname.js b/test/parallel/test-net-better-error-messages-port-hostname.js
@@ -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');
diff --git a/test/parallel/test-net-better-error-messages-port.js b/test/parallel/test-net-better-error-messages-port.js
@@ -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');
diff --git a/test/parallel/test-net-dns-lookup-skip.js b/test/parallel/test-net-dns-lookup-skip.js
@@ -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);
   });
 }
 
diff --git a/test/parallel/test-net-listen-fd0.js b/test/parallel/test-net-listen-fd0.js
@@ -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':
diff --git a/test/parallel/test-promises-unhandled-rejections.js b/test/parallel/test-promises-unhandled-rejections.js
diff --git a/test/parallel/test-timers-unref-remove-other-unref-timers.js b/test/parallel/test-timers-unref-remove-other-unref-timers.js
diff --git a/test/parallel/test-tls-cipher-list.js b/test/parallel/test-tls-cipher-list.js
diff --git a/test/parallel/test-tls-no-sslv3.js b/test/parallel/test-tls-no-sslv3.js
diff --git a/test/parallel/test-tls-timeout-server.js b/test/parallel/test-tls-timeout-server.js
diff --git a/test/parallel/test-vm-debug-context.js b/test/parallel/test-vm-debug-context.js
diff --git a/test/pummel/test-fs-watch-non-recursive.js b/test/pummel/test-fs-watch-non-recursive.js
diff --git a/test/sequential/test-cluster-listening-port.js b/test/sequential/test-cluster-listening-port.js

Original file line number	Diff line number	Diff line change
`@@ -442,3 +442,7 @@ exports.fileExists = function(pathname) {`
`442`	`442`	`return false;`
`443`	`443`	`}`
`444`	`444`	`};`
	`445`	`+`
	`446`	`+exports.fail = function(msg) {`
	`447`	`+ assert.fail(null, null, msg);`
	`448`	`+};`
Original file line number	Diff line number	Diff line change
`@@ -17,5 +17,5 @@ if (cluster.isMaster) {`
`17`	`17`	`}`
`18`	`18`	`else {`
`19`	`19`	`// listen() without port should not trigger a libuv assert`
`20`		`- net.createServer(assert.fail).listen(process.exit);`
	`20`	`+ net.createServer(common.fail).listen(process.exit);`
`21`	`21`	`}`