Skip to content

Commit

Permalink
assert: var -> const and added tests
Browse files Browse the repository at this point in the history
Cleaned up as per comments in issue

Ref: nodejs#6416
  • Loading branch information
josephg committed Mar 31, 2017
1 parent 031f6f3 commit d6baaee
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 11 deletions.
19 changes: 8 additions & 11 deletions lib/assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ function _deepEqual(actual, expected, strict, memos) {
// a) The same number of owned enumerable properties
// b) The same set of keys/indexes (although not necessarily the same order)
// c) Equivalent values for every corresponding key/index
// d) For Maps, strict-equal keys mapping to deep-equal values
// d) For Sets and Maps, equal contents
// Note: this accounts for both named and indexed properties on Arrays.

// Use memos to handle cycles.
Expand All @@ -291,22 +291,20 @@ function setEquiv(a, b, strict, actualVisitedObjects) {
// assert.deepEqual(new Set(['1', 1]), new Set([1]))
//
// In theory, all the items in the first set have a corresponding == value in
// the second set, but the sets have different sizes. Should they be
// considered to be non-strict deep equal to one another? Its a silly case,
// the second set, but the sets have different sizes. Its a silly case,
// and more evidence that deepStrictEqual should always be preferred over
// deepEqual. The implementation currently returns false, which is a simpler
// and faster implementation.
// deepEqual. The implementation currently returns false, which is simpler
// and slightly faster.
if (a.size !== b.size)
return false;

var val1, val2;
outer: for (val1 of a) {
outer: for (const val1 of a) {
if (!b.has(val1)) {
// The value doesn't exist in the second set by reference, so we'll go
// hunting for something thats deep-equal to it. Note that this is O(n^2)
// complexity, and will get slower if large, very similar sets / maps are
// nested inside. Unfortunately there's no real way around this.
for (val2 of b) {
for (const val2 of b) {
if (_deepEqual(val1, val2, strict, actualVisitedObjects)) {
continue outer;
}
Expand All @@ -328,8 +326,7 @@ function mapEquiv(a, b, strict, actualVisitedObjects) {
if (a.size !== b.size)
return false;

var key1, key2, item1, item2;
outer: for ([key1, item1] of a) {
outer: for (const [key1, item1] of a) {
// To be able to handle cases like:
// Map([[1, 'a'], ['1', 'b']]) vs Map([['1', 'a'], [1, 'b']])
// or:
Expand All @@ -346,7 +343,7 @@ function mapEquiv(a, b, strict, actualVisitedObjects) {

// Hunt for keys which are deep-equal to key1 in b. Just like setEquiv
// above, this hunt makes this function O(n^2).
for ([key2, item2] of b) {
for (const [key2, item2] of b) {
// Just for performance. We already checked these keys above.
if (key2 === key1)
continue;
Expand Down
21 changes: 21 additions & 0 deletions test/parallel/test-assert-deep.js
Original file line number Diff line number Diff line change
Expand Up @@ -245,4 +245,25 @@ assert.throws(() =>
assertNotDeepOrStrict(m1, m2);
}

{
// Circular references.
const s1 = new Set();
s1.add(s1);
const s2 = new Set();
s2.add(s2);
assertDeepAndStrictEqual(s1, s2);

const m1 = new Map();
m1.set(2, m1);
const m2 = new Map();
m2.set(2, m2);
assertDeepAndStrictEqual(m1, m2);

const m3 = new Map();
m3.set(m3, 2);
const m4 = new Map();
m4.set(m4, 2);
assertDeepAndStrictEqual(m3, m4);
}

/* eslint-enable */

0 comments on commit d6baaee

Please sign in to comment.