assert: refine assertion message

BridgeAR · targos · commit 3367bad08038 · 2019-05-07T20:37:38.000+02:00
This makes sure that the error message is more appropriate than before by checking closer what operator is used and which is not. It also increases the total number of lines printed to the user. PR-URL: #27525 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>
diff --git a/lib/internal/assert/assertion_error.js b/lib/internal/assert/assertion_error.js
@@ -17,19 +17,18 @@ const kReadableOperator = {
   strictEqual: 'Expected values to be strictly equal:',
   strictEqualObject: 'Expected "actual" to be reference-equal to "expected":',
   deepEqual: 'Expected values to be loosely deep-equal:',
-  equal: 'Expected values to be loosely equal:',
   notDeepStrictEqual: 'Expected "actual" not to be strictly deep-equal to:',
   notStrictEqual: 'Expected "actual" to be strictly unequal to:',
   notStrictEqualObject:
     'Expected "actual" not to be reference-equal to "expected":',
   notDeepEqual: 'Expected "actual" not to be loosely deep-equal to:',
-  notEqual: 'Expected "actual" to be loosely unequal to:',
   notIdentical: 'Values identical but not reference-equal:',
+  notDeepEqualUnequal: 'Expected values not to be loosely deep-equal:'
 };
 
 // Comparing short primitives should just show === / !== instead of using the
 // diff.
-const kMaxShortLength = 10;
+const kMaxShortLength = 12;
 
 function copyError(source) {
   const keys = Object.keys(source);
@@ -81,13 +80,12 @@ function createErrDiff(actual, expected, operator) {
   let i = 0;
   let indicator = '';
 
-  // In case both values are objects explicitly mark them as not reference equal
-  // for the `strictEqual` operator.
+  // In case both values are objects or functions explicitly mark them as not
+  // reference equal for the `strictEqual` operator.
   if (operator === 'strictEqual' &&
-      typeof actual === 'object' &&
-      typeof expected === 'object' &&
-      actual !== null &&
-      expected !== null) {
+      ((typeof actual === 'object' && actual !== null &&
+        typeof expected === 'object' && expected !== null) ||
+       (typeof actual === 'function' && typeof expected === 'function'))) {
     operator = 'strictEqualObject';
   }
 
@@ -153,9 +151,9 @@ function createErrDiff(actual, expected, operator) {
 
     // 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] = `${blue}...${white}`;
-      while (actualLines.length > 27) {
+    if (actualLines.length > 50) {
+      actualLines[46] = `${blue}...${white}`;
+      while (actualLines.length > 47) {
         actualLines.pop();
       }
     }
@@ -282,8 +280,8 @@ function createErrDiff(actual, expected, operator) {
         }
       }
     }
-    // Inspected object to big (Show ~20 rows max)
-    if (printedLines > 20 && i < maxLines - 2) {
+    // Inspected object to big (Show ~50 rows max)
+    if (printedLines > 50 && i < maxLines - 2) {
       return `${msg}${skippedMsg}\n${res}\n${blue}...${white}${other}\n` +
              `${blue}...${white}`;
     }
@@ -348,52 +346,61 @@ class AssertionError extends Error {
         let base = kReadableOperator[operator];
         const res = inspectValue(actual).split('\n');
 
-        // In case "actual" is an object, it should not be reference equal.
+        // In case "actual" is an object or a function, it should not be
+        // reference equal.
         if (operator === 'notStrictEqual' &&
-            typeof actual === 'object' &&
-            actual !== null) {
+            (typeof actual === 'object' && actual !== null ||
+             typeof actual === 'function')) {
           base = kReadableOperator.notStrictEqualObject;
         }
 
         // 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] = `${blue}...${white}`;
-          while (res.length > 27) {
+        if (res.length > 50) {
+          res[46] = `${blue}...${white}`;
+          while (res.length > 47) {
             res.pop();
           }
         }
 
         // Only print a single input.
         if (res.length === 1) {
-          super(`${base} ${res[0]}`);
+          super(`${base}${res[0].length > 5 ? '\n\n' : ' '}${res[0]}`);
         } else {
           super(`${base}\n\n${res.join('\n')}\n`);
         }
       } else {
         let res = inspectValue(actual);
-        let other = '';
+        let other = inspectValue(expected);
         const knownOperators = kReadableOperator[operator];
-        if (operator === 'notDeepEqual' || operator === 'notEqual') {
-          res = `${kReadableOperator[operator]}\n\n${res}`;
+        if ((operator === 'notDeepEqual' || operator === 'notEqual') &&
+            res === other) {
+          res = `${knownOperators}\n\n${res}`;
           if (res.length > 1024) {
             res = `${res.slice(0, 1021)}...`;
           }
+          super(res);
         } else {
-          other = `${inspectValue(expected)}`;
           if (res.length > 512) {
             res = `${res.slice(0, 509)}...`;
           }
           if (other.length > 512) {
             other = `${other.slice(0, 509)}...`;
           }
           if (operator === 'deepEqual' || operator === 'equal') {
-            res = `${knownOperators}\n\n${res}\n\nshould equal\n\n`;
+            const eq = operator === 'deepEqual' ? 'deep-equal' : 'equal';
+            res = `${knownOperators}\n\n${res}\n\nshould loosely ${eq}\n\n`;
           } else {
-            other = ` ${operator} ${other}`;
+            const newOperator = kReadableOperator[`${operator}Unequal`];
+            if (newOperator) {
+              const eq = operator === 'notDeepEqual' ? 'deep-equal' : 'equal';
+              res = `${newOperator}\n\n${res}\n\nshould not loosely ${eq}\n\n`;
+            } else {
+              other = ` ${operator} ${other}`;
+            }
           }
+          super(`${res}${other}`);
         }
-        super(`${res}${other}`);
       }
     }
 
diff --git a/test/parallel/test-assert-deep.js b/test/parallel/test-assert-deep.js
@@ -195,7 +195,7 @@ function assertDeepAndStrictEqual(a, b) {
 function assertNotDeepOrStrict(a, b, err) {
   assert.throws(
     () => assert.deepEqual(a, b),
-    err || re`${a}\n\nshould equal\n\n${b}`
+    err || re`${a}\n\nshould loosely deep-equal\n\n${b}`
   );
   assert.throws(
     () => assert.deepStrictEqual(a, b),
@@ -204,7 +204,7 @@ function assertNotDeepOrStrict(a, b, err) {
 
   assert.throws(
     () => assert.deepEqual(b, a),
-    err || re`${b}\n\nshould equal\n\n${a}`
+    err || re`${b}\n\nshould loosely deep-equal\n\n${a}`
   );
   assert.throws(
     () => assert.deepStrictEqual(b, a),
@@ -651,6 +651,20 @@ assertDeepAndStrictEqual(-0, -0);
   assertDeepAndStrictEqual(a, b);
 }
 
+assert.throws(
+  () => assert.notDeepEqual(1, true),
+  {
+    message: /1\n\nshould not loosely deep-equal\n\ntrue/
+  }
+);
+
+assert.throws(
+  () => assert.notDeepEqual(1, 1),
+  {
+    message: /Expected "actual" not to be loosely deep-equal to:\n\n1/
+  }
+);
+
 assert.deepEqual(new Date(2000, 3, 14), new Date(2000, 3, 14));
 
 assert.throws(() => { assert.deepEqual(new Date(), new Date(2000, 3, 14)); },
@@ -779,7 +793,7 @@ assert.throws(
   () => assert.notDeepStrictEqual(new Date(2000, 3, 14), new Date(2000, 3, 14)),
   {
     name: 'AssertionError',
-    message: 'Expected "actual" not to be strictly deep-equal to: ' +
+    message: 'Expected "actual" not to be strictly deep-equal to:\n\n' +
              util.inspect(new Date(2000, 3, 14))
   }
 );
@@ -815,11 +829,11 @@ assert.throws(
     message: `${defaultMsgStartFull}\n\n+ /a/m\n- /a/`
   });
 assert.throws(
-  () => assert.deepStrictEqual(/a/igm, /a/im),
+  () => assert.deepStrictEqual(/aa/igm, /aa/im),
   {
     code: 'ERR_ASSERTION',
     name: 'AssertionError',
-    message: `${defaultMsgStartFull}\n\n+ /a/gim\n- /a/im\n     ^`
+    message: `${defaultMsgStartFull}\n\n+ /aa/gim\n- /aa/im\n      ^`
   });
 
 {
@@ -939,12 +953,12 @@ assert.deepStrictEqual(obj1, obj2);
 }
 
 // Strict equal with identical objects that are not identical
-// by reference and longer than 30 elements
+// by reference and longer than 50 elements
 // E.g., assert.deepStrictEqual({ a: Symbol() }, { a: Symbol() })
 {
   const a = {};
   const b = {};
-  for (let i = 0; i < 35; i++) {
+  for (let i = 0; i < 55; i++) {
     a[`symbol${i}`] = Symbol();
     b[`symbol${i}`] = Symbol();
   }
diff --git a/test/parallel/test-assert.js b/test/parallel/test-assert.js
@@ -80,12 +80,20 @@ assert.throws(
 assert.throws(
   () => a.notStrictEqual('a '.repeat(30), 'a '.repeat(30)),
   {
-    message: 'Expected "actual" to be strictly unequal to: ' +
+    message: 'Expected "actual" to be strictly unequal to:\n\n' +
              `'${'a '.repeat(30)}'`,
     name: 'AssertionError'
   }
 );
 
+assert.throws(
+  () => a.notEqual(1, 1),
+  {
+    message: '1 != 1',
+    operator: '!='
+  }
+);
+
 a.notStrictEqual(2, '2');
 
 // Testing the throwing.
@@ -281,15 +289,15 @@ testShortAssertionMessage('a', '"a"');
 testShortAssertionMessage('foo', '\'foo\'');
 testShortAssertionMessage(0, '0');
 testShortAssertionMessage(Symbol(), 'Symbol()');
+testShortAssertionMessage(undefined, 'undefined');
+testShortAssertionMessage(-Infinity, '-Infinity');
+testShortAssertionMessage(function() {}, '[Function]');
 testAssertionMessage([], '[]');
 testAssertionMessage(/a/, '/a/');
 testAssertionMessage(/abc/gim, '/abc/gim');
 testAssertionMessage({}, '{}');
-testAssertionMessage(undefined, 'undefined');
-testAssertionMessage(-Infinity, '-Infinity');
 testAssertionMessage([1, 2, 3], '[\n+   1,\n+   2,\n+   3\n+ ]');
 testAssertionMessage(function f() {}, '[Function: f]');
-testAssertionMessage(function() {}, '[Function]');
 testAssertionMessage(circular, '{\n+   x: [Circular],\n+   y: 1\n+ }');
 testAssertionMessage({ a: undefined, b: null },
                      '{\n+   a: undefined,\n+   b: null\n+ }');
@@ -579,12 +587,12 @@ assert.throws(
     `${actExp} ... Lines skipped\n` +
     '\n' +
     '  [\n' +
-    '+   1,\n'.repeat(10) +
+    '+   1,\n'.repeat(25) +
     '...\n' +
-    '-   2,\n'.repeat(10) +
+    '-   2,\n'.repeat(25) +
     '...';
   assert.throws(
-    () => assert.deepEqual(Array(12).fill(1), Array(12).fill(2)),
+    () => assert.deepEqual(Array(28).fill(1), Array(28).fill(2)),
     { message });
 
   const obj1 = {};
@@ -612,8 +620,8 @@ assert.throws(
   );
 
   message = 'Expected "actual" not to be strictly deep-equal to:' +
-            `\n\n[${'\n  1,'.repeat(25)}\n...\n`;
-  const data = Array(31).fill(1);
+            `\n\n[${'\n  1,'.repeat(45)}\n...\n`;
+  const data = Array(51).fill(1);
   assert.throws(
     () => assert.notDeepEqual(data, data),
     { message });
diff --git a/test/pseudo-tty/test-assert-position-indicator.js b/test/pseudo-tty/test-assert-position-indicator.js
@@ -13,6 +13,6 @@ assert.throws(
 
 // Confirm that there is a position indicator.
 assert.throws(
-  () => { assert.deepStrictEqual('aaa', 'aaaa'); },
+  () => { assert.deepStrictEqual('aaaa', 'aaaaa'); },
   (err) => err.message.includes('^')
 );
