test: remove common.fail()

common.fail() was added to paste over issues with assert.fail() function signature. assert.fail() has been updated to accept a single argument so common.fail() is no longer necessary. PR-URL: #12293 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
nodejs · Trott · Apr 9, 2017 · Apr 12, 2017 · Apr 9, 2017 · Apr 11, 2017
commit e5b11f812efadc5025013337da12a79490ff9ae6
diff --git a/test/common/index.js b/test/common/index.js
@@ -385,7 +385,7 @@ process.on('exit', function() {
   if (!exports.globalCheck) return;
   const leaked = leakedGlobals();
   if (leaked.length > 0) {
-    fail(`Unexpected global(s) found: ${leaked.join(', ')}`);
+    assert.fail(`Unexpected global(s) found: ${leaked.join(', ')}`);
   }
 });
 
@@ -473,14 +473,9 @@ exports.fileExists = function(pathname) {
   }
 };
 
-function fail(msg) {
-  assert.fail(null, null, msg);
-}
-exports.fail = fail;
-
 exports.mustNotCall = function(msg) {
   return function mustNotCall() {
-    fail(msg || 'function should not have been called');
+    assert.fail(msg || 'function should not have been called');
   };
 };
 

diff --git a/test/inspector/inspector-helper.js b/test/inspector/inspector-helper.js
@@ -222,7 +222,7 @@ TestSession.prototype.sendInspectorCommands = function(commands) {
     };
     this.sendAll_(commands, () => {
       timeoutId = setTimeout(() => {
-        common.fail(`Messages without response: ${
+        assert.fail(`Messages without response: ${
           Object.keys(this.messages_).join(', ')}`);
       }, TIMEOUT);
     });

diff --git a/test/internet/test-dgram-send-cb-quelches-error.js b/test/internet/test-dgram-send-cb-quelches-error.js
@@ -28,7 +28,7 @@ function callbackOnly(err) {
 }
 
 function onEvent(err) {
-  common.fail('Error should not be emitted if there is callback');
+  assert.fail('Error should not be emitted if there is callback');
 }
 
 function onError(err) {

diff --git a/test/internet/test-dns.js b/test/internet/test-dns.js
@@ -449,7 +449,7 @@ TEST(function test_lookup_all_mixed(done) {
       else if (isIPv6(ip.address))
         assert.strictEqual(ip.family, 6);
       else
-        assert(false);
+        assert.fail('unexpected IP address');
     });
 
     done();

diff --git a/test/internet/test-tls-add-ca-cert.js b/test/internet/test-tls-add-ca-cert.js
@@ -37,7 +37,7 @@ tls.connect(opts, fail).on('error', common.mustCall((err) => {
 }));
 
 function fail() {
-  assert(false, 'should fail to connect');
+  assert.fail('should fail to connect');
 }
 
 // New secure contexts have the well-known root CAs.

diff --git a/test/parallel/test-beforeexit-event-exit.js b/test/parallel/test-beforeexit-event-exit.js
@@ -1,8 +1,9 @@
 'use strict';
-const common = require('../common');
+require('../common');
+const assert = require('assert');
 
 process.on('beforeExit', function() {
-  common.fail('exit should not allow this to occur');
+  assert.fail('exit should not allow this to occur');
 });
 
 process.exit();
diff --git a/test/parallel/test-child-process-fork-and-spawn.js b/test/parallel/test-child-process-fork-and-spawn.js
@@ -15,7 +15,7 @@ switch (process.argv[2] || '') {
   case 'spawn':
     break;
   default:
-    common.fail();
+    assert.fail();
 }
 
 function checkExit(statusCode) {

diff --git a/test/parallel/test-child-process-stdout-flush-exit.js b/test/parallel/test-child-process-stdout-flush-exit.js
@@ -21,8 +21,7 @@ if (process.argv[2] === 'child') {
 
   child.stderr.setEncoding('utf8');
   child.stderr.on('data', function(data) {
-    console.log('parent stderr: ' + data);
-    assert.ok(false);
+    assert.fail(`Unexpected parent stderr: ${data}`);
   });
 
   // check if we receive both 'hello' at start and 'goodbye' at end

diff --git a/test/parallel/test-cluster-send-handle-twice.js b/test/parallel/test-cluster-send-handle-twice.js
@@ -33,7 +33,7 @@ if (cluster.isMaster) {
     setTimeout(function() { client.end(); }, 50);
   }).on('error', function(e) {
     console.error(e);
-    common.fail('server.listen failed');
+    assert.fail('server.listen failed');
     cluster.worker.disconnect();
   });
 }
diff --git a/test/parallel/test-domain-uncaught-exception.js b/test/parallel/test-domain-uncaught-exception.js
@@ -184,15 +184,16 @@ if (process.argv[2] === 'child') {
       test.expectedMessages.forEach(function(expectedMessage) {
         if (test.messagesReceived === undefined ||
           test.messagesReceived.indexOf(expectedMessage) === -1)
-          assert(false, `test ${test.fn.name} should have sent message: ${
-            expectedMessage} but didn't`);
+          assert.fail('test ' + test.fn.name + ' should have sent message: ' +
+                      expectedMessage + ' but didn\'t');
       });
 
       if (test.messagesReceived) {
         test.messagesReceived.forEach(function(receivedMessage) {
-          if (!test.expectedMessages.includes(receivedMessage)) {
-            assert(false, `test ${test.fn.name} should not have sent message: ${
-              receivedMessage} but did`);
+          if (test.expectedMessages.indexOf(receivedMessage) === -1) {
+            assert.fail('test ' + test.fn.name +
+                        ' should not have sent message: ' + receivedMessage +
+                        ' but did');
           }
         });
       }

diff --git a/test/parallel/test-event-emitter-add-listeners.js b/test/parallel/test-event-emitter-add-listeners.js
@@ -32,9 +32,9 @@ const EventEmitter = require('events');
   });
 
   ee.on('hello', hello);
-  ee.once('foo', common.fail);
+  ee.once('foo', assert.fail);
   assert.deepStrictEqual(['hello', 'foo'], events_new_listener_emited);
-  assert.deepStrictEqual([hello, common.fail], listeners_new_listener_emited);
+  assert.deepStrictEqual([hello, assert.fail], listeners_new_listener_emited);
 
   ee.emit('hello', 'a', 'b');
 }

diff --git a/test/parallel/test-event-emitter-listeners-side-effects.js b/test/parallel/test-event-emitter-listeners-side-effects.js
@@ -1,6 +1,6 @@
 'use strict';
 
-const common = require('../common');
+require('../common');
 const assert = require('assert');
 
 const EventEmitter = require('events').EventEmitter;
@@ -14,12 +14,12 @@ assert.strictEqual(fl.length, 0);
 assert(!(e._events instanceof Object));
 assert.deepStrictEqual(Object.keys(e._events), []);
 
-e.on('foo', common.fail);
+e.on('foo', assert.fail);
 fl = e.listeners('foo');
-assert.strictEqual(e._events.foo, common.fail);
+assert.strictEqual(e._events.foo, assert.fail);
 assert(Array.isArray(fl));
 assert.strictEqual(fl.length, 1);
-assert.strictEqual(fl[0], common.fail);
+assert.strictEqual(fl[0], assert.fail);
 
 e.listeners('bar');
 
@@ -28,12 +28,12 @@ fl = e.listeners('foo');
 
 assert(Array.isArray(e._events.foo));
 assert.strictEqual(e._events.foo.length, 2);
-assert.strictEqual(e._events.foo[0], common.fail);
+assert.strictEqual(e._events.foo[0], assert.fail);
 assert.strictEqual(e._events.foo[1], assert.ok);
 
 assert(Array.isArray(fl));
 assert.strictEqual(fl.length, 2);
-assert.strictEqual(fl[0], common.fail);
+assert.strictEqual(fl[0], assert.fail);
 assert.strictEqual(fl[1], assert.ok);
 
 console.log('ok');
diff --git a/test/parallel/test-event-emitter-once.js b/test/parallel/test-event-emitter-once.js
@@ -13,7 +13,7 @@ e.emit('hello', 'a', 'b');
 e.emit('hello', 'a', 'b');
 
 function remove() {
-  common.fail('once->foo should not be emitted');
+  assert.fail('once->foo should not be emitted');
 }
 
 e.once('foo', remove);

diff --git a/test/parallel/test-event-emitter-remove-listeners.js b/test/parallel/test-event-emitter-remove-listeners.js
@@ -49,11 +49,11 @@ function listener2() {}
   const ee = new EventEmitter();
 
   function remove1() {
-    common.fail('remove1 should not have been called');
+    assert.fail('remove1 should not have been called');
   }
 
   function remove2() {
-    common.fail('remove2 should not have been called');
+    assert.fail('remove2 should not have been called');
   }
 
   ee.on('removeListener', common.mustCall(function(name, cb) {

diff --git a/test/parallel/test-exception-handler2.js b/test/parallel/test-exception-handler2.js
@@ -1,5 +1,6 @@
 'use strict';
 const common = require('../common');
+const assert = require('assert');
 
 process.on('uncaughtException', function(err) {
   console.log(`Caught exception: ${err}`);
@@ -11,4 +12,4 @@ setTimeout(common.mustCall(function() {
 
 // Intentionally cause an exception, but don't catch it.
 nonexistentFunc(); // eslint-disable-line no-undef
-common.fail('This will not run.');
+assert.fail('This will not run.');
diff --git a/test/parallel/test-fs-stat.js b/test/parallel/test-fs-stat.js
@@ -41,7 +41,7 @@ fs.open('.', 'r', undefined, common.mustCall(function(err, fd) {
   try {
     stats = fs.fstatSync(fd);
   } catch (err) {
-    common.fail(err);
+    assert.fail(err);
   }
   if (stats) {
     console.dir(stats);

diff --git a/test/parallel/test-fs-write-stream.js b/test/parallel/test-fs-write-stream.js
@@ -23,7 +23,7 @@ common.refreshTmpDir();
   const stream = fs.createWriteStream(file);
 
   stream.on('drain', function() {
-    common.fail('\'drain\' event must not be emitted before ' +
+    assert.fail('\'drain\' event must not be emitted before ' +
                 'stream.write() has been called at least once.');
   });
   stream.destroy();

diff --git a/test/parallel/test-global-console-exists.js b/test/parallel/test-global-console-exists.js
@@ -4,7 +4,7 @@
 
 'use strict';
 
-const common = require('../common');
+require('../common');
 const assert = require('assert');
 const EventEmitter = require('events');
 const leak_warning = /EventEmitter memory leak detected\. 2 hello listeners/;
@@ -26,7 +26,7 @@ process.stderr.write = (data) => {
   if (write_calls === 0)
     assert.ok(leak_warning.test(data));
   else
-    common.fail('stderr.write should be called only once');
+    assert.fail('stderr.write should be called only once');
 
   write_calls++;
 };

diff --git a/test/parallel/test-http-createConnection.js b/test/parallel/test-http-createConnection.js
@@ -21,7 +21,7 @@ const server = http.createServer(common.mustCall(function(req, res) {
           res.resume();
           fn = common.mustCall(createConnectionError);
           http.get({ createConnection: fn }, function(res) {
-            common.fail('Unexpected response callback');
+            assert.fail('Unexpected response callback');
           }).on('error', common.mustCall(function(err) {
             assert.strictEqual(err.message, 'Could not create socket');
             server.close();

diff --git a/test/parallel/test-http-invalid-path-chars.js b/test/parallel/test-http-invalid-path-chars.js
@@ -1,5 +1,5 @@
 'use strict';
-const common = require('../common');
+require('../common');
 const assert = require('assert');
 const http = require('http');
 
@@ -8,7 +8,7 @@ const theExperimentallyDeterminedNumber = 39;
 
 function fail(path) {
   assert.throws(() => {
-    http.request({ path }, common.fail);
+    http.request({ path }, assert.fail);
   }, expectedError);
 }
 

diff --git a/test/parallel/test-http-localaddress-bind-error.js b/test/parallel/test-http-localaddress-bind-error.js
@@ -1,5 +1,6 @@
 'use strict';
 const common = require('../common');
+const assert = require('assert');
 const http = require('http');
 
 const invalidLocalAddress = '1.2.3.4';
@@ -22,7 +23,7 @@ server.listen(0, '127.0.0.1', common.mustCall(function() {
     method: 'GET',
     localAddress: invalidLocalAddress
   }, function(res) {
-    common.fail('unexpectedly got response from server');
+    assert.fail('unexpectedly got response from server');
   }).on('error', common.mustCall(function(e) {
     console.log(`client got error: ${e.message}`);
     server.close();

diff --git a/test/parallel/test-http-mutable-headers.js b/test/parallel/test-http-mutable-headers.js
@@ -63,6 +63,8 @@ const s = http.createServer(function(req, res) {
       res.setHeader('x-foo', 'keyboard cat');
       res.writeHead(200, { 'x-foo': 'bar', 'x-bar': 'baz' });
       break;
+    default:
+      assert.fail('Unknown test');
   }
 
   res.statusCode = 201;
@@ -120,7 +122,7 @@ function nextTest() {
         break;
 
       default:
-        throw new Error('?');
+        assert.fail('Unknown test');
     }
 
     response.setEncoding('utf8');

diff --git a/test/parallel/test-http-response-multi-content-length.js b/test/parallel/test-http-response-multi-content-length.js
@@ -19,7 +19,7 @@ const server = http.createServer((req, res) => {
       res.writeHead(200, {'content-length': [1, 2]});
       break;
     default:
-      common.fail('should never get here');
+      assert.fail('should never get here');
   }
   res.end('ok');
 });
@@ -35,7 +35,7 @@ server.listen(0, common.mustCall(() => {
     http.get(
       {port: server.address().port, headers: {'x-num': n}},
       (res) => {
-        assert(false, 'client allowed multiple content-length headers.');
+        assert.fail('client allowed multiple content-length headers.');
       }
     ).on('error', common.mustCall((err) => {
       assert(/^Parse Error/.test(err.message));

diff --git a/test/parallel/test-http-response-splitting.js b/test/parallel/test-http-response-splitting.js
@@ -1,6 +1,6 @@
 'use strict';
 
-const common = require('../common');
+require('../common');
 const http = require('http');
 const net = require('net');
 const url = require('url');
@@ -38,7 +38,7 @@ const server = http.createServer((req, res) => {
       test(res, 200, {'foo': y});
       break;
     default:
-      common.fail('should not get to here.');
+      assert.fail('should not get to here.');
   }
   if (count === 3)
     server.close();

diff --git a/test/parallel/test-http-response-status-message.js b/test/parallel/test-http-response-status-message.js
@@ -23,7 +23,7 @@ testCases.findByPath = function(path) {
     return testCase.path === path;
   });
   if (matching.length === 0) {
-    throw 'failed to find test case with path ' + path;
+    assert.fail(`failed to find test case with path ${path}`);
   }
   return matching[0];
 };

diff --git a/test/parallel/test-http-server-reject-chunked-with-content-length.js b/test/parallel/test-http-server-reject-chunked-with-content-length.js
@@ -23,7 +23,7 @@ server.listen(0, () => {
   client.on('data', (data) => {
     // Should not get to this point because the server should simply
     // close the connection without returning any data.
-    common.fail('no data should be returned by the server');
+    assert.fail('no data should be returned by the server');
   });
   client.on('end', common.mustCall(() => {}));
 });
diff --git a/test/parallel/test-http-unix-socket.js b/test/parallel/test-http-unix-socket.js
@@ -45,7 +45,7 @@ server.listen(common.PIPE, common.mustCall(function() {
   }));
 
   req.on('error', function(e) {
-    common.fail(e.stack);
+    assert.fail(e.stack);
   });
 
   req.end();

diff --git a/test/parallel/test-https-localaddress-bind-error.js b/test/parallel/test-https-localaddress-bind-error.js
@@ -4,6 +4,7 @@ const common = require('../common');
 if (!common.hasCrypto)
   common.skip('missing crypto');
 
+const assert = require('assert');
 const fs = require('fs');
 const https = require('https');
 
@@ -32,7 +33,7 @@ server.listen(0, '127.0.0.1', common.mustCall(function() {
     method: 'GET',
     localAddress: invalidLocalAddress
   }, function(res) {
-    common.fail('unexpectedly got response from server');
+    assert.fail('unexpectedly got response from server');
   }).on('error', common.mustCall(function(e) {
     console.log(`client got error: ${e.message}`);
     server.close();

diff --git a/test/parallel/test-net-error-twice.js b/test/parallel/test-net-error-twice.js
@@ -20,7 +20,7 @@ const srv = net.createServer(function onConnection(conn) {
   conn.on('error', function(err) {
     errs.push(err);
     if (errs.length > 1 && errs[0] === errs[1])
-      assert(false, 'We should not be emitting the same error twice');
+      assert.fail('Should not emit the same error twice');
   });
   conn.on('close', function() {
     srv.unref();