Skip to content

Commit

Permalink
assert: fix deepEqual inconsistencies
Browse files Browse the repository at this point in the history
PR-URL: #14491
Fixes: #14441
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Khaidi Chu <i@2333.moe>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
  • Loading branch information
BridgeAR authored and jasnell committed Sep 20, 2017
1 parent 7d95dc3 commit 7fa175f
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 4 deletions.
11 changes: 9 additions & 2 deletions lib/assert.js
Expand Up @@ -279,8 +279,15 @@ function innerDeepEqual(actual, expected, strict, memos) {
position: 0
};
} else {
if (memos.actual.has(actual)) {
return memos.actual.get(actual) === memos.expected.get(expected);
// We prevent up to two map.has(x) calls by directly retrieving the value
// and checking for undefined. The map can only contain numbers, so it is
// safe to check for undefined only.
const expectedMemoA = memos.actual.get(actual);
if (expectedMemoA !== undefined) {
const expectedMemoB = memos.expected.get(expected);
if (expectedMemoB !== undefined) {
return expectedMemoA === expectedMemoB;
}
}
memos.position++;
}
Expand Down
20 changes: 20 additions & 0 deletions test/parallel/test-assert-deep.js
Expand Up @@ -303,6 +303,26 @@ assertOnlyDeepEqual(
new Set([undefined])
);

// Circular structures
{
const a = {};
const b = {};
a.a = a;
b.a = {};
b.a.a = a;
assertDeepAndStrictEqual(a, b);
}

{
const a = new Set();
const b = new Set();
const c = new Set();
a.add(a);
b.add(b);
c.add(a);
assertDeepAndStrictEqual(b, c);
}

{
const values = [
123,
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-assert.js
Expand Up @@ -550,8 +550,8 @@ a.throws(makeBlock(thrower, TypeError), function(err) {

const h = {ref: g};

a.throws(makeBlock(a.deepEqual, f, h), /AssertionError/);
a.throws(makeBlock(a.deepStrictEqual, f, h), /AssertionError/);
a.doesNotThrow(makeBlock(a.deepEqual, f, h));
a.doesNotThrow(makeBlock(a.deepStrictEqual, f, h));
}
// GH-7178. Ensure reflexivity of deepEqual with `arguments` objects.
const args = (function() { return arguments; })();
Expand Down

0 comments on commit 7fa175f

Please sign in to comment.