assert: fix error diff

BridgeAR · BridgeAR · commit 82174412a522 · 2019-06-17T21:12:25.000+02:00
In some edge cases an identical line could be printed twice. This is now fixed by changing the algorithm a bit. It will now verify how many lines were identical before the current one. PR-URL: #28058 Reviewed-By: Rich Trott <rtrott@gmail.com>
diff --git a/lib/internal/assert/assertion_error.js b/lib/internal/assert/assertion_error.js
@@ -52,25 +52,18 @@ function inspectValue(val) {
       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 for now.
-      // TODO(BridgeAR): `breakLength` should be limited as soon as soon as we
-      // have meta information about the inspected properties (i.e., know where
-      // in what line the property starts and ends).
-      breakLength: Infinity,
       // Assert does not detect proxies currently.
       showProxy: false,
       sorted: true,
       // Inspect getters as we also check them when comparing entries.
-      getters: true
+      getters: true,
     }
   );
 }
 
 function createErrDiff(actual, expected, operator) {
   let other = '';
   let res = '';
-  let lastPos = 0;
   let end = '';
   let skipped = false;
   const actualInspected = inspectValue(actual);
@@ -171,51 +164,48 @@ function createErrDiff(actual, expected, operator) {
   }
 
   let printedLines = 0;
+  let identical = 0;
   const msg = kReadableOperator[operator] +
         `\n${green}+ actual${white} ${red}- expected${white}`;
   const skippedMsg = ` ${blue}...${white} Lines skipped`;
 
   for (i = 0; i < maxLines; i++) {
-    // Only extra expected lines exist
-    const cur = i - lastPos;
     if (actualLines.length < i + 1) {
-      // If the last diverging line is more than one line above and the
-      // current line is at least line three, add some of the former lines and
-      // also add dots to indicate skipped entries.
-      if (cur > 1 && i > 2) {
-        if (cur > 4) {
+      // If more than one former line is identical, print that. Collapse those
+      // in case more than three lines before were identical.
+      if (identical > 1) {
+        if (identical > 3) {
           res += `\n${blue}...${white}`;
           skipped = true;
-        } else if (cur > 3) {
+        } else if (identical > 2) {
           res += `\n  ${expectedLines[i - 2]}`;
           printedLines++;
         }
         res += `\n  ${expectedLines[i - 1]}`;
         printedLines++;
       }
-      // Mark the current line as the last diverging one.
-      lastPos = i;
+      // No identical lines before.
+      identical = 0;
       // Add the expected line to the cache.
       other += `\n${red}-${white} ${expectedLines[i]}`;
       printedLines++;
     // Only extra actual lines exist
     } else if (expectedLines.length < i + 1) {
-      // If the last diverging line is more than one line above and the
-      // current line is at least line three, add some of the former lines and
-      // also add dots to indicate skipped entries.
-      if (cur > 1 && i > 2) {
-        if (cur > 4) {
+      // If more than one former line is identical, print that. Collapse those
+      // in case more than three lines before were identical.
+      if (identical > 1) {
+        if (identical > 3) {
           res += `\n${blue}...${white}`;
           skipped = true;
-        } else if (cur > 3) {
+        } else if (identical > 2) {
           res += `\n  ${actualLines[i - 2]}`;
           printedLines++;
         }
         res += `\n  ${actualLines[i - 1]}`;
         printedLines++;
       }
-      // Mark the current line as the last diverging one.
-      lastPos = i;
+      // No identical lines before.
+      identical = 0;
       // Add the actual line to the result.
       res += `\n${green}+${white} ${actualLines[i]}`;
       printedLines++;
@@ -245,22 +235,21 @@ function createErrDiff(actual, expected, operator) {
         actualLine += ',';
       }
       if (divergingLines) {
-        // If the last diverging line is more than one line above and the
-        // current line is at least line three, add some of the former lines and
-        // also add dots to indicate skipped entries.
-        if (cur > 1 && i > 2) {
-          if (cur > 4) {
+        // If more than one former line is identical, print that. Collapse those
+        // in case more than three lines before were identical.
+        if (identical > 1) {
+          if (identical > 3) {
             res += `\n${blue}...${white}`;
             skipped = true;
-          } else if (cur > 3) {
+          } else if (identical > 2) {
             res += `\n  ${actualLines[i - 2]}`;
             printedLines++;
           }
           res += `\n  ${actualLines[i - 1]}`;
           printedLines++;
         }
-        // Mark the current line as the last diverging one.
-        lastPos = i;
+        // No identical lines before.
+        identical = 0;
         // Add the actual line to the result and cache the expected diverging
         // line so consecutive diverging lines show up as +++--- and not +-+-+-.
         res += `\n${green}+${white} ${actualLine}`;
@@ -272,9 +261,10 @@ function createErrDiff(actual, expected, operator) {
         // and reset the cache.
         res += other;
         other = '';
-        // If the last diverging line is exactly one line above or if it is the
-        // very first line, add the line to the result.
-        if (cur === 1 || i === 0) {
+        identical++;
+        // The very first identical line since the last diverging line is be
+        // added to the result.
+        if (identical === 1) {
           res += `\n  ${actualLine}`;
           printedLines++;
         }
@@ -316,7 +306,7 @@ class AssertionError extends Error {
       if (process.stderr.isTTY) {
         // Reset on each call to make sure we handle dynamically set environment
         // variables correct.
-        if (process.stderr.getColorDepth() !== 1) {
+        if (process.stderr.hasColors()) {
           blue = '\u001b[34m';
           green = '\u001b[32m';
           white = '\u001b[39m';
diff --git a/test/parallel/test-assert-deep.js b/test/parallel/test-assert-deep.js
@@ -68,7 +68,6 @@ assert.deepEqual(arr, buf);
       code: 'ERR_ASSERTION',
       message: `${defaultMsgStartFull} ... Lines skipped\n\n` +
                '  Buffer [Uint8Array] [\n' +
-               '    120,\n' +
                '...\n' +
                '    10,\n' +
                '+   prop: 1\n' +
@@ -87,7 +86,6 @@ assert.deepEqual(arr, buf);
       code: 'ERR_ASSERTION',
       message: `${defaultMsgStartFull} ... Lines skipped\n\n` +
                '  Uint8Array [\n' +
-               '    120,\n' +
                '...\n' +
                '    10,\n' +
                '-   prop: 5\n' +
@@ -921,13 +919,30 @@ assert.deepStrictEqual(obj1, obj2);
   const a = new TypeError('foo');
   const b = new TypeError('foo');
   a.foo = 'bar';
-  b.foo = 'baz';
+  b.foo = 'baz.';
 
   assert.throws(
-    () => assert.deepStrictEqual(a, b),
+    () => assert.throws(
+      () => assert.deepStrictEqual(a, b),
+      {
+        operator: 'throws',
+        message: `${defaultMsgStartFull}\n\n` +
+                '  [TypeError: foo] {\n+   foo: \'bar\'\n-   foo: \'baz\'\n  }',
+      }
+    ),
     {
-      message: `${defaultMsgStartFull}\n\n` +
-               '  [TypeError: foo] {\n+   foo: \'bar\'\n-   foo: \'baz\'\n  }'
+      message: 'Expected values to be strictly deep-equal:\n' +
+               '+ actual - expected ... Lines skipped\n' +
+               '\n' +
+               '  Comparison {\n' +
+               '...\n' +
+               "      \"+   foo: 'bar'\\n\" +\n" +
+               "+     \"-   foo: 'baz.'\\n\" +\n" +
+               "-     \"-   foo: 'baz'\\n\" +\n" +
+               "      '  }',\n" +
+               "+   operator: 'deepStrictEqual'\n" +
+               "-   operator: 'throws'\n" +
+               '  }'
     }
   );
 }
