assert: print more lines in the error diff

BridgeAR · BridgeAR · commit 8157a5016131 · 2019-06-17T21:12:25.000+02:00
So far consequitive identical lines were collapsed if there were at least three. Now they are only collapsed from five identical lines on. This also simplifies the implementation a tiny bit by abstracting some logic. 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
@@ -122,7 +122,7 @@ function createErrDiff(actual, expected, operator) {
   let a = actualLines[actualLines.length - 1];
   let b = expectedLines[expectedLines.length - 1];
   while (a === b) {
-    if (i++ < 2) {
+    if (i++ < 3) {
       end = `\n  ${a}${end}`;
     } else {
       other = a;
@@ -154,7 +154,9 @@ function createErrDiff(actual, expected, operator) {
     return `${kReadableOperator.notIdentical}\n\n${actualLines.join('\n')}\n`;
   }
 
-  if (i > 3) {
+  // There were at least five identical lines at the end. Mark a couple of
+  // skipped.
+  if (i >= 5) {
     end = `\n${blue}...${white}${end}`;
     skipped = true;
   }
@@ -169,46 +171,46 @@ function createErrDiff(actual, expected, operator) {
         `\n${green}+ actual${white} ${red}- expected${white}`;
   const skippedMsg = ` ${blue}...${white} Lines skipped`;
 
+  let lines = actualLines;
+  let plusMinus = `${green}+${white}`;
+  let maxLength = expectedLines.length;
+  if (actualLines.length < maxLines) {
+    lines = expectedLines;
+    plusMinus = `${red}-${white}`;
+    maxLength = actualLines.length;
+  }
+
   for (i = 0; i < maxLines; i++) {
-    if (actualLines.length < i + 1) {
-      // 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 (maxLength < i + 1) {
+      // If more than two former lines are identical, print them. Collapse them
+      // in case more than five lines were identical.
+      if (identical > 2) {
         if (identical > 3) {
-          res += `\n${blue}...${white}`;
-          skipped = true;
-        } else if (identical > 2) {
-          res += `\n  ${expectedLines[i - 2]}`;
+          if (identical > 4) {
+            if (identical === 5) {
+              res += `\n  ${lines[i - 3]}`;
+              printedLines++;
+            } else {
+              res += `\n${blue}...${white}`;
+              skipped = true;
+            }
+          }
+          res += `\n  ${lines[i - 2]}`;
           printedLines++;
         }
-        res += `\n  ${expectedLines[i - 1]}`;
+        res += `\n  ${lines[i - 1]}`;
         printedLines++;
       }
       // 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 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 (identical > 2) {
-          res += `\n  ${actualLines[i - 2]}`;
-          printedLines++;
-        }
-        res += `\n  ${actualLines[i - 1]}`;
-        printedLines++;
+      if (lines === actualLines) {
+        res += `\n${plusMinus} ${lines[i]}`;
+      } else {
+        other += `\n${plusMinus} ${lines[i]}`;
       }
-      // No identical lines before.
-      identical = 0;
-      // Add the actual line to the result.
-      res += `\n${green}+${white} ${actualLines[i]}`;
       printedLines++;
+    // Only extra actual lines exist
     // Lines diverge
     } else {
       const expectedLine = expectedLines[i];
@@ -235,13 +237,19 @@ function createErrDiff(actual, expected, operator) {
         actualLine += ',';
       }
       if (divergingLines) {
-        // 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 more than two former lines are identical, print them. Collapse
+        // them in case more than five lines were identical.
+        if (identical > 2) {
           if (identical > 3) {
-            res += `\n${blue}...${white}`;
-            skipped = true;
-          } else if (identical > 2) {
+            if (identical > 4) {
+              if (identical === 5) {
+                res += `\n  ${actualLines[i - 3]}`;
+                printedLines++;
+              } else {
+                res += `\n${blue}...${white}`;
+                skipped = true;
+              }
+            }
             res += `\n  ${actualLines[i - 2]}`;
             printedLines++;
           }
@@ -264,7 +272,7 @@ function createErrDiff(actual, expected, operator) {
         identical++;
         // The very first identical line since the last diverging line is be
         // added to the result.
-        if (identical === 1) {
+        if (identical <= 2) {
           res += `\n  ${actualLine}`;
           printedLines++;
         }
diff --git a/test/parallel/test-assert-deep.js b/test/parallel/test-assert-deep.js
@@ -53,7 +53,7 @@ assert.throws(
     code: 'ERR_ASSERTION',
     message: `${defaultMsgStartFull} ... Lines skipped\n\n` +
              '+ Uint8Array [\n' +
-             '- Buffer [Uint8Array] [\n    120,\n...\n    10\n  ]'
+             '- Buffer [Uint8Array] [\n    120,\n...\n    122,\n    10\n  ]'
   }
 );
 assert.deepEqual(arr, buf);
@@ -66,9 +66,11 @@ assert.deepEqual(arr, buf);
     () => assert.deepStrictEqual(buf2, buf),
     {
       code: 'ERR_ASSERTION',
-      message: `${defaultMsgStartFull} ... Lines skipped\n\n` +
+      message: `${defaultMsgStartFull}\n\n` +
                '  Buffer [Uint8Array] [\n' +
-               '...\n' +
+               '    120,\n' +
+               '    121,\n' +
+               '    122,\n' +
                '    10,\n' +
                '+   prop: 1\n' +
                '  ]'
@@ -84,9 +86,11 @@ assert.deepEqual(arr, buf);
     () => assert.deepStrictEqual(arr, arr2),
     {
       code: 'ERR_ASSERTION',
-      message: `${defaultMsgStartFull} ... Lines skipped\n\n` +
+      message: `${defaultMsgStartFull}\n\n` +
                '  Uint8Array [\n' +
-               '...\n' +
+               '    120,\n' +
+               '    121,\n' +
+               '    122,\n' +
                '    10,\n' +
                '-   prop: 5\n' +
                '  ]'
@@ -932,17 +936,19 @@ assert.deepStrictEqual(obj1, obj2);
     ),
     {
       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" +
-               '  }'
+        '+ actual - expected ... Lines skipped\n' +
+        '\n' +
+        '  Comparison {\n' +
+        "    message: 'Expected values to be strictly deep-equal:\\n' +\n" +
+        '...\n' +
+        "      '  [TypeError: foo] {\\n' +\n" +
+        "      \"+   foo: 'bar'\\n\" +\n" +
+        "+     \"-   foo: 'baz.'\\n\" +\n" +
+        "-     \"-   foo: 'baz'\\n\" +\n" +
+        "      '  }',\n" +
+        "+   operator: 'deepStrictEqual'\n" +
+        "-   operator: 'throws'\n" +
+        '  }'
     }
   );
 }
diff --git a/test/parallel/test-assert.js b/test/parallel/test-assert.js
@@ -481,12 +481,14 @@ assert.throws(
     '',
     '  [',
     '    [',
-    '...',
+    '      [',
+    '        1,',
     '        2,',
     '+       3',
     "-       '3'",
     '      ]',
     '...',
+    '    4,',
     '    5',
     '  ]'].join('\n');
   assert.throws(
@@ -500,10 +502,12 @@ assert.throws(
     '  [',
     '    1,',
     '...',
+    '    1,',
     '    0,',
     '-   1,',
     '    1,',
     '...',
+    '    1,',
     '    1',
     '  ]'
   ].join('\n');
@@ -520,10 +524,11 @@ assert.throws(
     '  [',
     '    1,',
     '...',
+    '    1,',
     '    0,',
     '+   1,',
     '    1,',
-    '...',
+    '    1,',
     '    1',
     '  ]'
   ].join('\n');
diff --git a/test/pseudo-tty/test-assert-colors.js b/test/pseudo-tty/test-assert-colors.js
@@ -9,7 +9,7 @@ try {
   // active.
   process.env.TERM = 'FOOBAR';
   delete process.env.NODE_DISABLE_COLORS;
-  assert.deepStrictEqual([1, 2, 2, 2], [2, 2, 2, 2]);
+  assert.deepStrictEqual([1, 2, 2, 2, 2], [2, 2, 2, 2, 2]);
 } catch (err) {
   const expected = 'Expected values to be strictly deep-equal:\n' +
     '\u001b[32m+ actual\u001b[39m \u001b[31m- expected\u001b[39m' +
@@ -19,6 +19,7 @@ try {
     '\u001b[31m-\u001b[39m   2,\n' +
     '    2,\n' +
     '\u001b[34m...\u001b[39m\n' +
+    '    2,\n' +
     '    2\n' +
     '  ]';
   assert.strictEqual(err.message, expected);
