Skip to content

Commit

Permalink
assert: handle (deep) equal(NaN, NaN) as being identical
Browse files Browse the repository at this point in the history
This aligns the `equal` and `deepEqual()` implementations with the
strict versions by accepting `NaN` as being identical in case both
sides are NaN.

Refs: #30350 (comment)

PR-URL: #30766
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
  • Loading branch information
BridgeAR committed Dec 6, 2019
1 parent b5f2942 commit 5360dd1
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 10 deletions.
29 changes: 26 additions & 3 deletions doc/api/assert.md
Expand Up @@ -160,6 +160,10 @@ An alias of [`assert.ok()`][].
<!-- YAML
added: v0.1.21
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/30766
description: NaN is now treated as being identical in case both sides are
NaN.
- version: v12.0.0
pr-url: https://github.com/nodejs/node/pull/25008
description: The type tags are now properly compared and there are a couple
Expand Down Expand Up @@ -203,7 +207,8 @@ are also recursively evaluated by the following rules.
### Comparison details

* Primitive values are compared with the [Abstract Equality Comparison][]
( `==` ).
( `==` ) with the exception of `NaN`. It is treated as being identical in case
both sides are `NaN`.
* [Type tags][Object.prototype.toString()] of objects should be the same.
* Only [enumerable "own" properties][] are considered.
* [`Error`][] names and messages are always compared, even if these are not
Expand Down Expand Up @@ -554,6 +559,11 @@ assert.doesNotThrow(
## assert.equal(actual, expected\[, message\])
<!-- YAML
added: v0.1.21
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/30766
description: NaN is now treated as being identical in case both sides are
NaN.
-->

* `actual` {any}
Expand All @@ -569,7 +579,8 @@ An alias of [`assert.strictEqual()`][].
> Stability: 0 - Deprecated: Use [`assert.strictEqual()`][] instead.
Tests shallow, coercive equality between the `actual` and `expected` parameters
using the [Abstract Equality Comparison][] ( `==` ).
using the [Abstract Equality Comparison][] ( `==` ). `NaN` is special handled
and treated as being identical in case both sides are `NaN`.

```js
const assert = require('assert');
Expand All @@ -578,6 +589,8 @@ assert.equal(1, 1);
// OK, 1 == 1
assert.equal(1, '1');
// OK, 1 == '1'
assert.equal(NaN, NaN);
// OK

assert.equal(1, 2);
// AssertionError: 1 == 2
Expand Down Expand Up @@ -732,6 +745,10 @@ let err;
<!-- YAML
added: v0.1.21
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/30766
description: NaN is now treated as being identical in case both sides are
NaN.
- version: v9.0.0
pr-url: https://github.com/nodejs/node/pull/15001
description: The `Error` names and messages are now properly compared
Expand Down Expand Up @@ -853,6 +870,11 @@ instead of the [`AssertionError`][].
## assert.notEqual(actual, expected\[, message\])
<!-- YAML
added: v0.1.21
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/30766
description: NaN is now treated as being identical in case both sides are
NaN.
-->

* `actual` {any}
Expand All @@ -868,7 +890,8 @@ An alias of [`assert.notStrictEqual()`][].
> Stability: 0 - Deprecated: Use [`assert.notStrictEqual()`][] instead.
Tests shallow, coercive inequality with the [Abstract Equality Comparison][]
( `!=` ).
(`!=` ). `NaN` is special handled and treated as being identical in case both
sides are `NaN`.

```js
const assert = require('assert');
Expand Down
5 changes: 3 additions & 2 deletions lib/assert.js
Expand Up @@ -25,6 +25,7 @@ const {
ObjectIs,
ObjectKeys,
ObjectPrototypeIsPrototypeOf,
NumberIsNaN
} = primordials;

const { Buffer } = require('buffer');
Expand Down Expand Up @@ -398,7 +399,7 @@ assert.equal = function equal(actual, expected, message) {
throw new ERR_MISSING_ARGS('actual', 'expected');
}
// eslint-disable-next-line eqeqeq
if (actual != expected) {
if (actual != expected && (!NumberIsNaN(actual) || !NumberIsNaN(expected))) {
innerFail({
actual,
expected,
Expand All @@ -416,7 +417,7 @@ assert.notEqual = function notEqual(actual, expected, message) {
throw new ERR_MISSING_ARGS('actual', 'expected');
}
// eslint-disable-next-line eqeqeq
if (actual == expected) {
if (actual == expected || (NumberIsNaN(actual) && NumberIsNaN(expected))) {
innerFail({
actual,
expected,
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/util/comparisons.js
Expand Up @@ -180,7 +180,7 @@ function innerDeepEqual(val1, val2, strict, memos) {
if (val1 === null || typeof val1 !== 'object') {
if (val2 === null || typeof val2 !== 'object') {
// eslint-disable-next-line eqeqeq
return val1 == val2;
return val1 == val2 || (NumberIsNaN(val1) && NumberIsNaN(val2));
}
return false;
}
Expand Down
7 changes: 3 additions & 4 deletions test/parallel/test-assert-deep.js
Expand Up @@ -590,10 +590,9 @@ assertNotDeepOrStrict(
}

// Handle NaN
assert.notDeepEqual(NaN, NaN);
assert.deepStrictEqual(NaN, NaN);
assert.deepStrictEqual({ a: NaN }, { a: NaN });
assert.deepStrictEqual([ 1, 2, NaN, 4 ], [ 1, 2, NaN, 4 ]);
assertDeepAndStrictEqual(NaN, NaN);
assertDeepAndStrictEqual({ a: NaN }, { a: NaN });
assertDeepAndStrictEqual([ 1, 2, NaN, 4 ], [ 1, 2, NaN, 4 ]);

// Handle boxed primitives
{
Expand Down
6 changes: 6 additions & 0 deletions test/parallel/test-assert.js
Expand Up @@ -501,6 +501,12 @@ assert.throws(
}
);

a.equal(NaN, NaN);
a.throws(
() => a.notEqual(NaN, NaN),
a.AssertionError
);

// Test strict assert.
{
const a = require('assert');
Expand Down

0 comments on commit 5360dd1

Please sign in to comment.