Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit a488892

Browse filesBrowse files
TrottMyles Borins
authored andcommitted
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: #7413 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 6c94c67 commit a488892
Copy full SHA for a488892

File tree

Expand file treeCollapse file tree

2 files changed

+15
-7
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

2 files changed

+15
-7
lines changed
Open diff view settings
Collapse file

‎lib/assert.js‎

Copy file name to clipboardExpand all lines: lib/assert.js
-6Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828
const compare = process.binding('buffer').compare;
2929
const util = require('util');
3030
const Buffer = require('buffer').Buffer;
31-
const pSlice = Array.prototype.slice;
3231
const pToString = (obj) => Object.prototype.toString.call(obj);
3332

3433
// 1. The assert module provides functions that throw
@@ -223,11 +222,6 @@ function objEquiv(a, b, strict, actualVisitedObjects) {
223222
const bIsArgs = isArguments(b);
224223
if ((aIsArgs && !bIsArgs) || (!aIsArgs && bIsArgs))
225224
return false;
226-
if (aIsArgs) {
227-
a = pSlice.call(a);
228-
b = pSlice.call(b);
229-
return _deepEqual(a, b, strict);
230-
}
231225
const ka = Object.keys(a);
232226
const kb = Object.keys(b);
233227
var key, i;
Collapse file

‎test/parallel/test-assert.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-assert.js
+15-1Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ assert.throws(makeBlock(a.strictEqual, null, undefined),
5858
assert.doesNotThrow(makeBlock(a.notStrictEqual, 2, '2'),
5959
'notStrictEqual(2, \'2\')');
6060

61-
// deepEquals joy!
61+
// deepEqual joy!
6262
// 7.2
6363
assert.doesNotThrow(makeBlock(a.deepEqual, new Date(2000, 3, 14),
6464
new Date(2000, 3, 14)),
@@ -408,6 +408,20 @@ var args = (function() { return arguments; })();
408408
a.throws(makeBlock(a.deepEqual, [], args));
409409
a.throws(makeBlock(a.deepEqual, args, []));
410410

411+
// more checking that arguments objects are handled correctly
412+
{
413+
const returnArguments = function() { return arguments; };
414+
415+
const someArgs = returnArguments('a');
416+
const sameArgs = returnArguments('a');
417+
const diffArgs = returnArguments('b');
418+
419+
a.throws(makeBlock(a.deepEqual, someArgs, ['a']));
420+
a.throws(makeBlock(a.deepEqual, ['a'], someArgs));
421+
a.throws(makeBlock(a.deepEqual, someArgs, {'0': 'a'}));
422+
a.throws(makeBlock(a.deepEqual, someArgs, diffArgs));
423+
a.doesNotThrow(makeBlock(a.deepEqual, someArgs, sameArgs));
424+
}
411425

412426
var circular = {y: 1};
413427
circular.x = circular;

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.