Skip to content

Commit

Permalink
assert: remove unneeded arguments special handling
Browse files Browse the repository at this point in the history
Remove special handling when asserting on a pair of arguments objects.
The code being removed will only run if both `expected` and `actual` are
arguments objects. Given that situation, the subsequent code for
handling everything else works just fine.

Tests added to confirm expected behavior.

This came about while trying to improve test coverage. The segment of
code removed had no test coverage. I was unable to write a test that
would both exercise the code and fail if the code was removed. Further
examination indicated that this was because the special handling was not
needed.

PR-URL: #7413
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
Trott authored and Fishrock123 committed Jul 5, 2016
1 parent c39f6c0 commit 40211e8
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 7 deletions.
6 changes: 0 additions & 6 deletions lib/assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
const compare = process.binding('buffer').compare;
const util = require('util');
const Buffer = require('buffer').Buffer;
const pSlice = Array.prototype.slice;
const pToString = (obj) => Object.prototype.toString.call(obj);

// 1. The assert module provides functions that throw
Expand Down Expand Up @@ -223,11 +222,6 @@ function objEquiv(a, b, strict, actualVisitedObjects) {
const bIsArgs = isArguments(b);
if ((aIsArgs && !bIsArgs) || (!aIsArgs && bIsArgs))
return false;
if (aIsArgs) {
a = pSlice.call(a);
b = pSlice.call(b);
return _deepEqual(a, b, strict);
}
const ka = Object.keys(a);
const kb = Object.keys(b);
var key, i;
Expand Down
16 changes: 15 additions & 1 deletion test/parallel/test-assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ assert.throws(makeBlock(a.strictEqual, null, undefined),
assert.doesNotThrow(makeBlock(a.notStrictEqual, 2, '2'),
'notStrictEqual(2, \'2\')');

// deepEquals joy!
// deepEqual joy!
// 7.2
assert.doesNotThrow(makeBlock(a.deepEqual, new Date(2000, 3, 14),
new Date(2000, 3, 14)),
Expand Down Expand Up @@ -409,6 +409,20 @@ var args = (function() { return arguments; })();
a.throws(makeBlock(a.deepEqual, [], args));
a.throws(makeBlock(a.deepEqual, args, []));

// more checking that arguments objects are handled correctly
{
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));
}

var circular = {y: 1};
circular.x = circular;
Expand Down

0 comments on commit 40211e8

Please sign in to comment.