assert: deprecate assert.fail partially

Using `assert.fail()` with more than one argument is not intuitive
to use and has no benefit over using a message on its own.

Therefore this introduces a runtime deprecation in case it is used
in that way.

PR-URL: https://github.com/nodejs/node/pull/18418
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

2 files changed, 72 insertions, 63 deletions

diff --git a/test/parallel/test-assert-fail-deprecation.js b/test/parallel/test-assert-fail-deprecation.js
new file mode 100644
index 0000000000..9da26dab3f
--- /dev/null
+++ b/test/parallel/test-assert-fail-deprecation.js
@@ -0,0 +1,64 @@
+'use strict';
+
+const common = require('../common');
+const assert = require('assert');
+
+common.expectWarning(
+  'DeprecationWarning',
+  'assert.fail() with more than one argument is deprecated. ' +
+    'Please use assert.strictEqual() instead or only pass a message.'
+);
+
+// Two args only, operator defaults to '!='
+assert.throws(() => {
+  assert.fail('first', 'second');
+}, {
+  code: 'ERR_ASSERTION',
+  name: 'AssertionError [ERR_ASSERTION]',
+  message: '\'first\' != \'second\'',
+  operator: '!=',
+  actual: 'first',
+  expected: 'second'
+});
+
+// Three args
+assert.throws(() => {
+  assert.fail('ignored', 'ignored', 'another custom message');
+}, {
+  code: 'ERR_ASSERTION',
+  name: 'AssertionError [ERR_ASSERTION]',
+  message: 'another custom message',
+  operator: undefined,
+  actual: 'ignored',
+  expected: 'ignored'
+});
+
+// Three args with custom Error
+assert.throws(() => {
+  assert.fail(typeof 1, 'object', new TypeError('another custom message'));
+}, {
+  name: 'TypeError',
+  message: 'another custom message',
+  operator: undefined,
+  actual: undefined,
+  expected: undefined,
+  code: undefined
+});
+
+// No third arg (but a fourth arg)
+assert.throws(() => {
+  assert.fail('first', 'second', undefined, 'operator');
+}, {
+  code: 'ERR_ASSERTION',
+  name: 'AssertionError [ERR_ASSERTION]',
+  message: '\'first\' operator \'second\'',
+  operator: 'operator',
+  actual: 'first',
+  expected: 'second'
+});
+
+// The stackFrameFunction should exclude the foo frame
+assert.throws(
+  function foo() { assert.fail('first', 'second', 'message', '!==', foo); },
+  (err) => !/^\s*at\sfoo\b/m.test(err.stack)
+);
diff --git a/test/parallel/test-assert-fail.js b/test/parallel/test-assert-fail.js
index 8d67a6e63f..fc2b6cc03c 100644
--- a/test/parallel/test-assert-fail.js
+++ b/test/parallel/test-assert-fail.js
@@ -1,29 +1,27 @@
 'use strict';
 
-/* eslint-disable prefer-common-expectserror */
-
-const common = require('../common');
+require('../common');
 const assert = require('assert');
 
 // No args
 assert.throws(
   () => { assert.fail(); },
-  common.expectsError({
+  {
     code: 'ERR_ASSERTION',
-    type: assert.AssertionError,
+    name: 'AssertionError [ERR_ASSERTION]',
     message: 'Failed',
     operator: undefined,
     actual: undefined,
     expected: undefined
-  })
+  }
 );
 
 // One arg = message
-common.expectsError(() => {
+assert.throws(() => {
   assert.fail('custom message');
 }, {
   code: 'ERR_ASSERTION',
-  type: assert.AssertionError,
+  name: 'AssertionError [ERR_ASSERTION]',
   message: 'custom message',
   operator: undefined,
   actual: undefined,
@@ -31,65 +29,12 @@ common.expectsError(() => {
 });
 
 // One arg = Error
-common.expectsError(() => {
+assert.throws(() => {
   assert.fail(new TypeError('custom message'));
 }, {
-  type: TypeError,
+  name: 'TypeError',
   message: 'custom message',
   operator: undefined,
   actual: undefined,
   expected: undefined
 });
-
-// Two args only, operator defaults to '!='
-common.expectsError(() => {
-  assert.fail('first', 'second');
-}, {
-  code: 'ERR_ASSERTION',
-  type: assert.AssertionError,
-  message: '\'first\' != \'second\'',
-  operator: '!=',
-  actual: 'first',
-  expected: 'second'
-});
-
-// Three args
-common.expectsError(() => {
-  assert.fail('ignored', 'ignored', 'another custom message');
-}, {
-  code: 'ERR_ASSERTION',
-  type: assert.AssertionError,
-  message: 'another custom message',
-  operator: undefined,
-  actual: 'ignored',
-  expected: 'ignored'
-});
-
-// Three args with custom Error
-common.expectsError(() => {
-  assert.fail(typeof 1, 'object', new TypeError('another custom message'));
-}, {
-  type: TypeError,
-  message: 'another custom message',
-  operator: undefined,
-  actual: 'number',
-  expected: 'object'
-});
-
-// No third arg (but a fourth arg)
-common.expectsError(() => {
-  assert.fail('first', 'second', undefined, 'operator');
-}, {
-  code: 'ERR_ASSERTION',
-  type: assert.AssertionError,
-  message: '\'first\' operator \'second\'',
-  operator: 'operator',
-  actual: 'first',
-  expected: 'second'
-});
-
-// The stackFrameFunction should exclude the foo frame
-assert.throws(
-  function foo() { assert.fail('first', 'second', 'message', '!==', foo); },
-  (err) => !/^\s*at\sfoo\b/m.test(err.stack)
-);