Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

assert: use Same-value equality in deepStrictEqual #15398

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions doc/api/assert.md
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,9 @@ parameter is an instance of an `Error` then it will be thrown instead of the
<!-- YAML
added: v1.2.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/REPLACEME
description: -0 and +0 are not considered equal anymore.
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/15036
description: NaN is now compared using the [SameValueZero][] comparison.
Expand Down Expand Up @@ -144,6 +147,7 @@ Generally identical to `assert.deepEqual()` with a few exceptions:
the [Strict Equality Comparison][] too.
3. [Type tags][Object.prototype.toString()] of objects should be the same.
4. [Object wrappers][] are compared both as objects and unwrapped values.
5. `0` and `-0` are not considered equal.

```js
const assert = require('assert');
Expand Down Expand Up @@ -181,6 +185,11 @@ assert.deepStrictEqual(new Number(1), new Number(2));
// Fails because the wrapped number is unwrapped and compared as well.
assert.deepStrictEqual(new String('foo'), Object('foo'));
// OK because the object and the string are identical when unwrapped.

assert.deepStrictEqual(-0, -0);
// OK
assert.deepStrictEqual(0, -0);
// AssertionError: 0 deepStrictEqual -0
```

If the values are not equal, an `AssertionError` is thrown with a `message`
Expand Down
25 changes: 13 additions & 12 deletions lib/assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -118,12 +118,12 @@ function areSimilarRegExps(a, b) {
// For small buffers it's faster to compare the buffer in a loop. The c++
// barrier including the Uint8Array operation takes the advantage of the faster
// binary compare otherwise. The break even point was at about 300 characters.
function areSimilarTypedArrays(a, b) {
function areSimilarTypedArrays(a, b, max) {
const len = a.byteLength;
if (len !== b.byteLength) {
return false;
}
if (len < 300) {
if (len < max) {
for (var offset = 0; offset < len; offset++) {
if (a[offset] !== b[offset]) {
return false;
Expand Down Expand Up @@ -160,10 +160,7 @@ function isObjectOrArrayTag(tag) {
// reasonable to interpret their underlying memory in the same way,
// which is checked by comparing their type tags.
// (e.g. a Uint8Array and a Uint16Array with the same memory content
// could still be different because they will be interpreted differently)
// Never perform binary comparisons for Float*Arrays, though,
// since e.g. +0 === -0 is true despite the two values' bit patterns
// not being identical.
// could still be different because they will be interpreted differently).
//
// For strict comparison, objects should have
// a) The same built-in type tags
Expand Down Expand Up @@ -211,8 +208,9 @@ function strictDeepEqual(actual, expected) {
if (actual.message !== expected.message) {
return false;
}
} else if (!isFloatTypedArrayTag(actualTag) && ArrayBuffer.isView(actual)) {
if (!areSimilarTypedArrays(actual, expected)) {
} else if (ArrayBuffer.isView(actual)) {
if (!areSimilarTypedArrays(actual, expected,
isFloatTypedArrayTag(actualTag) ? 0 : 300)) {
return false;
}
// Buffer.compare returns true, so actual.length === expected.length
Expand Down Expand Up @@ -266,9 +264,10 @@ function looseDeepEqual(actual, expected) {
const actualTag = objectToString(actual);
const expectedTag = objectToString(expected);
if (actualTag === expectedTag) {
if (!isObjectOrArrayTag(actualTag) && !isFloatTypedArrayTag(actualTag) &&
ArrayBuffer.isView(actual)) {
return areSimilarTypedArrays(actual, expected);
if (!isObjectOrArrayTag(actualTag) && ArrayBuffer.isView(actual)) {
return areSimilarTypedArrays(actual, expected,
isFloatTypedArrayTag(actualTag) ?
Infinity : 300);
}
// Ensure reflexivity of deepEqual with `arguments` objects.
// See https://github.com/nodejs/node-v0.x-archive/pull/7178
Expand All @@ -280,7 +279,9 @@ function looseDeepEqual(actual, expected) {
function innerDeepEqual(actual, expected, strict, memos) {
// All identical values are equivalent, as determined by ===.
if (actual === expected) {
return true;
if (actual !== 0)
return true;
return strict ? Object.is(actual, expected) : true;
}

// Returns a boolean if (not) equal and undefined in case we have to check
Expand Down
3 changes: 3 additions & 0 deletions test/parallel/test-assert-deep.js
Original file line number Diff line number Diff line change
Expand Up @@ -507,5 +507,8 @@ assert.doesNotThrow(
boxedSymbol.slow = true;
assertNotDeepOrStrict(boxedSymbol, {});
}
// Minus zero
assertOnlyDeepEqual(0, -0);
assertDeepAndStrictEqual(-0, -0);

/* eslint-enable */
28 changes: 23 additions & 5 deletions test/parallel/test-assert-typedarray-deepequal.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,20 @@ const equalArrayPairs = [
[new Int32Array(1e5), new Int32Array(1e5)],
[new Float32Array(1e5), new Float32Array(1e5)],
[new Float64Array(1e5), new Float64Array(1e5)],
[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 Float32Array([-0.0])],
[new Float64Array([+0.0]), new Float64Array([-0.0])],
[new Float32Array([+0.0]), new Float32Array([+0.0])],
[new Uint8Array([1, 2, 3, 4]).subarray(1), new Uint8Array([2, 3, 4])],
[new Uint16Array([1, 2, 3, 4]).subarray(1), new Uint16Array([2, 3, 4])],
[new Uint32Array([1, 2, 3, 4]).subarray(1, 3), new Uint32Array([2, 3])]
];

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])]
];

const notEqualArrayPairs = [
[new Uint8Array(2), new Uint8Array(3)],
[new Uint8Array([1, 2, 3]), new Uint8Array([4, 5, 6])],
Expand All @@ -46,6 +50,16 @@ const notEqualArrayPairs = [
equalArrayPairs.forEach((arrayPair) => {
// eslint-disable-next-line no-restricted-properties
assert.deepEqual(arrayPair[0], arrayPair[1]);
assert.deepStrictEqual(arrayPair[0], arrayPair[1]);
});

looseEqualArrayPairs.forEach((arrayPair) => {
// eslint-disable-next-line no-restricted-properties
assert.deepEqual(arrayPair[0], arrayPair[1]);
assert.throws(
makeBlock(assert.deepStrictEqual, arrayPair[0], arrayPair[1]),
assert.AssertionError
);
});

notEqualArrayPairs.forEach((arrayPair) => {
Expand All @@ -54,4 +68,8 @@ notEqualArrayPairs.forEach((arrayPair) => {
makeBlock(assert.deepEqual, arrayPair[0], arrayPair[1]),
assert.AssertionError
);
assert.throws(
makeBlock(assert.deepStrictEqual, arrayPair[0], arrayPair[1]),
assert.AssertionError
);
});