Skip to content

Commit

Permalink
assert: check object prototype in deepEqual
Browse files Browse the repository at this point in the history
Objects with different prototypes should not be considered deep equal,
e.g. `assert.deepEqual({}, [])` should throw. Previously `deepEqual`
was implemented as per CommonJS Unit Testing/1.0 spec. It states that
objects should have 'an identical "prototype" property' to be
considered deep equal, which is incorrect. Instead object prototypes
should be compared, i.e. `Object.getPrototypeOf` or `__proto__`.

Fixes: #620
  • Loading branch information
vkurchatkin committed Jan 27, 2015
1 parent 69ce064 commit 5fd9451
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 11 deletions.
5 changes: 3 additions & 2 deletions lib/assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -199,11 +199,12 @@ function isArguments(object) {
function objEquiv(a, b) {
if (util.isNullOrUndefined(a) || util.isNullOrUndefined(b))
return false;
// an identical 'prototype' property.
if (a.prototype !== b.prototype) return false;
// if one is a primitive, the other must be same
if (util.isPrimitive(a) || util.isPrimitive(b))
return a === b;
// an identical prototype.
if (Object.getPrototypeOf(a) !== Object.getPrototypeOf(b))
return false;
var aIsArgs = isArguments(a),
bIsArgs = isArguments(b);
if ((aIsArgs && !bIsArgs) || (!aIsArgs && bIsArgs))
Expand Down
16 changes: 11 additions & 5 deletions test/parallel/test-assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ assert.doesNotThrow(makeBlock(a.deepEqual, {a: 4, b: '2'}, {a: 4, b: '2'}));
assert.doesNotThrow(makeBlock(a.deepEqual, [4], ['4']));
assert.throws(makeBlock(a.deepEqual, {a: 4}, {a: 4, b: true}),
a.AssertionError);
assert.doesNotThrow(makeBlock(a.deepEqual, ['a'], {0: 'a'}));
assert.throws(makeBlock(a.deepEqual, ['a'], {0: 'a'}));
//(although not necessarily the same order),
assert.doesNotThrow(makeBlock(a.deepEqual, {a: 4, b: '1'}, {b: '1', a: 4}));
var a1 = [1, 2, 3];
Expand Down Expand Up @@ -144,10 +144,16 @@ if (typeof Symbol === 'symbol') {
}

// primitive wrappers and object
assert.doesNotThrow(makeBlock(a.deepEqual, new String('a'), ['a']), a.AssertionError);
assert.doesNotThrow(makeBlock(a.deepEqual, new String('a'), {0: 'a'}), a.AssertionError);
assert.doesNotThrow(makeBlock(a.deepEqual, new Number(1), {}), a.AssertionError);
assert.doesNotThrow(makeBlock(a.deepEqual, new Boolean(true), {}), a.AssertionError);
assert.throws(makeBlock(a.deepEqual, new String('a'), ['a']),
a.AssertionError);
assert.throws(makeBlock(a.deepEqual, new String('a'),{0: 'a'}),
a.AssertionError);
assert.throws(makeBlock(a.deepEqual, new Number(1), {}), a.AssertionError);
assert.throws(makeBlock(a.deepEqual, new Boolean(true), {}), a.AssertionError);

// prototypes
assert.throws(makeBlock(assert.deepEqual, Object.create({}), {}),
a.AssertionError);

// Testing the throwing
function thrower(errorConstructor) {
Expand Down
6 changes: 3 additions & 3 deletions test/parallel/test-url.js
Original file line number Diff line number Diff line change
Expand Up @@ -867,8 +867,8 @@ for (var u in parseTests) {
}
});

assert.deepEqual(actual, expected);
assert.deepEqual(spaced, expected);
assert.deepEqual(JSON.parse(JSON.stringify(actual)), expected);
assert.deepEqual(JSON.parse(JSON.stringify(spaced)), expected);

var expected = parseTests[u].href,
actual = url.format(parseTests[u]);
Expand Down Expand Up @@ -937,7 +937,7 @@ for (var u in parseTestsWithQueryString) {
}
}

assert.deepEqual(actual, expected);
assert.deepEqual(JSON.parse(JSON.stringify(actual)), expected);
}

// some extra formatting tests, just to verify
Expand Down
3 changes: 2 additions & 1 deletion test/parallel/test-vm-create-context-accessors.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,5 @@ Object.defineProperty(ctx, 'setter', {
ctx = vm.createContext(ctx);

var result = vm.runInContext('setter = "test";[getter,setter]', ctx);
assert.deepEqual(result, ['ok', 'ok=test']);
assert.equal(result[0], 'ok');
assert.equal(result[1], 'ok=test');

0 comments on commit 5fd9451

Please sign in to comment.