FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

assert: implement partial error comparison · nodejs/node@64b0867 · GitHub

/ node Public

Commit 64b0867

Browse files
BridgeAR
authored and
aduh95
committed
assert: implement partial error comparison
assert.partialDeepStrictEqual now also handled error properties as expected. On top of that, the main implementation also handles non-string `name` and `message` properties and the comparison is a tad faster by removing duplicated comparison steps. As a drive-by fix this also cleans up some code by abstracting code and renaming variables for clarity. PR-URL: #57370 Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
1 parent 80d9d56 commit 64b0867

3 files changed

Lines changed: 94 additions & 62 deletions

File tree

‎lib/internal/util/comparisons.js‎

Lines changed: 44 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -10,17 +10,17 @@ const {
1010
Error,
1111
NumberIsNaN,
1212
NumberPrototypeValueOf,
13-
ObjectGetOwnPropertySymbols,
13+
ObjectGetOwnPropertySymbols: getOwnSymbols,
1414
ObjectGetPrototypeOf,
1515
ObjectIs,
1616
ObjectKeys,
17-
ObjectPrototypeHasOwnProperty,
18-
ObjectPrototypePropertyIsEnumerable,
17+
ObjectPrototypeHasOwnProperty: hasOwn,
18+
ObjectPrototypePropertyIsEnumerable: hasEnumerable,
1919
ObjectPrototypeToString,
2020
SafeSet,
2121
StringPrototypeValueOf,
2222
SymbolPrototypeValueOf,
23-
TypedArrayPrototypeGetByteLength,
23+
TypedArrayPrototypeGetByteLength: getByteLength,
2424
TypedArrayPrototypeGetSymbolToStringTag,
2525
Uint8Array,
2626
} = primordials;
@@ -78,8 +78,8 @@ function areSimilarRegExps(a, b) {
7878
}
7979

8080
function isPartialUint8Array(a, b) {
81-
const lenA = TypedArrayPrototypeGetByteLength(a);
82-
const lenB = TypedArrayPrototypeGetByteLength(b);
81+
const lenA = getByteLength(a);
82+
const lenB = getByteLength(b);
8383
if (lenA < lenB) {
8484
return false;
8585
}
@@ -107,10 +107,10 @@ function isPartialArrayBufferView(a, b) {
107107
}
108108

109109
function areSimilarFloatArrays(a, b) {
110-
if (a.byteLength !== b.byteLength) {
110+
const len = getByteLength(a);
111+
if (len !== getByteLength(b)) {
111112
return false;
112113
}
113-
const len = TypedArrayPrototypeGetByteLength(a);
114114
for (let offset = 0; offset < len; offset++) {
115115
if (a[offset] !== b[offset]) {
116116
return false;
@@ -158,6 +158,12 @@ function isEqualBoxedPrimitive(val1, val2) {
158158
assert.fail(`Unknown boxed type ${val1}`);
159159
}
160160

161+
function isEnumerableOrIdentical(val1, val2, prop, mode, memos, method) {
162+
return hasEnumerable(val2, prop) || // This is handled by Object.keys()
163+
(mode === kPartial && (val2[prop] === undefined || (prop === 'message' && val2[prop] === ''))) ||
164+
innerDeepEqual(val1[prop], val2[prop], mode, memos);
165+
}
166+
161167
// Notes: Type tags are historical [[Class]] properties that can be set by
162168
// FunctionTemplate::SetClassName() in C++ or Symbol.toStringTag in JS
163169
// and retrieved using Object.prototype.toString.call(obj) in JS
@@ -263,8 +269,7 @@ function innerDeepEqual(val1, val2, mode, memos) {
263269
(val1.size !== val2.size && (mode !== kPartial || val1.size < val2.size))) {
264270
return false;
265271
}
266-
const result = keyCheck(val1, val2, mode, memos, kIsSet);
267-
return result;
272+
return keyCheck(val1, val2, mode, memos, kIsSet);
268273
} else if (isMap(val1)) {
269274
if (!isMap(val2) ||
270275
(val1.size !== val2.size && (mode !== kPartial || val1.size < val2.size))) {
@@ -285,26 +290,15 @@ function innerDeepEqual(val1, val2, mode, memos) {
285290
} else if (isError(val1)) {
286291
// Do not compare the stack as it might differ even though the error itself
287292
// is otherwise identical.
288-
if (!isError(val2)) {
293+
if (!isError(val2) ||
294+
!isEnumerableOrIdentical(val1, val2, 'message', mode, memos) ||
295+
!isEnumerableOrIdentical(val1, val2, 'name', mode, memos) ||
296+
!isEnumerableOrIdentical(val1, val2, 'cause', mode, memos) ||
297+
!isEnumerableOrIdentical(val1, val2, 'errors', mode, memos)) {
289298
return false;
290299
}
291-
292-
const message1Enumerable = ObjectPrototypePropertyIsEnumerable(val1, 'message');
293-
const name1Enumerable = ObjectPrototypePropertyIsEnumerable(val1, 'name');
294-
// TODO(BridgeAR): Adjust cause and errors properties for partial mode.
295-
const cause1Enumerable = ObjectPrototypePropertyIsEnumerable(val1, 'cause');
296-
const errors1Enumerable = ObjectPrototypePropertyIsEnumerable(val1, 'errors');
297-
298-
if ((message1Enumerable !== ObjectPrototypePropertyIsEnumerable(val2, 'message') ||
299-
(!message1Enumerable && val1.message !== val2.message)) ||
300-
(name1Enumerable !== ObjectPrototypePropertyIsEnumerable(val2, 'name') ||
301-
(!name1Enumerable && val1.name !== val2.name)) ||
302-
(cause1Enumerable !== ObjectPrototypePropertyIsEnumerable(val2, 'cause') ||
303-
(!cause1Enumerable && (
304-
ObjectPrototypeHasOwnProperty(val1, 'cause') !== ObjectPrototypeHasOwnProperty(val2, 'cause') ||
305-
!innerDeepEqual(val1.cause, val2.cause, mode, memos)))) ||
306-
(errors1Enumerable !== ObjectPrototypePropertyIsEnumerable(val2, 'errors') ||
307-
(!errors1Enumerable && !innerDeepEqual(val1.errors, val2.errors, mode, memos)))) {
300+
const hasOwnVal2Cause = hasOwn(val2, 'cause');
301+
if ((hasOwnVal2Cause !== hasOwn(val1, 'cause') && (mode !== kPartial || hasOwnVal2Cause))) {
308302
return false;
309303
}
310304
} else if (isBoxedPrimitive(val1)) {
@@ -348,10 +342,7 @@ function innerDeepEqual(val1, val2, mode, memos) {
348342
}
349343

350344
function getEnumerables(val, keys) {
351-
return ArrayPrototypeFilter(
352-
keys,
353-
(k) => ObjectPrototypePropertyIsEnumerable(val, k),
354-
);
345+
return ArrayPrototypeFilter(keys, (key) => hasEnumerable(val, key));
355346
}
356347

357348
function keyCheck(val1, val2, mode, memos, iterationType, keys2) {
@@ -371,7 +362,7 @@ function keyCheck(val1, val2, mode, memos, iterationType, keys2) {
371362
// Cheap key test
372363
if (keys2.length > 0) {
373364
for (const key of keys2) {
374-
if (!ObjectPrototypePropertyIsEnumerable(val1, key)) {
365+
if (!hasEnumerable(val1, key)) {
375366
return false;
376367
}
377368
}
@@ -380,11 +371,11 @@ function keyCheck(val1, val2, mode, memos, iterationType, keys2) {
380371
if (!isArrayLikeObject) {
381372
// The pair must have the same number of owned properties.
382373
if (mode === kPartial) {
383-
const symbolKeys = ObjectGetOwnPropertySymbols(val2);
374+
const symbolKeys = getOwnSymbols(val2);
384375
if (symbolKeys.length !== 0) {
385376
for (const key of symbolKeys) {
386-
if (ObjectPrototypePropertyIsEnumerable(val2, key)) {
387-
if (!ObjectPrototypePropertyIsEnumerable(val1, key)) {
377+
if (hasEnumerable(val2, key)) {
378+
if (!hasEnumerable(val1, key)) {
388379
return false;
389380
}
390381
ArrayPrototypePush(keys2, key);
@@ -396,27 +387,27 @@ function keyCheck(val1, val2, mode, memos, iterationType, keys2) {
396387
}
397388

398389
if (mode === kStrict) {
399-
const symbolKeysA = ObjectGetOwnPropertySymbols(val1);
390+
const symbolKeysA = getOwnSymbols(val1);
400391
if (symbolKeysA.length !== 0) {
401392
let count = 0;
402393
for (const key of symbolKeysA) {
403-
if (ObjectPrototypePropertyIsEnumerable(val1, key)) {
404-
if (!ObjectPrototypePropertyIsEnumerable(val2, key)) {
394+
if (hasEnumerable(val1, key)) {
395+
if (!hasEnumerable(val2, key)) {
405396
return false;
406397
}
407398
ArrayPrototypePush(keys2, key);
408399
count++;
409-
} else if (ObjectPrototypePropertyIsEnumerable(val2, key)) {
400+
} else if (hasEnumerable(val2, key)) {
410401
return false;
411402
}
412403
}
413-
const symbolKeysB = ObjectGetOwnPropertySymbols(val2);
404+
const symbolKeysB = getOwnSymbols(val2);
414405
if (symbolKeysA.length !== symbolKeysB.length &&
415406
getEnumerables(val2, symbolKeysB).length !== count) {
416407
return false;
417408
}
418409
} else {
419-
const symbolKeysB = ObjectGetOwnPropertySymbols(val2);
410+
const symbolKeysB = getOwnSymbols(val2);
420411
if (symbolKeysB.length !== 0 &&
421412
getEnumerables(val2, symbolKeysB).length !== 0) {
422413
return false;
@@ -441,7 +432,6 @@ function keyCheck(val1, val2, mode, memos, iterationType, keys2) {
441432
c: undefined,
442433
d: undefined,
443434
deep: false,
444-
deleteFailures: false,
445435
};
446436
return objEquiv(val1, val2, mode, keys2, memos, iterationType);
447437
}
@@ -476,27 +466,21 @@ function keyCheck(val1, val2, mode, memos, iterationType, keys2) {
476466

477467
const areEq = objEquiv(val1, val2, mode, keys2, memos, iterationType);
478468

479-
if (areEq || memos.deleteFailures) {
480-
set.delete(val1);
481-
set.delete(val2);
482-
}
469+
set.delete(val1);
470+
set.delete(val2);
483471

484472
return areEq;
485473
}
486474

487475
function setHasEqualElement(set, val1, mode, memo) {
488-
const { deleteFailures } = memo;
489-
memo.deleteFailures = true;
490476
for (const val2 of set) {
491477
if (innerDeepEqual(val1, val2, mode, memo)) {
492478
// Remove the matching element to make sure we do not check that again.
493479
set.delete(val2);
494-
memo.deleteFailures = deleteFailures;
495480
return true;
496481
}
497482
}
498483

499-
memo.deleteFailures = deleteFailures;
500484
return false;
501485
}
502486

@@ -557,6 +541,8 @@ function partialObjectSetEquiv(a, b, mode, set, memo) {
557541
return false;
558542
}
559543
}
544+
/* c8 ignore next */
545+
assert.fail('Unreachable code');
560546
}
561547

562548
function setObjectEquiv(a, b, mode, set, memo) {
@@ -623,18 +609,14 @@ function mapHasEqualEntry(set, map, key1, item1, mode, memo) {
623609
// To be able to handle cases like:
624610
// Map([[{}, 'a'], [{}, 'b']]) vs Map([[{}, 'b'], [{}, 'a']])
625611
// ... we need to consider *all* matching keys, not just the first we find.
626-
const { deleteFailures } = memo;
627-
memo.deleteFailures = true;
628612
for (const key2 of set) {
629613
if (innerDeepEqual(key1, key2, mode, memo) &&
630614
innerDeepEqual(item1, map.get(key2), mode, memo)) {
631615
set.delete(key2);
632-
memo.deleteFailures = deleteFailures;
633616
return true;
634617
}
635618
}
636619

637-
memo.deleteFailures = deleteFailures;
638620
return false;
639621
}
640622

@@ -652,6 +634,8 @@ function partialObjectMapEquiv(a, b, mode, set, memo) {
652634
return false;
653635
}
654636
}
637+
/* c8 ignore next */
638+
assert.fail('Unreachable code');
655639
}
656640

657641
function mapObjectEquivalence(a, b, mode, set, memo) {
@@ -747,11 +731,11 @@ function partialSparseArrayEquiv(a, b, mode, memos, startA, startB) {
747731
function partialArrayEquiv(a, b, mode, memos) {
748732
let aPos = 0;
749733
for (let i = 0; i < b.length; i++) {
750-
let isSparse = b[i] === undefined && !ObjectPrototypeHasOwnProperty(b, i);
734+
let isSparse = b[i] === undefined && !hasOwn(b, i);
751735
if (isSparse) {
752736
return partialSparseArrayEquiv(a, b, mode, memos, aPos, i);
753737
}
754-
while (!(isSparse = a[aPos] === undefined && !ObjectPrototypeHasOwnProperty(a, aPos)) &&
738+
while (!(isSparse = a[aPos] === undefined && !hasOwn(a, aPos)) &&
755739
!innerDeepEqual(a[aPos], b[i], mode, memos)) {
756740
aPos++;
757741
if (aPos > a.length - b.length + i) {
@@ -776,8 +760,7 @@ function sparseArrayEquiv(a, b, mode, memos, i) {
776760
}
777761
for (; i < keysA.length; i++) {
778762
const key = keysA[i];
779-
if (!ObjectPrototypeHasOwnProperty(b, key) ||
780-
!innerDeepEqual(a[key], b[key], mode, memos)) {
763+
if (!hasOwn(b, key) || !innerDeepEqual(a[key], b[key], mode, memos)) {
781764
return false;
782765
}
783766
}
@@ -802,8 +785,8 @@ function objEquiv(a, b, mode, keys2, memos, iterationType) {
802785
if (!innerDeepEqual(a[i], b[i], mode, memos)) {
803786
return false;
804787
}
805-
const isSparseA = a[i] === undefined && !ObjectPrototypeHasOwnProperty(a, i);
806-
const isSparseB = b[i] === undefined && !ObjectPrototypeHasOwnProperty(b, i);
788+
const isSparseA = a[i] === undefined && !hasOwn(a, i);
789+
const isSparseB = b[i] === undefined && !hasOwn(b, i);
807790
if (isSparseA !== isSparseB) {
808791
return false;
809792
}

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,13 @@ require('../common');
33
const assert = require('assert');
44
const { test } = require('node:test');
55

6+
// Disable colored output to prevent color codes from breaking assertion
7+
// message comparisons. This should only be an issue when process.stdout
8+
// is a TTY.
9+
if (process.stdout.isTTY) {
10+
process.env.NODE_DISABLE_COLORS = '1';
11+
}
12+
613
const defaultStartMessage = 'Expected values to be strictly deep-equal:\n' +
714
'+ actual - expected\n' +
815
'\n';

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

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,10 +176,32 @@ describe('Object Comparison Tests', () => {
176176
},
177177
{
178178
description:
179-
'throws when comparing two objects with different Error instances',
179+
'throws when comparing two objects with different Error message',
180180
actual: { error: new Error('Test error 1') },
181181
expected: { error: new Error('Test error 2') },
182182
},
183+
{
184+
description:
185+
'throws when comparing two objects with missing cause on the actual Error',
186+
actual: { error: new Error('Test error 1') },
187+
expected: { error: new Error('Test error 1', { cause: 42 }) },
188+
},
189+
{
190+
description:
191+
'throws when comparing two objects with missing message on the actual Error',
192+
actual: { error: new Error() },
193+
expected: { error: new Error('Test error 1') },
194+
},
195+
{
196+
description: 'throws when comparing two Errors with missing cause on the actual Error',
197+
actual: { error: new Error('Test error 1') },
198+
expected: { error: new Error('Test error 1', { cause: undefined }) },
199+
},
200+
{
201+
description: 'throws when comparing two AggregateErrors with missing message on the actual Error',
202+
actual: { error: new AggregateError([], 'Test error 1') },
203+
expected: { error: new AggregateError([new Error()], 'Test error 1') },
204+
},
183205
{
184206
description:
185207
'throws when comparing two objects with different TypedArray instances and content',
@@ -1105,6 +1127,26 @@ describe('Object Comparison Tests', () => {
11051127
}
11061128
}]
11071129
},
1130+
{
1131+
description: 'comparing two Errors with missing cause on the expected Error',
1132+
actual: { error: new Error('Test error 1', { cause: 42 }) },
1133+
expected: { error: new Error('Test error 1') },
1134+
},
1135+
{
1136+
description: 'comparing two Errors with cause set to undefined on the actual Error',
1137+
actual: { error: new Error('Test error 1', { cause: undefined }) },
1138+
expected: { error: new Error('Test error 1') },
1139+
},
1140+
{
1141+
description: 'comparing two Errors with missing message on the expected Error',
1142+
actual: { error: new Error('Test error 1') },
1143+
expected: { error: new Error() },
1144+
},
1145+
{
1146+
description: 'comparing two AggregateErrors with no message or errors on the expected Error',
1147+
actual: { error: new AggregateError([new Error(), 123]) },
1148+
expected: { error: new AggregateError([]) },
1149+
},
11081150
].forEach(({ description, actual, expected }) => {
11091151
it(description, () => {
11101152
assert.partialDeepStrictEqual(actual, expected);

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL