1berry/nodejs
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

assert: adjust loose assertions

Browse Source 
This changes the loose deep equal comparison by using the same logic
as done in the strict deep equal comparison besides comparing
primitives loosely, not comparing symbol properties and not comparing
the prototype.

`assert.deepEqual` is still commenly used and this is likely the
biggest pitfall.

Most changes are only minor and won't have a big impact besides
likely fixing user expectations.

PR-URL: https://github.com/nodejs/node/pull/25008
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
This commit is contained in:
Ruben Bridgewater 2018-12-13 07:33:31 +01:00
parent 5cb196441a
commit 7493db21b6
No known key found for this signature in database
GPG Key ID: F07496B3EB3C1762
6 changed files with 116 additions and 161 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

47
doc/api/assert.md
View File

@ -163,6 +163,10 @@ An alias of [`assert.ok()`][].
<!-- YAML
added: v0.1.21
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/25008
description: The type tags are now properly compared and there are a couple
minor comparison adjustments to make the check less surprising.
- version: v9.0.0
pr-url: https://github.com/nodejs/node/pull/15001
description: The `Error` names and messages are now properly compared
@ -191,26 +195,39 @@ An alias of [`assert.deepStrictEqual()`][].
> Stability: 0 - Deprecated: Use [`assert.deepStrictEqual()`][] instead.
Tests for deep equality between the `actual` and `expected` parameters.
Primitive values are compared with the [Abstract Equality Comparison][]
( `==` ).
Tests for deep equality between the `actual` and `expected` parameters. Consider
using [`assert.deepStrictEqual()`][] instead. [`assert.deepEqual()`][] can have
potentially surprising results.
Only [enumerable "own" properties][] are considered. The
[`assert.deepEqual()`][] implementation does not test the
[`[[Prototype]]`][prototype-spec] of objects or enumerable own [`Symbol`][]
properties. For such checks, consider using [`assert.deepStrictEqual()`][]
instead. [`assert.deepEqual()`][] can have potentially surprising results. The
following example does not throw an `AssertionError` because the properties on
the [`RegExp`][] object are not enumerable:
"Deep" equality means that the enumerable "own" properties of child objects
are also recursively evaluated by the following rules.
### Comparison details
* Primitive values are compared with the [Abstract Equality Comparison][]
( `==` ).
* [Type tags][Object.prototype.toString()] of objects should be the same.
* Only [enumerable "own" properties][] are considered.
* [`Error`][] names and messages are always compared, even if these are not
enumerable properties.
* [Object wrappers][] are compared both as objects and unwrapped values.
* `Object` properties are compared unordered.
* [`Map`][] keys and [`Set`][] items are compared unordered.
* Recursion stops when both sides differ or both sides encounter a circular
reference.
* Implementation does not test the [`[[Prototype]]`][prototype-spec] of
objects.
* [`Symbol`][] properties are not compared.
* [`WeakMap`][] and [`WeakSet`][] comparison does not rely on their values.
The following example does not throw an `AssertionError` because the primitives
are considered equal by the [Abstract Equality Comparison][] ( `==` ).
```js
// WARNING: This does not throw an AssertionError!
assert.deepEqual(/a/gi, new Date());
assert.deepEqual('+00000000', false);
```
An exception is made for [`Map`][] and [`Set`][]. `Map`s and `Set`s have their
contained items compared too, as expected.
"Deep" equality means that the enumerable "own" properties of child objects
are evaluated also:
@ -304,7 +321,7 @@ are recursively evaluated also by the following rules.
* Enumerable own [`Symbol`][] properties are compared as well.
* [Object wrappers][] are compared both as objects and unwrapped values.
* `Object` properties are compared unordered.
* `Map` keys and `Set` items are compared unordered.
* [`Map`][] keys and [`Set`][] items are compared unordered.
* Recursion stops when both sides differ or both sides encounter a circular
reference.
* [`WeakMap`][] and [`WeakSet`][] comparison does not rely on their values. See

153
lib/internal/util/comparisons.js
View File

@ -14,7 +14,9 @@ const {
isStringObject,
isBooleanObject,
isBigIntObject,
isSymbolObject
isSymbolObject,
isFloat32Array,
isFloat64Array
} = require('internal/util/types');
const {
getOwnNonIndexProperties,
@ -84,18 +86,6 @@ function areEqualArrayBuffers(buf1, buf2) {
compare(new Uint8Array(buf1), new Uint8Array(buf2)) === 0;
}
function isFloatTypedArrayTag(tag) {
return tag === '[object Float32Array]' || tag === '[object Float64Array]';
}
function isArguments(tag) {
return tag === '[object Arguments]';
}
function isObjectOrArrayTag(tag) {
return tag === '[object Array]' || tag === '[object Object]';
}
function isEqualBoxedPrimitive(val1, val2) {
if (isNumberObject(val1)) {
return isNumberObject(val2) &&
@ -132,13 +122,38 @@ function isEqualBoxedPrimitive(val1, val2) {
// For strict comparison, objects should have
// a) The same built-in type tags
// b) The same prototypes.
function strictDeepEqual(val1, val2, memos) {
if (typeof val1 !== 'object') {
return typeof val1 === 'number' && numberIsNaN(val1) &&
numberIsNaN(val2);
function innerDeepEqual(val1, val2, strict, memos) {
// All identical values are equivalent, as determined by ===.
if (val1 === val2) {
if (val1 !== 0)
return true;
return strict ? objectIs(val1, val2) : true;
}
if (typeof val2 !== 'object' || val1 === null || val2 === null) {
return false;
// Check more closely if val1 and val2 are equal.
if (strict) {
if (typeof val1 !== 'object') {
return typeof val1 === 'number' && numberIsNaN(val1) &&
numberIsNaN(val2);
}
if (typeof val2 !== 'object' || val1 === null || val2 === null) {
return false;
}
if (getPrototypeOf(val1) !== getPrototypeOf(val2)) {
return false;
}
} else {
if (val1 === null || typeof val1 !== 'object') {
if (val2 === null || typeof val2 !== 'object') {
// eslint-disable-next-line eqeqeq
return val1 == val2;
}
return false;
}
if (val2 === null || typeof val2 !== 'object') {
return false;
}
}
const val1Tag = objectToString(val1);
const val2Tag = objectToString(val2);
@ -146,9 +161,6 @@ function strictDeepEqual(val1, val2, memos) {
if (val1Tag !== val2Tag) {
return false;
}
if (getPrototypeOf(val1) !== getPrototypeOf(val2)) {
return false;
}
if (Array.isArray(val1)) {
// Check for sparse arrays and general fast path
if (val1.length !== val2.length) {
@ -159,10 +171,10 @@ function strictDeepEqual(val1, val2, memos) {
if (keys1.length !== keys2.length) {
return false;
}
return keyCheck(val1, val2, kStrict, memos, kIsArray, keys1);
return keyCheck(val1, val2, strict, memos, kIsArray, keys1);
}
if (val1Tag === '[object Object]') {
return keyCheck(val1, val2, kStrict, memos, kNoIterator);
return keyCheck(val1, val2, strict, memos, kNoIterator);
}
if (isDate(val1)) {
if (dateGetTime(val1) !== dateGetTime(val2)) {
@ -174,13 +186,16 @@ function strictDeepEqual(val1, val2, memos) {
}
} else if (isNativeError(val1) || val1 instanceof Error) {
// Do not compare the stack as it might differ even though the error itself
// is otherwise identical. The non-enumerable name should be identical as
// the prototype is also identical. Otherwise this is caught later on.
if (val1.message !== val2.message) {
// is otherwise identical.
if (val1.message !== val2.message || val1.name !== val2.name) {
return false;
}
} else if (isArrayBufferView(val1)) {
if (!areSimilarTypedArrays(val1, val2)) {
if (!strict && (isFloat32Array(val1) || isFloat64Array(val1))) {
if (!areSimilarFloatArrays(val1, val2)) {
return false;
}
} else if (!areSimilarTypedArrays(val1, val2)) {
return false;
}
// Buffer.compare returns true, so val1.length === val2.length. If they both
@ -191,17 +206,17 @@ function strictDeepEqual(val1, val2, memos) {
if (keys1.length !== keys2.length) {
return false;
}
return keyCheck(val1, val2, kStrict, memos, kNoIterator, keys1);
return keyCheck(val1, val2, strict, memos, kNoIterator, keys1);
} else if (isSet(val1)) {
if (!isSet(val2) || val1.size !== val2.size) {
return false;
}
return keyCheck(val1, val2, kStrict, memos, kIsSet);
return keyCheck(val1, val2, strict, memos, kIsSet);
} else if (isMap(val1)) {
if (!isMap(val2) || val1.size !== val2.size) {
return false;
}
return keyCheck(val1, val2, kStrict, memos, kIsMap);
return keyCheck(val1, val2, strict, memos, kIsMap);
} else if (isAnyArrayBuffer(val1)) {
if (!areEqualArrayBuffers(val1, val2)) {
return false;
@ -209,66 +224,7 @@ function strictDeepEqual(val1, val2, memos) {
} else if (isBoxedPrimitive(val1) && !isEqualBoxedPrimitive(val1, val2)) {
return false;
}
return keyCheck(val1, val2, kStrict, memos, kNoIterator);
}
function looseDeepEqual(val1, val2, memos) {
if (val1 === null || typeof val1 !== 'object') {
if (val2 === null || typeof val2 !== 'object') {
// eslint-disable-next-line eqeqeq
return val1 == val2;
}
return false;
}
if (val2 === null || typeof val2 !== 'object') {
return false;
}
const val1Tag = objectToString(val1);
const val2Tag = objectToString(val2);
if (val1Tag === val2Tag) {
if (isObjectOrArrayTag(val1Tag)) {
return keyCheck(val1, val2, kLoose, memos, kNoIterator);
}
if (isArrayBufferView(val1)) {
if (isFloatTypedArrayTag(val1Tag)) {
return areSimilarFloatArrays(val1, val2);
}
return areSimilarTypedArrays(val1, val2);
}
if (isDate(val1) && isDate(val2)) {
return val1.getTime() === val2.getTime();
}
if (isRegExp(val1) && isRegExp(val2)) {
return areSimilarRegExps(val1, val2);
}
if (val1 instanceof Error && val2 instanceof Error) {
if (val1.message !== val2.message || val1.name !== val2.name)
return false;
}
// Ensure reflexivity of deepEqual with `arguments` objects.
// See https://github.com/nodejs/node-v0.x-archive/pull/7178
} else if (isArguments(val1Tag) || isArguments(val2Tag)) {
return false;
}
if (isSet(val1)) {
if (!isSet(val2) || val1.size !== val2.size) {
return false;
}
return keyCheck(val1, val2, kLoose, memos, kIsSet);
} else if (isMap(val1)) {
if (!isMap(val2) || val1.size !== val2.size) {
return false;
}
return keyCheck(val1, val2, kLoose, memos, kIsMap);
} else if (isSet(val2) || isMap(val2)) {
return false;
}
if (isAnyArrayBuffer(val1) && isAnyArrayBuffer(val2)) {
if (!areEqualArrayBuffers(val1, val2)) {
return false;
}
}
return keyCheck(val1, val2, kLoose, memos, kNoIterator);
return keyCheck(val1, val2, strict, memos, kNoIterator);
}
function getEnumerables(val, keys) {
@ -370,21 +326,6 @@ function keyCheck(val1, val2, strict, memos, iterationType, aKeys) {
return areEq;
}
function innerDeepEqual(val1, val2, strict, memos) {
// All identical values are equivalent, as determined by ===.
if (val1 === val2) {
if (val1 !== 0)
return true;
return strict ? objectIs(val1, val2) : true;
}
// Check more closely if val1 and val2 are equal.
if (strict === true)
return strictDeepEqual(val1, val2, memos);
return looseDeepEqual(val1, val2, memos);
}
function setHasEqualElement(set, val1, strict, memo) {
// Go looking.
for (const val2 of set) {

9
test/es-module/test-esm-dynamic-import.js
View File

@ -23,9 +23,12 @@ function expectMissingModuleError(result) {
function expectOkNamespace(result) {
Promise.resolve(result)
.then(common.mustCall((ns) => {
// Can't deepStrictEqual because ns isn't a normal object
// eslint-disable-next-line no-restricted-properties
assert.deepEqual(ns, { default: true });
const expected = { default: true };
Object.defineProperty(expected, Symbol.toStringTag, {
value: 'Module'
});
Object.setPrototypeOf(expected, Object.getPrototypeOf(ns));
assert.deepStrictEqual(ns, expected);
}));
}

8
test/parallel/test-assert-checktag.js
View File

@ -18,8 +18,8 @@ if (process.stdout.isTTY)
FakeDate.prototype = Date.prototype;
const fake = new FakeDate();
assert.deepEqual(date, fake);
assert.deepEqual(fake, date);
assert.notDeepEqual(date, fake);
assert.notDeepEqual(fake, date);
// For deepStrictEqual we check the runtime type,
// then reveal the fakeness of the fake date
@ -45,7 +45,7 @@ if (process.stdout.isTTY)
for (const prop of Object.keys(global)) {
fakeGlobal[prop] = global[prop];
}
assert.deepEqual(fakeGlobal, global);
assert.notDeepEqual(fakeGlobal, global);
// Message will be truncated anyway, don't validate
assert.throws(() => assert.deepStrictEqual(fakeGlobal, global),
assert.AssertionError);
@ -57,7 +57,7 @@ if (process.stdout.isTTY)
for (const prop of Object.keys(process)) {
fakeProcess[prop] = process[prop];
}
assert.deepEqual(fakeProcess, process);
assert.notDeepEqual(fakeProcess, process);
// Message will be truncated anyway, don't validate
assert.throws(() => assert.deepStrictEqual(fakeProcess, process),
assert.AssertionError);

50
test/parallel/test-assert-deep.js
View File

@ -75,7 +75,7 @@ assert.deepEqual(arr, buf);
' ]'
}
);
assert.deepEqual(buf2, buf);
assert.notDeepEqual(buf2, buf);
}
{
@ -94,7 +94,7 @@ assert.deepEqual(arr, buf);
' ]'
}
);
assert.deepEqual(arr, arr2);
assert.notDeepEqual(arr, arr2);
}
const date = new Date('2016');
@ -110,8 +110,8 @@ const date2 = new MyDate('2016');
// deepEqual returns true as long as the time are the same,
// but deepStrictEqual checks own properties
assert.deepEqual(date, date2);
assert.deepEqual(date2, date);
assert.notDeepEqual(date, date2);
assert.notDeepEqual(date2, date);
assert.throws(
() => assert.deepStrictEqual(date, date2),
{
@ -143,7 +143,7 @@ const re2 = new MyRegExp('test');
// deepEqual returns true as long as the regexp-specific properties
// are the same, but deepStrictEqual checks all properties
assert.deepEqual(re1, re2);
assert.notDeepEqual(re1, re2);
assert.throws(
() => assert.deepStrictEqual(re1, re2),
{
@ -157,28 +157,24 @@ assert.throws(
// but deepStrictEqual should throw.
{
const similar = new Set([
{ 0: '1' }, // Object
{ 0: 1 }, // Object
new String('1'), // Object
['1'], // Array
[1], // Array
date2, // Date with this[0] = '1'
re2, // RegExp with this[0] = '1'
new Int8Array([1]), // Int8Array
new Uint8Array([1]), // Uint8Array
new Int16Array([1]), // Int16Array
new Uint16Array([1]), // Uint16Array
new Int32Array([1]), // Int32Array
new Uint32Array([1]), // Uint32Array
Buffer.from([1]),
// Arguments {'0': '1'} is not here
// See https://github.com/nodejs/node-v0.x-archive/pull/7178
Buffer.from([1]), // Uint8Array
(function() { return arguments; })(1)
]);
for (const a of similar) {
for (const b of similar) {
if (a !== b) {
assert.deepEqual(a, b);
assert.notDeepEqual(a, b);
assert.throws(
() => assert.deepStrictEqual(a, b),
{ code: 'ERR_ASSERTION' }
@ -576,7 +572,7 @@ assertNotDeepOrStrict(
// Handle sparse arrays.
{
assertDeepAndStrictEqual([1, , , 3], [1, , , 3]);
assertOnlyDeepEqual([1, , , 3], [1, , , 3, , , ]);
assertNotDeepOrStrict([1, , , 3], [1, , , 3, , , ]);
const a = new Array(3);
const b = new Array(3);
a[2] = true;
@ -594,13 +590,11 @@ assertNotDeepOrStrict(
assertNotDeepOrStrict(err1, new Error('foo2'), assert.AssertionError);
assertNotDeepOrStrict(err1, new TypeError('foo1'), assert.AssertionError);
assertDeepAndStrictEqual(err1, new Error('foo1'));
// TODO: evaluate if this should throw or not. The same applies for RegExp
// Date and any object that has the same keys but not the same prototype.
assertOnlyDeepEqual(err1, {});
assertNotDeepOrStrict(err1, {}, AssertionError);
}
// Handle NaN
assert.throws(() => { assert.deepEqual(NaN, NaN); }, assert.AssertionError);
assert.notDeepEqual(NaN, NaN);
assert.deepStrictEqual(NaN, NaN);
assert.deepStrictEqual({ a: NaN }, { a: NaN });
assert.deepStrictEqual([ 1, 2, NaN, 4 ], [ 1, 2, NaN, 4 ]);
@ -609,11 +603,11 @@ assert.deepStrictEqual([ 1, 2, NaN, 4 ], [ 1, 2, NaN, 4 ]);
{
const boxedString = new String('test');
const boxedSymbol = Object(Symbol());
assertOnlyDeepEqual(new Boolean(true), Object(false));
assertOnlyDeepEqual(Object(true), new Number(1));
assertOnlyDeepEqual(new Number(2), new Number(1));
assertOnlyDeepEqual(boxedSymbol, Object(Symbol()));
assertOnlyDeepEqual(boxedSymbol, {});
assertNotDeepOrStrict(new Boolean(true), Object(false));
assertNotDeepOrStrict(Object(true), new Number(1));
assertNotDeepOrStrict(new Number(2), new Number(1));
assertNotDeepOrStrict(boxedSymbol, Object(Symbol()));
assertNotDeepOrStrict(boxedSymbol, {});
assertDeepAndStrictEqual(boxedSymbol, boxedSymbol);
assertDeepAndStrictEqual(Object(true), Object(true));
assertDeepAndStrictEqual(Object(2), Object(2));
@ -645,7 +639,7 @@ assertDeepAndStrictEqual(-0, -0);
const b = new Uint8Array(4);
a[symbol1] = true;
b[symbol1] = false;
assertOnlyDeepEqual(a, b);
assertNotDeepOrStrict(a, b);
b[symbol1] = true;
assertDeepAndStrictEqual(a, b);
// The same as TypedArrays is valid for boxed primitives
@ -700,7 +694,7 @@ assert.deepEqual({ a: 4, b: '2' }, { a: 4, b: '2' });
assert.deepEqual([4], ['4']);
assert.throws(
() => assert.deepEqual({ a: 4 }, { a: 4, b: true }), AssertionError);
assert.deepEqual(['a'], { 0: 'a' });
assert.notDeepEqual(['a'], { 0: 'a' });
assert.deepEqual({ a: 4, b: '1' }, { b: '1', a: 4 });
const a1 = [1, 2, 3];
const a2 = [1, 2, 3];
@ -759,10 +753,10 @@ assertOnlyDeepEqual(true, 1);
}
// Primitive wrappers and object.
assertOnlyDeepEqual(new String('a'), ['a']);
assertOnlyDeepEqual(new String('a'), { 0: 'a' });
assertOnlyDeepEqual(new Number(1), {});
assertOnlyDeepEqual(new Boolean(true), {});
assertNotDeepOrStrict(new String('a'), ['a']);
assertNotDeepOrStrict(new String('a'), { 0: 'a' });
assertNotDeepOrStrict(new Number(1), {});
assertNotDeepOrStrict(new Boolean(true), {});
// Same number of keys but different key names.
assertNotDeepOrStrict({ a: 1 }, { b: 1 });

10
test/parallel/test-assert-typedarray-deepequal.js
View File

@ -29,15 +29,15 @@ const equalArrayPairs = [
];
const looseEqualArrayPairs = [
[new Float64Array([+0.0]), new Float32Array([-0.0])],
[new Int16Array(256), new Uint16Array(256)],
[new Int16Array([256]), new Uint16Array([256])],
[new Float32Array([+0.0]), new Float32Array([-0.0])],
[new Float64Array([+0.0]), new Float64Array([-0.0])],
[new ArrayBuffer(3), new SharedArrayBuffer(3)]
[new Float64Array([+0.0]), new Float64Array([-0.0])]
];
const notEqualArrayPairs = [
[new ArrayBuffer(3), new SharedArrayBuffer(3)],
[new Int16Array(256), new Uint16Array(256)],
[new Int16Array([256]), new Uint16Array([256])],
[new Float64Array([+0.0]), new Float32Array([-0.0])],
[new Uint8Array(2), new Uint8Array(3)],
[new Uint8Array([1, 2, 3]), new Uint8Array([4, 5, 6])],
[new Uint8ClampedArray([300, 2, 3]), new Uint8Array([300, 2, 3])],