Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

assert: handle (deep) equal(NaN, NaN) as being identical #30766

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 26 additions & 3 deletions doc/api/assert.md
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,10 @@ An alias of [`assert.ok()`][].
<!-- YAML
added: v0.1.21
changes:
- version: REPLACEME
pr-url: https:/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:/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:/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:/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:/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:/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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,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
Original file line number Diff line number Diff line change
Expand Up @@ -595,10 +595,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
Original file line number Diff line number Diff line change
Expand Up @@ -501,6 +501,12 @@ assert.throws(
}
);

a.equal(NaN, NaN);
a.throws(
() => a.notEqual(NaN, NaN),
a.AssertionError
);
BridgeAR marked this conversation as resolved.
Show resolved Hide resolved

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