Permalink
Browse files

assert: adjust loose assertions

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: #25008
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
  • Loading branch information...
BridgeAR committed Dec 13, 2018
1 parent 5cb1964 commit 7493db21b667ed746d39c9b54357eac4287232e3
@@ -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.

"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.

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:
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
@@ -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,23 +122,45 @@ 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);

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,84 +206,25 @@ 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;
}
} 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) {
@@ -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);
}));
}

@@ -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);
Oops, something went wrong.

0 comments on commit 7493db2

Please sign in to comment.