From dd2df003f6044af427f9a8b286a9e767d31c009c Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Tue, 25 Jul 2017 21:36:12 -0300 Subject: [PATCH 1/2] assert: fix deepEqual inconsistencies Fixes #14441 --- lib/assert.js | 11 +++++++++-- test/parallel/test-assert-deep.js | 20 ++++++++++++++++++++ test/parallel/test-assert.js | 4 ++-- 3 files changed, 31 insertions(+), 4 deletions(-) diff --git a/lib/assert.js b/lib/assert.js index f7a3350db10f1f..1350f0abbbb481 100644 --- a/lib/assert.js +++ b/lib/assert.js @@ -271,8 +271,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++; } diff --git a/test/parallel/test-assert-deep.js b/test/parallel/test-assert-deep.js index 51f07af09e05d0..b4f7fa47e3956f 100644 --- a/test/parallel/test-assert-deep.js +++ b/test/parallel/test-assert-deep.js @@ -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, diff --git a/test/parallel/test-assert.js b/test/parallel/test-assert.js index 65181996d5ace5..17adf33b8b3821 100644 --- a/test/parallel/test-assert.js +++ b/test/parallel/test-assert.js @@ -578,8 +578,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; })(); From b7ecf4db17da922756e93b4a1cc208bc41ea5d87 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Tue, 25 Jul 2017 22:10:46 -0300 Subject: [PATCH 2/2] test: clean up some assert deepEqual tests --- test/parallel/test-assert-deep.js | 62 ++++++++++++++++++++++++++----- test/parallel/test-assert.js | 56 ---------------------------- 2 files changed, 52 insertions(+), 66 deletions(-) diff --git a/test/parallel/test-assert-deep.js b/test/parallel/test-assert-deep.js index b4f7fa47e3956f..08b0b80d9d05f7 100644 --- a/test/parallel/test-assert-deep.js +++ b/test/parallel/test-assert-deep.js @@ -159,15 +159,17 @@ assertNotDeepOrStrict(new Set([1, 2, 3, 4]), new Set([1, 2, 3])); assertDeepAndStrictEqual(new Set(['1', '2', '3']), new Set(['1', '2', '3'])); assertDeepAndStrictEqual(new Set([[1, 2], [3, 4]]), new Set([[3, 4], [1, 2]])); -const a = [ 1, 2 ]; -const b = [ 3, 4 ]; -const c = [ 1, 2 ]; -const d = [ 3, 4 ]; - -assertDeepAndStrictEqual( - { a: a, b: b, s: new Set([a, b]) }, - { a: c, b: d, s: new Set([d, c]) } -); +{ + const a = [ 1, 2 ]; + const b = [ 3, 4 ]; + const c = [ 1, 2 ]; + const d = [ 3, 4 ]; + + assertDeepAndStrictEqual( + { a: a, b: b, s: new Set([a, b]) }, + { a: c, b: d, s: new Set([d, c]) } + ); +} assertDeepAndStrictEqual(new Map([[1, 1], [2, 2]]), new Map([[1, 1], [2, 2]])); assertDeepAndStrictEqual(new Map([[1, 1], [2, 2]]), new Map([[2, 2], [1, 1]])); @@ -303,7 +305,26 @@ assertOnlyDeepEqual( new Set([undefined]) ); -// Circular structures +// GH-6416. Make sure circular refs don't throw. +{ + const b = {}; + b.b = b; + const c = {}; + c.b = c; + + assertDeepAndStrictEqual(b, c); + + const d = {}; + d.a = 1; + d.b = d; + const e = {}; + e.a = 1; + e.b = {}; + + assertNotDeepOrStrict(d, e); +} + +// GH-14441. Circular structures should be consistent { const a = {}; const b = {}; @@ -323,6 +344,27 @@ assertOnlyDeepEqual( assertDeepAndStrictEqual(b, c); } +// GH-7178. Ensure reflexivity of deepEqual with `arguments` objects. +{ + const args = (function() { return arguments; })(); + assertNotDeepOrStrict([], args); +} + +// More checking that arguments objects are handled correctly +{ + // eslint-disable-next-line func-style + const returnArguments = function() { return arguments; }; + + const someArgs = returnArguments('a'); + const sameArgs = returnArguments('a'); + const diffArgs = returnArguments('b'); + + assertNotDeepOrStrict(someArgs, ['a']); + assertNotDeepOrStrict(someArgs, { '0': 'a' }); + assertNotDeepOrStrict(someArgs, diffArgs); + assertDeepAndStrictEqual(someArgs, sameArgs); +} + { const values = [ 123, diff --git a/test/parallel/test-assert.js b/test/parallel/test-assert.js index 17adf33b8b3821..164edc8dc25f2a 100644 --- a/test/parallel/test-assert.js +++ b/test/parallel/test-assert.js @@ -546,62 +546,6 @@ a.throws(makeBlock(thrower, TypeError), function(err) { assert.ok(threw); } -// https://github.com/nodejs/node/issues/6416 -// Make sure circular refs don't throw. -{ - const b = {}; - b.b = b; - - const c = {}; - c.b = c; - - a.doesNotThrow(makeBlock(a.deepEqual, b, c)); - a.doesNotThrow(makeBlock(a.deepStrictEqual, b, c)); - - const d = {}; - d.a = 1; - d.b = d; - - const e = {}; - e.a = 1; - e.b = e.a; - - a.throws(makeBlock(a.deepEqual, d, e), /AssertionError/); - a.throws(makeBlock(a.deepStrictEqual, d, e), /AssertionError/); - - // https://github.com/nodejs/node/issues/13314 - const f = {}; - f.ref = f; - - const g = {}; - g.ref = g; - - const h = { ref: g }; - - 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; })(); -a.throws(makeBlock(a.deepEqual, [], args)); -a.throws(makeBlock(a.deepEqual, args, [])); - -// more checking that arguments objects are handled correctly -{ - // eslint-disable-next-line func-style - const returnArguments = function() { return arguments; }; - - const someArgs = returnArguments('a'); - const sameArgs = returnArguments('a'); - const diffArgs = returnArguments('b'); - - a.throws(makeBlock(a.deepEqual, someArgs, ['a'])); - a.throws(makeBlock(a.deepEqual, ['a'], someArgs)); - a.throws(makeBlock(a.deepEqual, someArgs, { '0': 'a' })); - a.throws(makeBlock(a.deepEqual, someArgs, diffArgs)); - a.doesNotThrow(makeBlock(a.deepEqual, someArgs, sameArgs)); -} - // check messages from assert.throws() { const noop = () => {};