assert: improve the strict equal messages

In case reference (un)equal objects fail in assertion, it should be
clear that it is about the reference equality and not about the
object properties. This is fixed by improving the message in such
cases.

Refs: https://github.com/nodejs/node/issues/22763

PR-URL: https://github.com/nodejs/node/pull/23056
Refs: https://github.com/nodejs/node/issues/22763
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

lib/internal/assert.js

@@ -13,10 +13,13 @@ let white = '';
 const kReadableOperator = {
   deepStrictEqual: 'Expected inputs to be strictly deep-equal:',
   strictEqual: 'Expected inputs to be strictly equal:',
+  strictEqualObject: 'Expected "actual" to be reference-equal to "expected":',
   deepEqual: 'Expected inputs to be loosely deep-equal:',
   equal: 'Expected inputs to be loosely equal:',
   notDeepStrictEqual: 'Expected "actual" not to be strictly deep-equal to:',
   notStrictEqual: 'Expected "actual" to be strictly unequal to:',
+  // eslint-disable-next-line max-len
+  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: 'Inputs identical but not reference equal:',
@@ -67,13 +70,20 @@ function createErrDiff(actual, expected, operator) {
   const actualInspected = inspectValue(actual);
   const actualLines = actualInspected.split('\n');
   const expectedLines = inspectValue(expected).split('\n');
-  const msg = kReadableOperator[operator] +
-        `\n${green}+ actual${white} ${red}- expected${white}`;
-  const skippedMsg = ` ${blue}...${white} Lines skipped`;
 
   let i = 0;
   let indicator = '';
 
+  // In case both inputs are objects explicitly mark them as not reference equal
+  // for the `strictEqual` operator.
+  if (operator === 'strictEqual' &&
+      typeof actual === 'object' &&
+      typeof expected === 'object' &&
+      actual !== null &&
+      expected !== null) {
+    operator = 'strictEqualObject';
+  }
+
   // If "actual" and "expected" fit on a single line and they are not strictly
   // equal, check further special handling.
   if (actualLines.length === 1 && expectedLines.length === 1 &&
@@ -89,7 +99,7 @@ function createErrDiff(actual, expected, operator) {
         return `${kReadableOperator[operator]}\n\n` +
             `${actualLines[0]} !== ${expectedLines[0]}\n`;
       }
-    } else {
+    } else if (operator !== 'strictEqualObject') {
       // If the stderr is a tty and the input length is lower than the current
       // columns per line, add a mismatch indicator below the output. If it is
       // not a tty, use a default value of 80 characters.
@@ -156,6 +166,10 @@ function createErrDiff(actual, expected, operator) {
   }
 
   let printedLines = 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;
@@ -274,8 +288,15 @@ class AssertionError extends Error {
         operator === 'notStrictEqual') {
         // In case the objects are equal but the operator requires unequal, show
         // the first object and say A equals B
+        let base = kReadableOperator[operator];
         const res = inspectValue(actual).split('\n');
-        const base = kReadableOperator[operator];
+
+        // In case "actual" is an object, it should not be reference equal.
+        if (operator === 'notStrictEqual' &&
+            typeof actual === 'object' &&
+            actual !== null) {
+          base = kReadableOperator.notStrictEqualObject;
+        }
 
         // Only remove lines in case it makes sense to collapse those.
         // TODO: Accept env to always show the full error.

test/parallel/test-assert.js

@@ -415,9 +415,9 @@ assert.throws(
   {
     code: 'ERR_ASSERTION',
     name: 'AssertionError [ERR_ASSERTION]',
-    message: strictEqualMessageStart +
+    message: 'Expected "actual" to be reference-equal to "expected":\n' +
              '+ actual - expected\n\n' +
-             '+ [Error: foo]\n- [Error: foobar]\n             ^'
+             '+ [Error: foo]\n- [Error: foobar]'
   }
 );
 
@@ -1022,7 +1022,8 @@ assert.throws(
   assert.throws(
     () => assert.strictEqual(args, { 0: 'a' }),
     {
-      message: `${strictEqualMessageStart}+ actual - expected\n\n` +
+      message: 'Expected "actual" to be reference-equal to "expected":\n' +
+               '+ actual - expected\n\n' +
                "+ [Arguments] {\n- {\n    '0': 'a'\n  }"
     }
   );
@@ -1091,3 +1092,43 @@ assert.throws(
     }
   );
 }
+
+// Indicate where the strings diverge.
+assert.throws(
+  () => assert.strictEqual('test test', 'test foobar'),
+  {
+    code: 'ERR_ASSERTION',
+    name: 'AssertionError [ERR_ASSERTION]',
+    message: strictEqualMessageStart +
+             '+ actual - expected\n\n' +
+             "+ 'test test'\n" +
+             "- 'test foobar'\n" +
+             '        ^'
+  }
+);
+
+// Check for reference equal objects in `notStrictEqual()`
+assert.throws(
+  () => {
+    const obj = {};
+    assert.notStrictEqual(obj, obj);
+  },
+  {
+    code: 'ERR_ASSERTION',
+    name: 'AssertionError [ERR_ASSERTION]',
+    message: 'Expected "actual" not to be reference-equal to "expected": {}'
+  }
+);
+
+assert.throws(
+  () => {
+    const obj = { a: true };
+    assert.notStrictEqual(obj, obj);
+  },
+  {
+    code: 'ERR_ASSERTION',
+    name: 'AssertionError [ERR_ASSERTION]',
+    message: 'Expected "actual" not to be reference-equal to "expected":\n\n' +
+             '{\n  a: true\n}\n'
+  }
+);