assert: improve default error messages

This improves the error messages for:
- assert.notDeepStrictEqual
- assert.deepStrictEqual
- assert.notStrictEqual
- assert.strictEqual

Those will now always use the same error message as used in the
strict mode.

PR-URL: #19467
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

Author: BridgeAR

lib/assert.js

@@ -54,7 +54,6 @@ const meta = [
 
 const escapeFn = (str) => meta[str.charCodeAt(0)];
 
-const ERR_DIFF_DEACTIVATED = 0;
 const ERR_DIFF_NOT_EQUAL = 1;
 const ERR_DIFF_EQUAL = 2;
 
@@ -323,7 +322,7 @@ assert.deepStrictEqual = function deepStrictEqual(actual, expected, message) {
       message,
       operator: 'deepStrictEqual',
       stackStartFn: deepStrictEqual,
-      errorDiff: this === strict ? ERR_DIFF_EQUAL : ERR_DIFF_DEACTIVATED
+      errorDiff: ERR_DIFF_EQUAL
     });
   }
 };
@@ -337,7 +336,7 @@ function notDeepStrictEqual(actual, expected, message) {
       message,
       operator: 'notDeepStrictEqual',
       stackStartFn: notDeepStrictEqual,
-      errorDiff: this === strict ? ERR_DIFF_NOT_EQUAL : ERR_DIFF_DEACTIVATED
+      errorDiff: ERR_DIFF_NOT_EQUAL
     });
   }
 }
@@ -350,7 +349,7 @@ assert.strictEqual = function strictEqual(actual, expected, message) {
       message,
       operator: 'strictEqual',
       stackStartFn: strictEqual,
-      errorDiff: this === strict ? ERR_DIFF_EQUAL : ERR_DIFF_DEACTIVATED
+      errorDiff: ERR_DIFF_EQUAL
     });
   }
 };
@@ -363,7 +362,7 @@ assert.notStrictEqual = function notStrictEqual(actual, expected, message) {
       message,
       operator: 'notStrictEqual',
       stackStartFn: notStrictEqual,
-      errorDiff: this === strict ? ERR_DIFF_NOT_EQUAL : ERR_DIFF_DEACTIVATED
+      errorDiff: ERR_DIFF_NOT_EQUAL
     });
   }
 };

lib/internal/errors.js

@@ -19,6 +19,13 @@ let green = '';
 let red = '';
 let white = '';
 
+const READABLE_OPERATOR = {
+  deepStrictEqual: 'Input A expected to strictly deep-equal input B',
+  notDeepStrictEqual: 'Input A expected to strictly not deep-equal input B',
+  strictEqual: 'Input A expected to strictly equal input B',
+  notStrictEqual: 'Input A expected to strictly not equal input B'
+};
+
 const {
   errmap,
   UV_EAI_MEMORY,
@@ -40,10 +47,34 @@ function lazyInternalUtil() {
   return internalUtil;
 }
 
+function copyError(source) {
+  const keys = Object.keys(source);
+  const target = Object.create(Object.getPrototypeOf(source));
+  for (const key of keys) {
+    target[key] = source[key];
+  }
+  Object.defineProperty(target, 'message', { value: source.message });
+  return target;
+}
+
 function inspectValue(val) {
+  // The util.inspect default values could be changed. This makes sure the
+  // error messages contain the necessary information nevertheless.
   return util.inspect(
     val,
-    { compact: false, customInspect: false }
+    {
+      compact: false,
+      customInspect: false,
+      depth: 1000,
+      maxArrayLength: Infinity,
+      // Assert compares only enumerable properties (with a few exceptions).
+      showHidden: false,
+      // Having a long line as error is better than wrapping the line for
+      // comparison.
+      breakLength: Infinity,
+      // Assert does not detect proxies currently.
+      showProxy: false
+    }
   ).split('\n');
 }
 
@@ -226,8 +257,8 @@ function createErrDiff(actual, expected, operator) {
   if (util === undefined) util = require('util');
   const actualLines = inspectValue(actual);
   const expectedLines = inspectValue(expected);
-  const msg = `Input A expected to ${operator} input B:\n` +
-        `${green}+ expected${white} ${red}- actual${white}`;
+  const msg = READABLE_OPERATOR[operator] +
+        `:\n${green}+ expected${white} ${red}- actual${white}`;
   const skippedMsg = ' ... Lines skipped';
 
   // Remove all ending lines that match (this optimizes the output for
@@ -259,6 +290,7 @@ function createErrDiff(actual, expected, operator) {
 
   const maxLines = Math.max(actualLines.length, expectedLines.length);
   var printedLines = 0;
+  var identical = 0;
   for (i = 0; i < maxLines; i++) {
     // Only extra expected lines exist
     const cur = i - lastPos;
@@ -318,12 +350,38 @@ function createErrDiff(actual, expected, operator) {
         res += `\n  ${actualLines[i]}`;
         printedLines++;
       }
+      identical++;
     }
     // Inspected object to big (Show ~20 rows max)
     if (printedLines > 20 && i < maxLines - 2) {
       return `${msg}${skippedMsg}\n${res}\n...${other}\n...`;
     }
   }
+
+  // Strict equal with identical objects that are not identical by reference.
+  if (identical === maxLines) {
+    let base = 'Input object identical but not reference equal:';
+
+    if (operator !== 'strictEqual') {
+      // This code path should not be possible to reach.
+      // The output is identical but it is not clear why.
+      base = 'Input objects not identical:';
+    }
+
+    // We have to get the result again. The lines were all removed before.
+    const actualLines = inspectValue(actual);
+
+    // Only remove lines in case it makes sense to collapse those.
+    // TODO: Accept env to always show the full error.
+    if (actualLines.length > 30) {
+      actualLines[26] = '...';
+      while (actualLines.length > 27) {
+        actualLines.pop();
+      }
+    }
+
+    return `${base}\n\n  ${actualLines.join('\n  ')}\n`;
+  }
   return `${msg}${skipped ? skippedMsg : ''}\n${res}${other}${end}`;
 }
 
@@ -358,13 +416,15 @@ class AssertionError extends Error {
         }
       }
       if (util === undefined) util = require('util');
+      // Prevent the error stack from being visible by duplicating the error
+      // in a very close way to the original in case both sides are actually
+      // instances of Error.
       if (typeof actual === 'object' && actual !== null &&
-        'stack' in actual && actual instanceof Error) {
-        actual = `${actual.name}: ${actual.message}`;
-      }
-      if (typeof expected === 'object' && expected !== null &&
-        'stack' in expected && expected instanceof Error) {
-        expected = `${expected.name}: ${expected.message}`;
+          typeof expected === 'object' && expected !== null &&
+          'stack' in actual && actual instanceof Error &&
+          'stack' in expected && expected instanceof Error) {
+        actual = copyError(actual);
+        expected = copyError(expected);
       }
 
       if (errorDiff === 0) {
@@ -379,15 +439,23 @@ class AssertionError extends Error {
         // In case the objects are equal but the operator requires unequal, show
         // the first object and say A equals B
         const res = inspectValue(actual);
+        const base = `Identical input passed to ${operator}:`;
 
-        if (res.length > 20) {
-          res[19] = '...';
-          while (res.length > 20) {
+        // Only remove lines in case it makes sense to collapse those.
+        // TODO: Accept env to always show the full error.
+        if (res.length > 30) {
+          res[26] = '...';
+          while (res.length > 27) {
             res.pop();
           }
         }
-        // Only print a single object.
-        super(`Identical input passed to ${operator}:\n${res.join('\n')}`);
+
+        // Only print a single input.
+        if (res.length === 1) {
+          super(`${base} ${res[0]}`);
+        } else {
+          super(`${base}\n\n  ${res.join('\n  ')}\n`);
+        }
       } else {
         super(createErrDiff(actual, expected, operator));
       }

test/message/assert_throws_stack.out

@@ -2,7 +2,7 @@ assert.js:*
   throw new AssertionError(obj);
   ^
 
-AssertionError [ERR_ASSERTION]: Input A expected to deepStrictEqual input B:
+AssertionError [ERR_ASSERTION]: Input A expected to strictly deep-equal input B:
 + expected - actual
 
 - Comparison {}

test/message/error_exit.out

@@ -3,7 +3,11 @@ assert.js:*
   throw new AssertionError(obj);
   ^
 
-AssertionError [ERR_ASSERTION]: 1 strictEqual 2
+AssertionError [ERR_ASSERTION]: Input A expected to strictly equal input B:
++ expected - actual
+
+- 1
++ 2
     at Object.<anonymous> (*test*message*error_exit.js:*:*)
     at Module._compile (internal/modules/cjs/loader.js:*:*)
     at Object.Module._extensions..js (internal/modules/cjs/loader.js:*:*)

test/parallel/test-assert-checktag.js

@@ -1,24 +1,6 @@
 'use strict';
-const common = require('../common');
+require('../common');
 const assert = require('assert');
-const util = require('util');
-
-// Template tag function turning an error message into a RegExp
-// for assert.throws()
-function re(literals, ...values) {
-  let result = literals[0];
-  const escapeRE = /[\\^$.*+?()[\]{}|=!<>:-]/g;
-  for (const [i, value] of values.entries()) {
-    const str = util.inspect(value);
-    // Need to escape special characters.
-    result += str.replace(escapeRE, '\\$&');
-    result += literals[i + 1];
-  }
-  return common.expectsError({
-    code: 'ERR_ASSERTION',
-    message: new RegExp(`^${result}$`)
-  });
-}
 
 // Turn off no-restricted-properties because we are testing deepEqual!
 /* eslint-disable no-restricted-properties */
@@ -35,10 +17,20 @@ function re(literals, ...values) {
 
   // For deepStrictEqual we check the runtime type,
   // then reveal the fakeness of the fake date
-  assert.throws(() => assert.deepStrictEqual(date, fake),
-                re`${date} deepStrictEqual Date {}`);
-  assert.throws(() => assert.deepStrictEqual(fake, date),
-                re`Date {} deepStrictEqual ${date}`);
+  assert.throws(
+    () => assert.deepStrictEqual(date, fake),
+    {
+      message: 'Input A expected to strictly deep-equal input B:\n' +
+               '+ expected - actual\n\n- 2016-01-01T00:00:00.000Z\n+ Date {}'
+    }
+  );
+  assert.throws(
+    () => assert.deepStrictEqual(fake, date),
+    {
+      message: 'Input A expected to strictly deep-equal input B:\n' +
+               '+ expected - actual\n\n- Date {}\n+ 2016-01-01T00:00:00.000Z'
+    }
+  );
 }
 
 {  // At the moment global has its own type tag