From 1d859ef53224c982aee659d0e17b1e5747b19fa0 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Mon, 6 Aug 2018 18:20:44 +0200 Subject: [PATCH] assert: improve loose assertion message 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: https://github.com/nodejs/node/pull/22155 Reviewed-By: James M Snell Reviewed-By: Matteo Collina --- lib/internal/assert.js | 47 +++++++--- test/parallel/test-assert-deep.js | 151 +++++++++++------------------- 2 files changed, 88 insertions(+), 110 deletions(-) diff --git a/lib/internal/assert.js b/lib/internal/assert.js index 894b36d17fadab..d151efe3df86ce 100644 --- a/lib/internal/assert.js +++ b/lib/internal/assert.js @@ -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:', }; @@ -50,7 +54,7 @@ function inspectValue(val) { // Assert does not detect proxies currently. showProxy: false } - ).split('\n'); + ); } function createErrDiff(actual, expected, operator) { @@ -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`; @@ -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. @@ -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. @@ -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}`); } } diff --git a/test/parallel/test-assert-deep.js b/test/parallel/test-assert-deep.js index 4536b6d535f703..2cc158ec9bccbe 100644 --- a/test/parallel/test-assert-deep.js +++ b/test/parallel/test-assert-deep.js @@ -1,6 +1,6 @@ 'use strict'; -const common = require('../common'); +require('../common'); const assert = require('assert'); const util = require('util'); const { AssertionError } = assert; @@ -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. @@ -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); @@ -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' } @@ -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 ]; @@ -626,41 +631,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; @@ -720,23 +700,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)); @@ -757,11 +746,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/), { @@ -871,33 +855,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');