assert: multiple improvements

1) Switched + / - and red / green in diffs. It seems like that style is more natural to most people. 2) Short primitives do not use the diff anymore. Especially short numbers can be read well like 1 !== 2. Cases that can not be displayed like that (e.g., -0 and +0) use the regular diff output. 3) Improved error descriptions. It was not always clear what the messages stood for. That should now be resolved. 4) Added a position indicator for single lines in case a tty is used and the line is shorter than the visual columns.
nodejs · BridgeAR · Jul 3, 2018 · Jul 14, 2018 · Jul 16, 2018 · Jul 27, 2018
commit 7589f421ca6a382d0a09baa2504badc2fb40493d
diff --git a/lib/internal/assert.js b/lib/internal/assert.js
@@ -10,13 +10,18 @@ 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 kReadableOperator = {
+  deepStrictEqual: 'Expected input to be strictly deep-equal',
+  notDeepStrictEqual: 'Expected "actual" not to be strictly deep-equal',
+  strictEqual: 'Expected input to be strictly equal',
+  notStrictEqual: 'Expected "actual" to be strictly unequal',
+  notIdentical: 'Input identical but not reference equal:',
 };
 
+// Comparing short primitives should just show === / !== instead of using the
+// diff.
+const kMaxShortLength = 10;
+
 function copyError(source) {
   const keys = Object.keys(source);
   const target = Object.create(Object.getPrototypeOf(source));
@@ -49,22 +54,46 @@ function inspectValue(val) {
 }
 
 function createErrDiff(actual, expected, operator) {
-  var other = '';
-  var res = '';
-  var lastPos = 0;
-  var end = '';
-  var skipped = false;
+  let other = '';
+  let res = '';
+  let lastPos = 0;
+  let end = '';
+  let skipped = false;
   const actualLines = inspectValue(actual);
   const expectedLines = inspectValue(expected);
-  const msg = READABLE_OPERATOR[operator] +
-        `:\n${green}+ expected${white} ${red}- actual${white}`;
+  const msg = kReadableOperator[operator] +
+        `:\n${green}+ actual${white} ${red}- expected${white}`;
   const skippedMsg = ` ${blue}...${white} Lines skipped`;
 
+  let i = 0;
+  let indicator = '';
+
+  if (actualLines.length === 1 && expectedLines.length === 1) {
+    const inputLength = actualLines[0].length + expectedLines[0].length;
+    if (inputLength <= kMaxShortLength) {
+      if ((typeof actual !== 'object' || actual === null) &&
+          (typeof expected !== 'object' || expected === null) &&
+          (actual !== 0 || expected !== 0)) { // -0 === +0
+        return `${kReadableOperator[operator]}:\n\n` +
+            `${actualLines[0]} !== ${expectedLines[0]}\n`;
+      }
+    } else if (process.stdout.isTTY &&
+               inputLength < process.stdout.columns &&
+               actualLines[0] !== expectedLines[0]) {
+      while (actualLines[0][i] === expectedLines[0][i]) {
+        i++;
+      }
+      // Add position indicator for the first mismatch in case it is a single
+      // line and the input length is less than the column length.
+      indicator = `\n  ${' '.repeat(i)}^`;
+      i = 0;
+    }
+  }
+
   // 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;
+  let a = actualLines[actualLines.length - 1];
+  let b = expectedLines[expectedLines.length - 1];
   while (a === b) {
     if (i++ < 2) {
       end = `\n  ${a}${end}`;
@@ -88,8 +117,8 @@ function createErrDiff(actual, expected, operator) {
   }
 
   const maxLines = Math.max(actualLines.length, expectedLines.length);
-  var printedLines = 0;
-  var identical = 0;
+  let printedLines = 0;
+  let identical = 0;
   for (i = 0; i < maxLines; i++) {
     // Only extra expected lines exist
     const cur = i - lastPos;
@@ -106,7 +135,7 @@ function createErrDiff(actual, expected, operator) {
         printedLines++;
       }
       lastPos = i;
-      other += `\n${green}+${white} ${expectedLines[i]}`;
+      other += `\n${red}-${white} ${expectedLines[i]}`;
       printedLines++;
     // Only extra actual lines exist
     } else if (expectedLines.length < i + 1) {
@@ -122,7 +151,7 @@ function createErrDiff(actual, expected, operator) {
         printedLines++;
       }
       lastPos = i;
-      res += `\n${red}-${white} ${actualLines[i]}`;
+      res += `\n${green}+${white} ${actualLines[i]}`;
       printedLines++;
     // Lines diverge
     } else if (actualLines[i] !== expectedLines[i]) {
@@ -138,8 +167,8 @@ function createErrDiff(actual, expected, operator) {
         printedLines++;
       }
       lastPos = i;
-      res += `\n${red}-${white} ${actualLines[i]}`;
-      other += `\n${green}+${white} ${expectedLines[i]}`;
+      res += `\n${green}+${white} ${actualLines[i]}`;
+      other += `\n${red}-${white} ${expectedLines[i]}`;
       printedLines += 2;
     // Lines are identical
     } else {
@@ -159,12 +188,8 @@ function createErrDiff(actual, expected, operator) {
   }
 
   // Strict equal with identical objects that are not identical by reference.
+  // E.g., assert.deepStrictEqual({ a: Symbol() }, { a: Symbol() })
   if (identical === maxLines) {
-    // E.g., assert.deepStrictEqual(Symbol(), Symbol())
-    const base = operator === 'strictEqual' ?
-      'Input objects identical but not reference equal:' :
-      'Input objects not identical:';
-
     // We have to get the result again. The lines were all removed before.
     const actualLines = inspectValue(actual);
 
@@ -177,9 +202,10 @@ function createErrDiff(actual, expected, operator) {
       }
     }
 
-    return `${base}\n\n${actualLines.join('\n')}\n`;
+    return `${kReadableOperator.notIdentical}\n\n${actualLines.join('\n')}\n`;
   }
-  return `${msg}${skipped ? skippedMsg : ''}\n${res}${other}${end}`;
+
+  return `${msg}${skipped ? skippedMsg : ''}\n${res}${other}${end}${indicator}`;
 }
 
 class AssertionError extends Error {
@@ -231,7 +257,7 @@ 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}:`;
+        const base = kReadableOperator[operator];
 
         // Only remove lines in case it makes sense to collapse those.
         // TODO: Accept env to always show the full error.

diff --git a/test/message/assert_throws_stack.out b/test/message/assert_throws_stack.out
@@ -2,13 +2,13 @@ assert.js:*
       throw err;
       ^
 
-AssertionError [ERR_ASSERTION]: Input A expected to strictly deep-equal input B:
-+ expected - actual
+AssertionError [ERR_ASSERTION]: Expected input to be strictly deep-equal:
++ actual - expected
 
-- Comparison {}
-+ Comparison {
-+   bar: true
-+ }
++ Comparison {}
+- Comparison {
+-   bar: true
+- }
     at Object.<anonymous> (*assert_throws_stack.js:*:*)
     at *
     at *

diff --git a/test/message/core_line_numbers.out b/test/message/core_line_numbers.out
@@ -3,7 +3,7 @@ punycode.js:42
 	^
 
 RangeError: Invalid input
-    at error (punycode.js:42:*)
+    at error (punycode.js:42:8)
     at Object.decode (punycode.js:*:*)
     at Object.<anonymous> (*test*message*core_line_numbers.js:*:*)
     at Module._compile (internal/modules/cjs/loader.js:*:*)

diff --git a/test/message/error_exit.out b/test/message/error_exit.out
@@ -3,11 +3,10 @@ assert.js:*
   throw new AssertionError(obj);
   ^
 
-AssertionError [ERR_ASSERTION]: Input A expected to strictly equal input B:
-+ expected - actual
+AssertionError [ERR_ASSERTION]: Expected input to be strictly equal:
+
+1 !== 2
 
-- 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:*:*)

diff --git a/test/parallel/test-assert-checktag.js b/test/parallel/test-assert-checktag.js
@@ -26,15 +26,15 @@ if (process.stdout.isTTY)
   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 {}'
+      message: 'Expected input to be strictly deep-equal:\n' +
+               '+ actual - expected\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'
+      message: 'Expected input to be strictly deep-equal:\n' +
+               '+ actual - expected\n\n+ Date {}\n- 2016-01-01T00:00:00.000Z'
     }
   );
 }