Skip to content

Commit

Permalink
assert: improve the strict equal messages
Browse files Browse the repository at this point in the history
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: nodejs#22763
  • Loading branch information
BridgeAR committed Sep 24, 2018
1 parent de0441f commit b7def33
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 8 deletions.
31 changes: 26 additions & 5 deletions lib/internal/assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 reference equal inputs but got:',
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:',
Expand Down Expand Up @@ -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 &&
Expand All @@ -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.
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand Down
47 changes: 44 additions & 3 deletions test/parallel/test-assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -415,9 +415,9 @@ assert.throws(
{
code: 'ERR_ASSERTION',
name: 'AssertionError [ERR_ASSERTION]',
message: strictEqualMessageStart +
message: 'Expected reference equal inputs but got:\n' +
'+ actual - expected\n\n' +
'+ [Error: foo]\n- [Error: foobar]\n ^'
'+ [Error: foo]\n- [Error: foobar]'
}
);

Expand Down Expand Up @@ -1022,7 +1022,8 @@ assert.throws(
assert.throws(
() => assert.strictEqual(args, { 0: 'a' }),
{
message: `${strictEqualMessageStart}+ actual - expected\n\n` +
message: 'Expected reference equal inputs but got:\n' +
'+ actual - expected\n\n' +
"+ [Arguments] {\n- {\n '0': 'a'\n }"
}
);
Expand Down Expand Up @@ -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'
}
);

0 comments on commit b7def33

Please sign in to comment.