Skip to content

Commit

Permalink
assert: improve loose assertion message
Browse files Browse the repository at this point in the history 
So far the error message from a loose assertion is not always very
informative and could be misleading. This is fixed by:

* showing more from the actual error message
* having a better error description
* not using custom inspection
* inspecting a higher depth
* inspecting the full array instead of up to 100 entries

PR-URL: nodejs#22155
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
  • Loading branch information
@BridgeAR @oyyd
BridgeAR authored and oyyd committed Sep 25, 2018
1 parent f75e969 commit 7f54071
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 110 deletions.
47 changes: 34 additions & 13 deletions lib/internal/assert.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,13 @@ let white = '';

const kReadableOperator = {
deepStrictEqual: 'Expected inputs to be strictly deep-equal:',
notDeepStrictEqual: 'Expected "actual" not to be strictly deep-equal to:',
strictEqual: 'Expected inputs to be strictly equal:',
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:',
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 @@ -50,7 +54,7 @@ function inspectValue(val) {
// Assert does not detect proxies currently.
showProxy: false
}
).split('\n');
);
}

function createErrDiff(actual, expected, operator) {
Expand All @@ -59,8 +63,9 @@ function createErrDiff(actual, expected, operator) {
let lastPos = 0;
let end = '';
let skipped = false;
const actualLines = inspectValue(actual);
const expectedLines = inspectValue(expected);
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`;
Expand Down Expand Up @@ -126,7 +131,7 @@ function createErrDiff(actual, expected, operator) {
// E.g., assert.deepStrictEqual({ a: Symbol() }, { a: Symbol() })
if (maxLines === 0) {
// We have to get the result again. The lines were all removed before.
const actualLines = inspectValue(actual);
const actualLines = actualInspected.split('\n');

// Only remove lines in case it makes sense to collapse those.
// TODO: Accept env to always show the full error.
Expand Down Expand Up @@ -268,7 +273,7 @@ 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
const res = inspectValue(actual);
const res = inspectValue(actual).split('\n');
const base = kReadableOperator[operator];

// Only remove lines in case it makes sense to collapse those.
Expand All @@ -287,13 +292,29 @@ class AssertionError extends Error {
super(`${base}\n\n${res.join('\n')}\n`);
}
} else {
let res = inspect(actual);
let other = inspect(expected);
if (res.length > 128)
res = `${res.slice(0, 125)}...`;
if (other.length > 128)
other = `${other.slice(0, 125)}...`;
super(`${res} ${operator} ${other}`);
let res = inspectValue(actual);
let other = '';
const knownOperators = kReadableOperator[operator];
if (operator === 'notDeepEqual' || operator === 'notEqual') {
res = `${kReadableOperator[operator]}\n\n${res}`;
if (res.length > 1024) {
res = `${res.slice(0, 1021)}...`;
}
} 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`;
} else {
other = ` ${operator} ${other}`;
}
}
super(`${res}${other}`);
}
}

Expand Down
151 changes: 54 additions & 97 deletions test/parallel/test-assert-deep.js
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use strict';

const common = require('../common');
require('../common');
const assert = require('assert');
const util = require('util');
const { AssertionError } = assert;
Expand All @@ -16,18 +16,23 @@ if (process.stdout.isTTY)
// Template tag function turning an error message into a RegExp
// for assert.throws()
function re(literals, ...values) {
let result = literals[0];
const escapeRE = /[\\^$.*+?()[\]{}|=!<>:-]/g;
let result = 'Expected inputs to be loosely deep-equal:\n\n';
for (const [i, value] of values.entries()) {
const str = util.inspect(value);
const str = util.inspect(value, {
compact: false,
depth: 1000,
customInspect: false,
maxArrayLength: Infinity,
breakLength: Infinity
});
// Need to escape special characters.
result += str.replace(escapeRE, '\\$&');
result += str;
result += literals[i + 1];
}
return common.expectsError({
return {
code: 'ERR_ASSERTION',
message: new RegExp(`^${result}$`)
});
message: result
};
}

// The following deepEqual tests might seem very weird.
Expand Down Expand Up @@ -173,13 +178,6 @@ assert.throws(
}
}

common.expectsError(() => {
assert.deepEqual(new Set([{ a: 0 }]), new Set([{ a: 1 }]));
}, {
code: 'ERR_ASSERTION',
message: /^Set { { a: 0 } } deepEqual Set { { a: 1 } }$/
});

function assertDeepAndStrictEqual(a, b) {
assert.deepEqual(a, b);
assert.deepStrictEqual(a, b);
Expand All @@ -189,13 +187,19 @@ function assertDeepAndStrictEqual(a, b) {
}

function assertNotDeepOrStrict(a, b, err) {
assert.throws(() => assert.deepEqual(a, b), err || re`${a} deepEqual ${b}`);
assert.throws(
() => assert.deepEqual(a, b),
err || re`${a}\n\nshould equal\n\n${b}`
);
assert.throws(
() => assert.deepStrictEqual(a, b),
err || { code: 'ERR_ASSERTION' }
);

assert.throws(() => assert.deepEqual(b, a), err || re`${b} deepEqual ${a}`);
assert.throws(
() => assert.deepEqual(b, a),
err || re`${b}\n\nshould equal\n\n${a}`
);
assert.throws(
() => assert.deepStrictEqual(b, a),
err || { code: 'ERR_ASSERTION' }
Expand Down Expand Up @@ -225,6 +229,7 @@ assertNotDeepOrStrict(new Set([1, 2, 3]), new Set([1, 2, 3, 4]));
assertNotDeepOrStrict(new Set([1, 2, 3, 4]), new Set([1, 2, 3]));
assertDeepAndStrictEqual(new Set(['1', '2', '3']), new Set(['1', '2', '3']));
assertDeepAndStrictEqual(new Set([[1, 2], [3, 4]]), new Set([[3, 4], [1, 2]]));
assertNotDeepOrStrict(new Set([{ a: 0 }]), new Set([{ a: 1 }]));

{
const a = [ 1, 2 ];
Expand Down Expand Up @@ -634,41 +639,16 @@ assert.throws(

assert.notDeepEqual(new Date(), new Date(2000, 3, 14));

assert.deepEqual(/a/, /a/);
assert.deepEqual(/a/g, /a/g);
assert.deepEqual(/a/i, /a/i);
assert.deepEqual(/a/m, /a/m);
assert.deepEqual(/a/igm, /a/igm);
assert.throws(() => assert.deepEqual(/ab/, /a/),
{
code: 'ERR_ASSERTION',
name: 'AssertionError [ERR_ASSERTION]',
message: '/ab/ deepEqual /a/'
});
assert.throws(() => assert.deepEqual(/a/g, /a/),
{
code: 'ERR_ASSERTION',
name: 'AssertionError [ERR_ASSERTION]',
message: '/a/g deepEqual /a/'
});
assert.throws(() => assert.deepEqual(/a/i, /a/),
{
code: 'ERR_ASSERTION',
name: 'AssertionError [ERR_ASSERTION]',
message: '/a/i deepEqual /a/'
});
assert.throws(() => assert.deepEqual(/a/m, /a/),
{
code: 'ERR_ASSERTION',
name: 'AssertionError [ERR_ASSERTION]',
message: '/a/m deepEqual /a/'
});
assert.throws(() => assert.deepEqual(/a/igm, /a/im),
{
code: 'ERR_ASSERTION',
name: 'AssertionError [ERR_ASSERTION]',
message: '/a/gim deepEqual /a/im'
});
assertDeepAndStrictEqual(/a/, /a/);
assertDeepAndStrictEqual(/a/g, /a/g);
assertDeepAndStrictEqual(/a/i, /a/i);
assertDeepAndStrictEqual(/a/m, /a/m);
assertDeepAndStrictEqual(/a/igm, /a/igm);
assertNotDeepOrStrict(/ab/, /a/);
assertNotDeepOrStrict(/a/g, /a/);
assertNotDeepOrStrict(/a/i, /a/);
assertNotDeepOrStrict(/a/m, /a/);
assertNotDeepOrStrict(/a/igm, /a/im);

{
const re1 = /a/g;
Expand Down Expand Up @@ -728,23 +708,32 @@ nameBuilder2.prototype = Object;
nb2 = new nameBuilder2('Ryan', 'Dahl');
assert.deepEqual(nb1, nb2);

// Primitives and object.
assert.throws(() => assert.deepEqual(null, {}), AssertionError);
assert.throws(() => assert.deepEqual(undefined, {}), AssertionError);
assert.throws(() => assert.deepEqual('a', ['a']), AssertionError);
assert.throws(() => assert.deepEqual('a', { 0: 'a' }), AssertionError);
assert.throws(() => assert.deepEqual(1, {}), AssertionError);
assert.throws(() => assert.deepEqual(true, {}), AssertionError);
assert.throws(() => assert.deepEqual(Symbol(), {}), AssertionError);
// Primitives
assertNotDeepOrStrict(null, {});
assertNotDeepOrStrict(undefined, {});
assertNotDeepOrStrict('a', ['a']);
assertNotDeepOrStrict('a', { 0: 'a' });
assertNotDeepOrStrict(1, {});
assertNotDeepOrStrict(true, {});
assertNotDeepOrStrict(Symbol(), {});
assertNotDeepOrStrict(Symbol(), Symbol());

assertOnlyDeepEqual(4, '4');
assertOnlyDeepEqual(true, 1);

{
const s = Symbol();
assertDeepAndStrictEqual(s, s);
}

// Primitive wrappers and object.
assert.deepEqual(new String('a'), ['a']);
assert.deepEqual(new String('a'), { 0: 'a' });
assert.deepEqual(new Number(1), {});
assert.deepEqual(new Boolean(true), {});
assertOnlyDeepEqual(new String('a'), ['a']);
assertOnlyDeepEqual(new String('a'), { 0: 'a' });
assertOnlyDeepEqual(new Number(1), {});
assertOnlyDeepEqual(new Boolean(true), {});

// Same number of keys but different key names.
assert.throws(() => assert.deepEqual({ a: 1 }, { b: 1 }), AssertionError);
assertNotDeepOrStrict({ a: 1 }, { b: 1 });

assert.deepStrictEqual(new Date(2000, 3, 14), new Date(2000, 3, 14));

Expand All @@ -765,11 +754,6 @@ assert.throws(

assert.notDeepStrictEqual(new Date(), new Date(2000, 3, 14));

assert.deepStrictEqual(/a/, /a/);
assert.deepStrictEqual(/a/g, /a/g);
assert.deepStrictEqual(/a/i, /a/i);
assert.deepStrictEqual(/a/m, /a/m);
assert.deepStrictEqual(/a/igm, /a/igm);
assert.throws(
() => assert.deepStrictEqual(/ab/, /a/),
{
Expand Down Expand Up @@ -879,33 +863,6 @@ obj2 = new Constructor2('Ryan', 'Dahl');

assert.deepStrictEqual(obj1, obj2);

// primitives
assert.throws(() => assert.deepStrictEqual(4, '4'), AssertionError);
assert.throws(() => assert.deepStrictEqual(true, 1), AssertionError);
assert.throws(() => assert.deepStrictEqual(Symbol(), Symbol()),
AssertionError);

const s = Symbol();
assert.deepStrictEqual(s, s);

// Primitives and object.
assert.throws(() => assert.deepStrictEqual(null, {}), AssertionError);
assert.throws(() => assert.deepStrictEqual(undefined, {}), AssertionError);
assert.throws(() => assert.deepStrictEqual('a', ['a']), AssertionError);
assert.throws(() => assert.deepStrictEqual('a', { 0: 'a' }), AssertionError);
assert.throws(() => assert.deepStrictEqual(1, {}), AssertionError);
assert.throws(() => assert.deepStrictEqual(true, {}), AssertionError);
assert.throws(() => assert.deepStrictEqual(Symbol(), {}), AssertionError);

// Primitive wrappers and object.
assert.throws(() => assert.deepStrictEqual(new String('a'), ['a']),
AssertionError);
assert.throws(() => assert.deepStrictEqual(new String('a'), { 0: 'a' }),
AssertionError);
assert.throws(() => assert.deepStrictEqual(new Number(1), {}), AssertionError);
assert.throws(() => assert.deepStrictEqual(new Boolean(true), {}),
AssertionError);

// Check extra properties on errors.
{
const a = new TypeError('foo');
Expand Down

0 comments on commit 7f54071

Please sign in to comment.