1berry/nodejs
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

assert: remove unneeded arguments special handling

Browse Source 
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: https://github.com/nodejs/node/pull/7413
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This commit is contained in:
Rich Trott 2016-06-24 15:37:38 -07:00
parent c0e48bf27d
commit db21266427
2 changed files with 15 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
lib/assert.js
View File

@ -28,7 +28,6 @@
const compare = process.binding('buffer').compare; const compare = process.binding('buffer').compare;
const util = require('util'); const util = require('util');
const Buffer = require('buffer').Buffer; const Buffer = require('buffer').Buffer;
const pSlice = Array.prototype.slice;
const pToString = (obj) => Object.prototype.toString.call(obj); const pToString = (obj) => Object.prototype.toString.call(obj);
// 1. The assert module provides functions that throw // 1. The assert module provides functions that throw
@ -223,11 +222,6 @@ function objEquiv(a, b, strict, actualVisitedObjects) {
const bIsArgs = isArguments(b); const bIsArgs = isArguments(b);
if ((aIsArgs && !bIsArgs) || (!aIsArgs && bIsArgs)) if ((aIsArgs && !bIsArgs) || (!aIsArgs && bIsArgs))
return false; return false;
if (aIsArgs) {
a = pSlice.call(a);
b = pSlice.call(b);
return _deepEqual(a, b, strict);
}
const ka = Object.keys(a); const ka = Object.keys(a);
const kb = Object.keys(b); const kb = Object.keys(b);
var key, i; var key, i;

16
test/parallel/test-assert.js
View File

@ -59,7 +59,7 @@ assert.throws(makeBlock(a.strictEqual, null, undefined),
assert.doesNotThrow(makeBlock(a.notStrictEqual, 2, '2'), assert.doesNotThrow(makeBlock(a.notStrictEqual, 2, '2'),
'notStrictEqual(2, \'2\')'); 'notStrictEqual(2, \'2\')');
// deepEquals joy! // deepEqual joy!
// 7.2 // 7.2
assert.doesNotThrow(makeBlock(a.deepEqual, new Date(2000, 3, 14), assert.doesNotThrow(makeBlock(a.deepEqual, new Date(2000, 3, 14),
new Date(2000, 3, 14)), new Date(2000, 3, 14)),
@ -409,6 +409,20 @@ var args = (function() { return arguments; })();
a.throws(makeBlock(a.deepEqual, [], args)); a.throws(makeBlock(a.deepEqual, [], args));
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}; var circular = {y: 1};
circular.x = circular; circular.x = circular;