assert.fail() accept a single argument or two arguments

.eslintrc.yaml

@@ -110,9 +110,6 @@ rules:
   }, {
     selector: "ThrowStatement > CallExpression[callee.name=/Error$/]",
     message: "Use new keyword when throwing an Error."
-    }, {
-    selector: "CallExpression[callee.object.name='assert'][callee.property.name='fail'][arguments.length=1]",
-    message: "assert.fail() message should be third argument"
     }]
   no-tabs: 2
   no-trailing-spaces: 2

doc/api/assert.md

@@ -256,14 +256,15 @@ If the values are not equal, an `AssertionError` is thrown with a `message`
 property set equal to the value of the `message` parameter. If the `message`
 parameter is undefined, a default error message is assigned.
 
+## assert.fail(message)
 ## assert.fail(actual, expected, message, operator)
 <!-- YAML
 added: v0.1.21
 -->
 * `actual` {any}
 * `expected` {any}
 * `message` {any}
-* `operator` {string}
+* `operator` {string} (default: '!=')
 
 Throws an `AssertionError`. If `message` is falsy, the error message is set as
 the values of `actual` and `expected` separated by the provided `operator`.
@@ -277,6 +278,12 @@ assert.fail(1, 2, undefined, '>');
 
 assert.fail(1, 2, 'whoops', '>');
 // AssertionError: whoops
+
+assert.fail('boom');
+// AssertionError: boom
+
+assert.fail('a', 'b');
+// AssertionError: 'a' != 'b'
 ```
 
 ## assert.ifError(value)

lib/assert.js

@@ -79,6 +79,10 @@ function getMessage(self) {
 // display purposes.
 
 function fail(actual, expected, message, operator, stackStartFunction) {
+  if (arguments.length === 1)
+    message = actual;
+  if (arguments.length === 2)
+    operator = '!=';
   throw new assert.AssertionError({
     message: message,
     actual: actual,

test/README.md

@@ -246,12 +246,6 @@ Checks whether `IPv6` is supported on this platform.
 
 Checks if there are multiple localhosts available.
 
-### fail(msg)
-* `msg` [<String>](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Data_structures#String_type)
-* return [<Boolean>](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Data_structures#Boolean_type)
-
-Throws an `AssertionError` with `msg`
-
 ### fileExists(pathname)
 * pathname [<String>](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Data_structures#String_type)
 * return [<Boolean>](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Data_structures#Boolean_type)

test/common.js

@@ -404,7 +404,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(', ')}`);
   }
 });
 
@@ -507,14 +507,9 @@ exports.canCreateSymLink = function() {
   return true;
 };
 
-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');
   };
 };
 
@@ -627,7 +622,7 @@ exports.WPT = {
   },
   assert_array_equals: assert.deepStrictEqual,
   assert_unreached(desc) {
-    assert.fail(undefined, undefined, `Reached unreachable code: ${desc}`);
+    assert.fail(`Reached unreachable code: ${desc}`);
   }
 };
 

test/inspector/inspector-helper.js

@@ -217,7 +217,7 @@ TestSession.prototype.sendInspectorCommands = function(commands) {
         for (const id in this.messages_) {
           s += id + ', ';
         }
-        common.fail('Messages without response: ' +
+        assert.fail('Messages without response: ' +
                     s.substring(0, s.length - 2));
       }, TIMEOUT);
     });

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) {

test/internet/test-dns.js

@@ -453,7 +453,7 @@ TEST(function test_lookup_all_mixed(done) {
       else if (isIPv6(ip.address))
         assert.strictEqual(ip.family, 6);
       else
-        common.fail('unexpected IP address');
+        assert.fail('unexpected IP address');
     });
 
     done();

test/internet/test-tls-add-ca-cert.js

@@ -38,7 +38,7 @@ tls.connect(opts, fail).on('error', common.mustCall((err) => {
 }));
 
 function fail() {
-  common.fail('should fail to connect');
+  assert.fail('should fail to connect');
 }
 
 // New secure contexts have the well-known root CAs.

test/parallel/test-assert-fail.js

@@ -0,0 +1,33 @@
+'use strict';
+require('../common');
+const assert = require('assert');
+
+// no args
+assert.throws(
+  () => { assert.fail(); },
+  /^AssertionError: undefined undefined undefined$/
+);
+
+// one arg = message
+assert.throws(
+  () => { assert.fail('custom message'); },
+  /^AssertionError: custom message$/
+);
+
+// two args only, operator defaults to '!='
+assert.throws(
+  () => { assert.fail('first', 'second'); },
+  /^AssertionError: 'first' != 'second'$/
+);
+
+// three args
+assert.throws(
+  () => { assert.fail('ignored', 'ignored', 'another custom message'); },
+  /^AssertionError: another custom message$/
+);
+
+// no third arg (but a fourth arg)
+assert.throws(
+  () => { assert.fail('first', 'second', undefined, 'operator'); },
+  /^AssertionError: 'first' operator 'second'$/
+);

test/parallel/test-beforeexit-event-exit.js

@@ -20,10 +20,11 @@
 // USE OR OTHER DEALINGS IN THE SOFTWARE.
 
 '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();

test/parallel/test-child-process-fork-and-spawn.js

@@ -37,7 +37,7 @@ switch (process.argv[2] || '') {
   case 'spawn':
     break;
   default:
-    common.fail();
+    assert.fail();
 }
 
 function checkExit(statusCode) {

test/parallel/test-child-process-stdout-flush-exit.js

@@ -42,7 +42,7 @@ if (process.argv[2] === 'child') {
 
   child.stderr.setEncoding('utf8');
   child.stderr.on('data', function(data) {
-    common.fail(`Unexpected parent stderr: ${data}`);
+    assert.fail(`Unexpected parent stderr: ${data}`);
   });
 
   // check if we receive both 'hello' at start and 'goodbye' at end

test/parallel/test-cluster-send-handle-twice.js

@@ -51,7 +51,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();
   });
 }

test/parallel/test-common.js

@@ -40,8 +40,8 @@ assert.throws(function() {
 }, /^TypeError: Invalid expected value: \/foo\/$/);
 
 
-// common.fail() tests
+// assert.fail() tests
 assert.throws(
-  () => { common.fail('fhqwhgads'); },
+  () => { assert.fail('fhqwhgads'); },
   /^AssertionError: fhqwhgads$/
 );

test/parallel/test-dgram-address.js

@@ -41,7 +41,7 @@ const dgram = require('dgram');
 
   socket.on('error', (err) => {
     socket.close();
-    common.fail(`Unexpected error on udp4 socket. ${err.toString()}`);
+    assert.fail(`Unexpected error on udp4 socket. ${err.toString()}`);
   });
 
   socket.bind(0, common.localhostIPv4);
@@ -65,7 +65,7 @@ if (common.hasIPv6) {
 
   socket.on('error', (err) => {
     socket.close();
-    common.fail(`Unexpected error on udp6 socket. ${err.toString()}`);
+    assert.fail(`Unexpected error on udp6 socket. ${err.toString()}`);
   });
 
   socket.bind(0, localhost);

test/parallel/test-dgram-implicit-bind-failure.js

@@ -40,7 +40,7 @@ socket.on('error', (err) => {
     return;
   }
 
-  common.fail(`Unexpected error: ${err}`);
+  assert.fail(`Unexpected error: ${err}`);
 });
 
 // Initiate a few send() operations, which will fail.

test/parallel/test-domain-uncaught-exception.js

@@ -9,6 +9,7 @@
  */
 
 const common = require('../common');
+const assert = require('assert');
 const domain = require('domain');
 const child_process = require('child_process');
 
@@ -183,14 +184,14 @@ if (process.argv[2] === 'child') {
       test.expectedMessages.forEach(function(expectedMessage) {
         if (test.messagesReceived === undefined ||
           test.messagesReceived.indexOf(expectedMessage) === -1)
-          common.fail('test ' + test.fn.name + ' should have sent message: ' +
+          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.indexOf(receivedMessage) === -1) {
-            common.fail('test ' + test.fn.name +
+            assert.fail('test ' + test.fn.name +
                         ' should not have sent message: ' + receivedMessage +
                         ' but did');
           }

test/parallel/test-event-emitter-add-listeners.js

@@ -53,9 +53,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_emitted);
-  assert.deepStrictEqual([hello, common.fail], listeners_new_listener_emitted);
+  assert.deepStrictEqual([hello, assert.fail], listeners_new_listener_emitted);
 
   ee.emit('hello', 'a', 'b');
 }

test/parallel/test-event-emitter-listeners-side-effects.js

@@ -21,7 +21,7 @@
 
 'use strict';
 
-const common = require('../common');
+require('../common');
 const assert = require('assert');
 
 const EventEmitter = require('events').EventEmitter;
@@ -35,12 +35,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');
 
@@ -49,12 +49,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');

test/parallel/test-event-emitter-once.js

@@ -34,7 +34,7 @@ e.emit('hello', 'a', 'b');
 e.emit('hello', 'a', 'b');
 
 const remove = function() {
-  common.fail('once->foo should not be emitted');
+  assert.fail('once->foo should not be emitted');
 };
 
 e.once('foo', remove);

test/parallel/test-event-emitter-remove-listeners.js

@@ -70,11 +70,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) {

test/parallel/test-exception-handler2.js

@@ -21,6 +21,7 @@
 
 'use strict';
 const common = require('../common');
+const assert = require('assert');
 
 process.on('uncaughtException', function(err) {
   console.log('Caught exception: ' + err);
@@ -32,4 +33,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.');

test/parallel/test-fs-null-bytes.js

@@ -95,10 +95,10 @@ check(fs.symlink, fs.symlinkSync, fileUrl, 'foobar');
 check(fs.symlink, fs.symlinkSync, 'foobar', fileUrl);
 check(fs.truncate, fs.truncateSync, fileUrl);
 check(fs.unlink, fs.unlinkSync, fileUrl);
-check(null, fs.unwatchFile, fileUrl, common.fail);
+check(null, fs.unwatchFile, fileUrl, assert.fail);
 check(fs.utimes, fs.utimesSync, fileUrl, 0, 0);
-check(null, fs.watch, fileUrl, common.fail);
-check(null, fs.watchFile, fileUrl, common.fail);
+check(null, fs.watch, fileUrl, assert.fail);
+check(null, fs.watchFile, fileUrl, assert.fail);
 check(fs.writeFile, fs.writeFileSync, fileUrl, 'abc');
 
 check(fs.access, fs.accessSync, fileUrl2);
@@ -123,10 +123,10 @@ check(fs.symlink, fs.symlinkSync, fileUrl2, 'foobar');
 check(fs.symlink, fs.symlinkSync, 'foobar', fileUrl2);
 check(fs.truncate, fs.truncateSync, fileUrl2);
 check(fs.unlink, fs.unlinkSync, fileUrl2);
-check(null, fs.unwatchFile, fileUrl2, common.fail);
+check(null, fs.unwatchFile, fileUrl2, assert.fail);
 check(fs.utimes, fs.utimesSync, fileUrl2, 0, 0);
-check(null, fs.watch, fileUrl2, common.fail);
-check(null, fs.watchFile, fileUrl2, common.fail);
+check(null, fs.watch, fileUrl2, assert.fail);
+check(null, fs.watchFile, fileUrl2, assert.fail);
 check(fs.writeFile, fs.writeFileSync, fileUrl2, 'abc');
 
 // an 'error' for exists means that it doesn't exist.

test/parallel/test-fs-stat.js

@@ -62,7 +62,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);