From 5aa3a2d172efb5cdca07ba6f0f2b8b054ced97ea Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Sat, 9 Dec 2017 19:38:20 -0200
Subject: [PATCH] assert: improve error messages

From now on all error messages produced by `assert` in strict mode
will produce a error diff.

Backport-PR-URL: https://github.com/nodejs/node/pull/19230
PR-URL: https://github.com/nodejs/node/pull/17615
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
---
 doc/api/assert.md            |  33 ++++++++
 lib/assert.js                |  16 +++-
 lib/internal/errors.js       | 150 ++++++++++++++++++++++++++++++++++-
 test/parallel/test-assert.js | 143 ++++++++++++++++++++++++++++++++-
 4 files changed, 332 insertions(+), 10 deletions(-)

diff --git a/doc/api/assert.md b/doc/api/assert.md
index d53bccd21a998d..68b01350dfdee6 100644
--- a/doc/api/assert.md
+++ b/doc/api/assert.md
@@ -17,6 +17,9 @@ For more information about the used equality comparisons see
 <!-- YAML
 added: REPLACEME
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/REPLACEME
+    description: Added error diffs to the strict mode
   - version: REPLACEME
     pr-url: https://github.com/nodejs/node/pull/17002
     description: Added strict mode to the assert module.
@@ -26,12 +29,42 @@ When using the `strict mode`, any `assert` function will use the equality used i
 the strict function mode. So [`assert.deepEqual()`][] will, for example, work the
 same as [`assert.deepStrictEqual()`][].
 
+On top of that, error messages which involve objects produce an error diff
+instead of displaying both objects. That is not the case for the legacy mode.
+
 It can be accessed using:
 
 ```js
 const assert = require('assert').strict;
 ```
 
+Example error diff (the `expected`, `actual`, and `Lines skipped` will be on a
+single row):
+
+```js
+const assert = require('assert').strict;
+
+assert.deepEqual([[[1, 2, 3]], 4, 5], [[[1, 2, '3']], 4, 5]);
+```
+
+```diff
+AssertionError [ERR_ASSERTION]: Input A expected to deepStrictEqual input B:
++ expected
+- actual
+... Lines skipped
+
+  [
+    [
+...
+      2,
+-     3
++     '3'
+    ],
+...
+    5
+  ]
+```
+
 ## Legacy mode
 
 > Stability: 0 - Deprecated: Use strict mode instead.
diff --git a/lib/assert.js b/lib/assert.js
index c3570afeb38a3a..48670de0c5458a 100644
--- a/lib/assert.js
+++ b/lib/assert.js
@@ -25,6 +25,10 @@ const { isDeepEqual, isDeepStrictEqual } =
 const errors = require('internal/errors');
 const { inspect } = require('util');
 
+const ERR_DIFF_DEACTIVATED = 0;
+const ERR_DIFF_NOT_EQUAL = 1;
+const ERR_DIFF_EQUAL = 2;
+
 // The assert module provides functions that throw
 // AssertionError's when particular conditions are not met. The
 // assert module must conform to the following interface.
@@ -154,7 +158,8 @@ assert.deepStrictEqual = function deepStrictEqual(actual, expected, message) {
       expected,
       message,
       operator: 'deepStrictEqual',
-      stackStartFn: deepStrictEqual
+      stackStartFn: deepStrictEqual,
+      errorDiff: this === strict ? ERR_DIFF_EQUAL : ERR_DIFF_DEACTIVATED
     });
   }
 };
@@ -167,7 +172,8 @@ function notDeepStrictEqual(actual, expected, message) {
       expected,
       message,
       operator: 'notDeepStrictEqual',
-      stackStartFn: notDeepStrictEqual
+      stackStartFn: notDeepStrictEqual,
+      errorDiff: this === strict ? ERR_DIFF_NOT_EQUAL : ERR_DIFF_DEACTIVATED
     });
   }
 }
@@ -180,7 +186,8 @@ assert.strictEqual = function strictEqual(actual, expected, message) {
       expected,
       message,
       operator: '===',
-      stackStartFn: strictEqual
+      stackStartFn: strictEqual,
+      errorDiff: this === strict ? ERR_DIFF_EQUAL : ERR_DIFF_DEACTIVATED
     });
   }
 };
@@ -194,7 +201,8 @@ assert.notStrictEqual = function notStrictEqual(actual, expected, message) {
       expected,
       message,
       operator: '!==',
-      stackStartFn: notStrictEqual
+      stackStartFn: notStrictEqual,
+      errorDiff: this === strict ? ERR_DIFF_NOT_EQUAL : ERR_DIFF_DEACTIVATED
     });
   }
 };
diff --git a/lib/internal/errors.js b/lib/internal/errors.js
index 1d927df397b953..02802746951fad 100644
--- a/lib/internal/errors.js
+++ b/lib/internal/errors.js
@@ -78,13 +78,131 @@ function makeNodeError(Base) {
   };
 }
 
+function createErrDiff(actual, expected, operator) {
+  var other = '';
+  var res = '';
+  var lastPos = 0;
+  var end = '';
+  var skipped = false;
+  const util = lazyUtil();
+  const actualLines = util
+    .inspect(actual, { compact: false }).split('\n');
+  const expectedLines = util
+    .inspect(expected, { compact: false }).split('\n');
+  const msg = `Input A expected to ${operator} input B:\n` +
+        '\u001b[32m+ expected\u001b[39m \u001b[31m- actual\u001b[39m';
+  const skippedMsg = ' ... Lines skipped';
+
+  // Remove all ending lines that match (this optimizes the output for
+  // readability by reducing the number of total changed lines).
+  var a = actualLines[actualLines.length - 1];
+  var b = expectedLines[expectedLines.length - 1];
+  var i = 0;
+  while (a === b) {
+    if (i++ < 2) {
+      end = `\n  ${a}${end}`;
+    } else {
+      other = a;
+    }
+    actualLines.pop();
+    expectedLines.pop();
+    a = actualLines[actualLines.length - 1];
+    b = expectedLines[expectedLines.length - 1];
+  }
+  if (i > 3) {
+    end = `\n...${end}`;
+    skipped = true;
+  }
+  if (other !== '') {
+    end = `\n  ${other}${end}`;
+    other = '';
+  }
+
+  const maxLines = Math.max(actualLines.length, expectedLines.length);
+  var printedLines = 0;
+  for (i = 0; i < maxLines; i++) {
+    // Only extra expected lines exist
+    const cur = i - lastPos;
+    if (actualLines.length < i + 1) {
+      if (cur > 1 && i > 2) {
+        if (cur > 4) {
+          res += '\n...';
+          skipped = true;
+        } else if (cur > 3) {
+          res += `\n  ${expectedLines[i - 2]}`;
+          printedLines++;
+        }
+        res += `\n  ${expectedLines[i - 1]}`;
+        printedLines++;
+      }
+      lastPos = i;
+      other += `\n\u001b[32m+\u001b[39m ${expectedLines[i]}`;
+      printedLines++;
+    // Only extra actual lines exist
+    } else if (expectedLines.length < i + 1) {
+      if (cur > 1 && i > 2) {
+        if (cur > 4) {
+          res += '\n...';
+          skipped = true;
+        } else if (cur > 3) {
+          res += `\n  ${actualLines[i - 2]}`;
+          printedLines++;
+        }
+        res += `\n  ${actualLines[i - 1]}`;
+        printedLines++;
+      }
+      lastPos = i;
+      res += `\n\u001b[31m-\u001b[39m ${actualLines[i]}`;
+      printedLines++;
+    // Lines diverge
+    } else if (actualLines[i] !== expectedLines[i]) {
+      if (cur > 1 && i > 2) {
+        if (cur > 4) {
+          res += '\n...';
+          skipped = true;
+        } else if (cur > 3) {
+          res += `\n  ${actualLines[i - 2]}`;
+          printedLines++;
+        }
+        res += `\n  ${actualLines[i - 1]}`;
+        printedLines++;
+      }
+      lastPos = i;
+      res += `\n\u001b[31m-\u001b[39m ${actualLines[i]}`;
+      other += `\n\u001b[32m+\u001b[39m ${expectedLines[i]}`;
+      printedLines += 2;
+    // Lines are identical
+    } else {
+      res += other;
+      other = '';
+      if (cur === 1 || i === 0) {
+        res += `\n  ${actualLines[i]}`;
+        printedLines++;
+      }
+    }
+    // Inspected object to big (Show ~20 rows max)
+    if (printedLines > 20 && i < maxLines - 2) {
+      return `${msg}${skippedMsg}\n${res}\n...${other}\n...`;
+    }
+  }
+  return `${msg}${skipped ? skippedMsg : ''}\n${res}${other}${end}`;
+}
+
 class AssertionError extends Error {
   constructor(options) {
     if (typeof options !== 'object' || options === null) {
       throw new exports.TypeError('ERR_INVALID_ARG_TYPE', 'options', 'Object');
     }
-    var { actual, expected, message, operator, stackStartFn } = options;
-    if (message) {
+    var {
+      actual,
+      expected,
+      message,
+      operator,
+      stackStartFn,
+      errorDiff = 0
+    } = options;
+
+    if (message != null) {
       super(message);
     } else {
       const util = lazyUtil();
@@ -92,8 +210,32 @@ class AssertionError extends Error {
         actual = `${actual.name}: ${actual.message}`;
       if (expected && expected.stack && expected instanceof Error)
         expected = `${expected.name}: ${expected.message}`;
-      super(`${util.inspect(actual).slice(0, 128)} ` +
-        `${operator} ${util.inspect(expected).slice(0, 128)}`);
+
+      if (errorDiff === 0) {
+        let res = util.inspect(actual);
+        let other = util.inspect(expected);
+        if (res.length > 128)
+          res = `${res.slice(0, 125)}...`;
+        if (other.length > 128)
+          other = `${other.slice(0, 125)}...`;
+        super(`${res} ${operator} ${other}`);
+      } else if (errorDiff === 1) {
+        // In case the objects are equal but the operator requires unequal, show
+        // the first object and say A equals B
+        const res = util
+          .inspect(actual, { compact: false }).split('\n');
+
+        if (res.length > 20) {
+          res[19] = '...';
+          while (res.length > 20) {
+            res.pop();
+          }
+        }
+        // Only print a single object.
+        super(`Identical input passed to ${operator}:\n${res.join('\n')}`);
+      } else {
+        super(createErrDiff(actual, expected, operator));
+      }
     }
 
     this.generatedMessage = !message;
diff --git a/test/parallel/test-assert.js b/test/parallel/test-assert.js
index 462c01425dab5b..fa4ee21a6bd202 100644
--- a/test/parallel/test-assert.js
+++ b/test/parallel/test-assert.js
@@ -710,7 +710,8 @@ assert.throws(() => {
   assert.strictEqual('A'.repeat(1000), '');
 }, common.expectsError({
   code: 'ERR_ASSERTION',
-  message: new RegExp(`^'${'A'.repeat(127)} === ''$`) }));
+  message: /^'A{124}\.\.\. === ''$/
+}));
 
 {
   // bad args to AssertionError constructor should throw TypeError
@@ -751,7 +752,6 @@ common.expectsError(
   assert.equal(assert.notEqual, assert.notStrictEqual);
   assert.equal(assert.notDeepEqual, assert.notDeepStrictEqual);
   assert.equal(Object.keys(assert).length, Object.keys(a).length);
-  /* eslint-enable no-restricted-properties */
   assert(7);
   common.expectsError(
     () => assert(),
@@ -761,6 +761,145 @@ common.expectsError(
       message: 'undefined == true'
     }
   );
+
+  // Test error diffs
+  const start = 'Input A expected to deepStrictEqual input B:';
+  const actExp = '\u001b[32m+ expected\u001b[39m \u001b[31m- actual\u001b[39m';
+  const plus = '\u001b[32m+\u001b[39m';
+  const minus = '\u001b[31m-\u001b[39m';
+  let message = [
+    start,
+    `${actExp} ... Lines skipped`,
+    '',
+    '  [',
+    '    [',
+    '...',
+    '        2,',
+    `${minus}       3`,
+    `${plus}       '3'`,
+    '      ]',
+    '...',
+    '    5',
+    '  ]'].join('\n');
+  assert.throws(
+    () => assert.deepEqual([[[1, 2, 3]], 4, 5], [[[1, 2, '3']], 4, 5]),
+    { message });
+
+  message = [
+    start,
+    `${actExp} ... Lines skipped`,
+    '',
+    '  [',
+    '    1,',
+    '...',
+    '    0,',
+    `${plus}   1,`,
+    '    1,',
+    '...',
+    '    1',
+    '  ]'
+  ].join('\n');
+  assert.throws(
+    () => assert.deepEqual(
+      [1, 1, 1, 1, 1, 1, 1, 1, 0, 1, 1, 1, 1],
+      [1, 1, 1, 1, 1, 1, 1, 1, 0, 1, 1, 1, 1, 1]),
+    { message });
+
+  message = [
+    start,
+    `${actExp} ... Lines skipped`,
+    '',
+    '  [',
+    '    1,',
+    '...',
+    '    0,',
+    `${minus}   1,`,
+    '    1,',
+    '...',
+    '    1',
+    '  ]'
+  ].join('\n');
+  assert.throws(
+    () => assert.deepEqual(
+      [1, 1, 1, 1, 1, 1, 1, 1, 0, 1, 1, 1, 1],
+      [1, 1, 1, 1, 1, 1, 1, 1, 0, 1, 1, 1]),
+    { message });
+
+  message = [
+    start,
+    actExp,
+    '',
+    '  [',
+    '    1,',
+    `${minus}   2,`,
+    `${plus}   1,`,
+    '    1,',
+    '    1,',
+    '    0,',
+    `${minus}   1,`,
+    '    1',
+    '  ]'
+  ].join('\n');
+  assert.throws(
+    () => assert.deepEqual(
+      [1, 2, 1, 1, 0, 1, 1],
+      [1, 1, 1, 1, 0, 1]),
+    { message });
+
+  message = [
+    start,
+    actExp,
+    '',
+    `${minus} [`,
+    `${minus}   1,`,
+    `${minus}   2,`,
+    `${minus}   1`,
+    `${minus} ]`,
+    `${plus} undefined`,
+  ].join('\n');
+  assert.throws(
+    () => assert.deepEqual([1, 2, 1]),
+    { message });
+
+  message = [
+    start,
+    actExp,
+    '',
+    '  [',
+    `${minus}   1,`,
+    '    2,',
+    '    1',
+    '  ]'
+  ].join('\n');
+  assert.throws(
+    () => assert.deepEqual([1, 2, 1], [2, 1]),
+    { message });
+
+  message = `${start}\n` +
+    `${actExp} ... Lines skipped\n` +
+    '\n' +
+    '  [\n' +
+    `${minus}   1,\n`.repeat(10) +
+    '...\n' +
+    `${plus}   2,\n`.repeat(10) +
+    '...';
+  assert.throws(
+    () => assert.deepEqual(Array(12).fill(1), Array(12).fill(2)),
+    { message });
+
+  // notDeepEqual tests
+  message = 'Identical input passed to notDeepStrictEqual:\n[\n  1\n]';
+  assert.throws(
+    () => assert.notDeepEqual([1], [1]),
+    { message });
+
+  message = 'Identical input passed to notDeepStrictEqual:' +
+        `\n[${'\n  1,'.repeat(18)}\n...`;
+  const data = Array(21).fill(1);
+  assert.throws(
+    () => assert.notDeepEqual(data, data),
+    { message });
+  /* eslint-enable no-restricted-properties */
 }
 
 common.expectsError(