Skip to content

Commit 0cd6028

Browse files
BridgeARTrott
BridgeAR
authored and
Trott
committed
assert: refine assertion message
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: nodejs#27525 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>
1 parent 3593af0 commit 0cd6028

4 files changed

Lines changed: 73 additions & 44 deletions

File tree

‎lib/internal/assert/assertion_error.js‎

Lines changed: 35 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -17,19 +17,18 @@ const kReadableOperator = {
1717
strictEqual: 'Expected values to be strictly equal:',
1818
strictEqualObject: 'Expected "actual" to be reference-equal to "expected":',
1919
deepEqual: 'Expected values to be loosely deep-equal:',
20-
equal: 'Expected values to be loosely equal:',
2120
notDeepStrictEqual: 'Expected "actual" not to be strictly deep-equal to:',
2221
notStrictEqual: 'Expected "actual" to be strictly unequal to:',
2322
notStrictEqualObject:
2423
'Expected "actual" not to be reference-equal to "expected":',
2524
notDeepEqual: 'Expected "actual" not to be loosely deep-equal to:',
26-
notEqual: 'Expected "actual" to be loosely unequal to:',
2725
notIdentical: 'Values identical but not reference-equal:',
26+
notDeepEqualUnequal: 'Expected values not to be loosely deep-equal:'
2827
};
2928

3029
// Comparing short primitives should just show === / !== instead of using the
3130
// diff.
32-
const kMaxShortLength = 10;
31+
const kMaxShortLength = 12;
3332

3433
function copyError(source) {
3534
const keys = Object.keys(source);
@@ -81,13 +80,12 @@ function createErrDiff(actual, expected, operator) {
8180
let i = 0;
8281
let indicator = '';
8382

84-
// In case both values are objects explicitly mark them as not reference equal
85-
// for the `strictEqual` operator.
83+
// In case both values are objects or functions explicitly mark them as not
84+
// reference equal for the `strictEqual` operator.
8685
if (operator === 'strictEqual' &&
87-
typeof actual === 'object' &&
88-
typeof expected === 'object' &&
89-
actual !== null &&
90-
expected !== null) {
86+
((typeof actual === 'object' && actual !== null &&
87+
typeof expected === 'object' && expected !== null) ||
88+
(typeof actual === 'function' && typeof expected === 'function'))) {
9189
operator = 'strictEqualObject';
9290
}
9391

@@ -153,9 +151,9 @@ function createErrDiff(actual, expected, operator) {
153151

154152
// Only remove lines in case it makes sense to collapse those.
155153
// TODO: Accept env to always show the full error.
156-
if (actualLines.length > 30) {
157-
actualLines[26] = `${blue}...${white}`;
158-
while (actualLines.length > 27) {
154+
if (actualLines.length > 50) {
155+
actualLines[46] = `${blue}...${white}`;
156+
while (actualLines.length > 47) {
159157
actualLines.pop();
160158
}
161159
}
@@ -282,8 +280,8 @@ function createErrDiff(actual, expected, operator) {
282280
}
283281
}
284282
}
285-
// Inspected object to big (Show ~20 rows max)
286-
if (printedLines > 20 && i < maxLines - 2) {
283+
// Inspected object to big (Show ~50 rows max)
284+
if (printedLines > 50 && i < maxLines - 2) {
287285
return `${msg}${skippedMsg}\n${res}\n${blue}...${white}${other}\n` +
288286
`${blue}...${white}`;
289287
}
@@ -348,52 +346,61 @@ class AssertionError extends Error {
348346
let base = kReadableOperator[operator];
349347
const res = inspectValue(actual).split('\n');
350348

351-
// In case "actual" is an object, it should not be reference equal.
349+
// In case "actual" is an object or a function, it should not be
350+
// reference equal.
352351
if (operator === 'notStrictEqual' &&
353-
typeof actual === 'object' &&
354-
actual !== null) {
352+
(typeof actual === 'object' && actual !== null ||
353+
typeof actual === 'function')) {
355354
base = kReadableOperator.notStrictEqualObject;
356355
}
357356

358357
// Only remove lines in case it makes sense to collapse those.
359358
// TODO: Accept env to always show the full error.
360-
if (res.length > 30) {
361-
res[26] = `${blue}...${white}`;
362-
while (res.length > 27) {
359+
if (res.length > 50) {
360+
res[46] = `${blue}...${white}`;
361+
while (res.length > 47) {
363362
res.pop();
364363
}
365364
}
366365

367366
// Only print a single input.
368367
if (res.length === 1) {
369-
super(`${base} ${res[0]}`);
368+
super(`${base}${res[0].length > 5 ? '\n\n' : ' '}${res[0]}`);
370369
} else {
371370
super(`${base}\n\n${res.join('\n')}\n`);
372371
}
373372
} else {
374373
let res = inspectValue(actual);
375-
let other = '';
374+
let other = inspectValue(expected);
376375
const knownOperators = kReadableOperator[operator];
377-
if (operator === 'notDeepEqual' || operator === 'notEqual') {
378-
res = `${kReadableOperator[operator]}\n\n${res}`;
376+
if ((operator === 'notDeepEqual' || operator === 'notEqual') &&
377+
res === other) {
378+
res = `${knownOperators}\n\n${res}`;
379379
if (res.length > 1024) {
380380
res = `${res.slice(0, 1021)}...`;
381381
}
382+
super(res);
382383
} else {
383-
other = `${inspectValue(expected)}`;
384384
if (res.length > 512) {
385385
res = `${res.slice(0, 509)}...`;
386386
}
387387
if (other.length > 512) {
388388
other = `${other.slice(0, 509)}...`;
389389
}
390390
if (operator === 'deepEqual' || operator === 'equal') {
391-
res = `${knownOperators}\n\n${res}\n\nshould equal\n\n`;
391+
const eq = operator === 'deepEqual' ? 'deep-equal' : 'equal';
392+
res = `${knownOperators}\n\n${res}\n\nshould loosely ${eq}\n\n`;
392393
} else {
393-
other = ` ${operator} ${other}`;
394+
const newOperator = kReadableOperator[`${operator}Unequal`];
395+
if (newOperator) {
396+
const eq = operator === 'notDeepEqual' ? 'deep-equal' : 'equal';
397+
res = `${newOperator}\n\n${res}\n\nshould not loosely ${eq}\n\n`;
398+
} else {
399+
other = ` ${operator} ${other}`;
400+
}
394401
}
402+
super(`${res}${other}`);
395403
}
396-
super(`${res}${other}`);
397404
}
398405
}
399406

‎test/parallel/test-assert-deep.js‎

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,7 @@ function assertDeepAndStrictEqual(a, b) {
195195
function assertNotDeepOrStrict(a, b, err) {
196196
assert.throws(
197197
() => assert.deepEqual(a, b),
198-
err || re`${a}\n\nshould equal\n\n${b}`
198+
err || re`${a}\n\nshould loosely deep-equal\n\n${b}`
199199
);
200200
assert.throws(
201201
() => assert.deepStrictEqual(a, b),
@@ -204,7 +204,7 @@ function assertNotDeepOrStrict(a, b, err) {
204204

205205
assert.throws(
206206
() => assert.deepEqual(b, a),
207-
err || re`${b}\n\nshould equal\n\n${a}`
207+
err || re`${b}\n\nshould loosely deep-equal\n\n${a}`
208208
);
209209
assert.throws(
210210
() => assert.deepStrictEqual(b, a),
@@ -651,6 +651,20 @@ assertDeepAndStrictEqual(-0, -0);
651651
assertDeepAndStrictEqual(a, b);
652652
}
653653

654+
assert.throws(
655+
() => assert.notDeepEqual(1, true),
656+
{
657+
message: /1\n\nshould not loosely deep-equal\n\ntrue/
658+
}
659+
);
660+
661+
assert.throws(
662+
() => assert.notDeepEqual(1, 1),
663+
{
664+
message: /Expected "actual" not to be loosely deep-equal to:\n\n1/
665+
}
666+
);
667+
654668
assert.deepEqual(new Date(2000, 3, 14), new Date(2000, 3, 14));
655669

656670
assert.throws(() => { assert.deepEqual(new Date(), new Date(2000, 3, 14)); },
@@ -779,7 +793,7 @@ assert.throws(
779793
() => assert.notDeepStrictEqual(new Date(2000, 3, 14), new Date(2000, 3, 14)),
780794
{
781795
name: 'AssertionError',
782-
message: 'Expected "actual" not to be strictly deep-equal to: ' +
796+
message: 'Expected "actual" not to be strictly deep-equal to:\n\n' +
783797
util.inspect(new Date(2000, 3, 14))
784798
}
785799
);
@@ -815,11 +829,11 @@ assert.throws(
815829
message: `${defaultMsgStartFull}\n\n+ /a/m\n- /a/`
816830
});
817831
assert.throws(
818-
() => assert.deepStrictEqual(/a/igm, /a/im),
832+
() => assert.deepStrictEqual(/aa/igm, /aa/im),
819833
{
820834
code: 'ERR_ASSERTION',
821835
name: 'AssertionError',
822-
message: `${defaultMsgStartFull}\n\n+ /a/gim\n- /a/im\n ^`
836+
message: `${defaultMsgStartFull}\n\n+ /aa/gim\n- /aa/im\n ^`
823837
});
824838

825839
{
@@ -939,12 +953,12 @@ assert.deepStrictEqual(obj1, obj2);
939953
}
940954

941955
// Strict equal with identical objects that are not identical
942-
// by reference and longer than 30 elements
956+
// by reference and longer than 50 elements
943957
// E.g., assert.deepStrictEqual({ a: Symbol() }, { a: Symbol() })
944958
{
945959
const a = {};
946960
const b = {};
947-
for (let i = 0; i < 35; i++) {
961+
for (let i = 0; i < 55; i++) {
948962
a[`symbol${i}`] = Symbol();
949963
b[`symbol${i}`] = Symbol();
950964
}

‎test/parallel/test-assert.js‎

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -80,12 +80,20 @@ assert.throws(
8080
assert.throws(
8181
() => a.notStrictEqual('a '.repeat(30), 'a '.repeat(30)),
8282
{
83-
message: 'Expected "actual" to be strictly unequal to: ' +
83+
message: 'Expected "actual" to be strictly unequal to:\n\n' +
8484
`'${'a '.repeat(30)}'`,
8585
name: 'AssertionError'
8686
}
8787
);
8888

89+
assert.throws(
90+
() => a.notEqual(1, 1),
91+
{
92+
message: '1 != 1',
93+
operator: '!='
94+
}
95+
);
96+
8997
a.notStrictEqual(2, '2');
9098

9199
// Testing the throwing.
@@ -281,12 +289,12 @@ testShortAssertionMessage('a', '"a"');
281289
testShortAssertionMessage('foo', '\'foo\'');
282290
testShortAssertionMessage(0, '0');
283291
testShortAssertionMessage(Symbol(), 'Symbol()');
292+
testShortAssertionMessage(undefined, 'undefined');
293+
testShortAssertionMessage(-Infinity, '-Infinity');
284294
testAssertionMessage([], '[]');
285295
testAssertionMessage(/a/, '/a/');
286296
testAssertionMessage(/abc/gim, '/abc/gim');
287297
testAssertionMessage({}, '{}');
288-
testAssertionMessage(undefined, 'undefined');
289-
testAssertionMessage(-Infinity, '-Infinity');
290298
testAssertionMessage([1, 2, 3], '[\n+ 1,\n+ 2,\n+ 3\n+ ]');
291299
testAssertionMessage(function f() {}, '[Function: f]');
292300
testAssertionMessage(function() {}, '[Function (anonymous)]');
@@ -579,12 +587,12 @@ assert.throws(
579587
`${actExp} ... Lines skipped\n` +
580588
'\n' +
581589
' [\n' +
582-
'+ 1,\n'.repeat(10) +
590+
'+ 1,\n'.repeat(25) +
583591
'...\n' +
584-
'- 2,\n'.repeat(10) +
592+
'- 2,\n'.repeat(25) +
585593
'...';
586594
assert.throws(
587-
() => assert.deepEqual(Array(12).fill(1), Array(12).fill(2)),
595+
() => assert.deepEqual(Array(28).fill(1), Array(28).fill(2)),
588596
{ message });
589597

590598
const obj1 = {};
@@ -612,8 +620,8 @@ assert.throws(
612620
);
613621

614622
message = 'Expected "actual" not to be strictly deep-equal to:' +
615-
`\n\n[${'\n 1,'.repeat(25)}\n...\n`;
616-
const data = Array(31).fill(1);
623+
`\n\n[${'\n 1,'.repeat(45)}\n...\n`;
624+
const data = Array(51).fill(1);
617625
assert.throws(
618626
() => assert.notDeepEqual(data, data),
619627
{ message });

‎test/pseudo-tty/test-assert-position-indicator.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,6 @@ assert.throws(
1313

1414
// Confirm that there is a position indicator.
1515
assert.throws(
16-
() => { assert.deepStrictEqual('aaa', 'aaaa'); },
16+
() => { assert.deepStrictEqual('aaaa', 'aaaaa'); },
1717
(err) => err.message.includes('^')
1818
);

0 commit comments

Comments
 (0)