From a4888926a2fc55a46092e1ee2b03f82cd263d0b7 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Fri, 24 Jun 2016 15:37:38 -0700 Subject: [PATCH] assert: remove unneeded arguments special handling 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 Reviewed-By: Colin Ihrig Reviewed-By: James M Snell --- lib/assert.js | 6 ------ test/parallel/test-assert.js | 16 +++++++++++++++- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/lib/assert.js b/lib/assert.js index 93b5876487deda..c0f222d510e6c5 100644 --- a/lib/assert.js +++ b/lib/assert.js @@ -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 @@ -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; diff --git a/test/parallel/test-assert.js b/test/parallel/test-assert.js index a97809b88bf3db..3d35376864e323 100644 --- a/test/parallel/test-assert.js +++ b/test/parallel/test-assert.js @@ -58,7 +58,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)), @@ -408,6 +408,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;