Make t.is() use Object.is()
#1353
Conversation
… so that `NaN` compares favourably to another `NaN`. - Update documentation to explain this. - Update tests to explicitly test for this (including adding a few extra passing tests for `t.is()`).
…matically, x = y, rather than using `===` or `Object.is`
test/assert.js
Outdated
| message: 'my message', | ||
| values: [ | ||
| {label: 'Actual:', formatted: /0/}, | ||
| {label: 'Must be strictly equal (using Object.is) to:', formatted: /0/} |
There was a problem hiding this comment.
I suspect this outputs 0, rather than -0? #1341 would fix, but I think we should work around this in this branch too.
lib/assert.js
Outdated
| pass(this); | ||
| } else { | ||
| const diff = formatAssertError.formatDiff(actual, expected); | ||
| const values = diff ? [diff] : [ | ||
| formatAssertError.formatWithLabel('Actual:', actual), | ||
| formatAssertError.formatWithLabel('Must be strictly equal to:', expected) | ||
| formatAssertError.formatWithLabel('Must be strictly equal (using Object.is) to:', expected) |
There was a problem hiding this comment.
Perhaps we can take a leaf from the Object.is() description and say Must be the same value as:.
readme.md
Outdated
| @@ -914,23 +914,23 @@ Assert that `value` is `false`. | |||
|
|
|||
| ### `.is(value, expected, [message])` | |||
|
|
|||
| Assert that `value` is equal to `expected`. | |||
| Assert that `value` is equal to `expected` (using `Object.is()` for equality testing). | |||
There was a problem hiding this comment.
We could say:
Assert that
valueis the same asexpected. This is based onObject.is().
|
@novemberborn the latest commit addresses all three of these. I have gone for "the same as" rather than "the same value as" (you used both in your two comments on this) so let me know if you'd prefer "the same value as". Also, the I think when I originally did this work on the tests I was really unclear as to how the AVA test helpers worked with all the formatted values and regexes, etc., so I went for a conservative |
There was a problem hiding this comment.
I have gone for "the same as" rather than "the same value as" (you used both in your two comments on this) so let me know if you'd prefer "the same value as".
"the same as" is great.
Also, the 0/-0 thing was my bad - Just adding the - into the test expectation worked, so it appears that the value formatter does indeed include the negative sign. Phew!
Nice.
Just one documentation nit, and I'd like to hear from @sindresorhus or @vadimdemedes on this before merging.
readme.md
Outdated
| @@ -914,11 +914,11 @@ Assert that `value` is `false`. | |||
|
|
|||
| ### `.is(value, expected, [message])` | |||
|
|
|||
| Assert that `value` is equal to `expected` (using `Object.is()` for equality testing). | |||
| Assert that `value` is the same as `expected`. This is based on using [`Object.is()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/is). | |||
There was a problem hiding this comment.
The "using" seems superfluous. Either This is based on or This relies on I think. Also for .not().
There was a problem hiding this comment.
My bad. Updated.
|
I'm a little bit sceptical of differentiating between |
sindresorhus
changed the title
t.is() use Object.is()
|
Thank you @alexrussell :) |
|
Woohoo thanks for merging 🎉 |
Refactor
isassertion to useObject.is()rather than===.Also add a few more test cases in for
isto make it clearer what it is expected to do.As per discussion in #1339, this will make it more sensible to compare
NaNagainstNaN.The good:
NaNnow compares toNaNin a way that people expect (truthily)The odd:
+0/0and-0are now not equal. As we discussed in Feature request: UseObject.is()instead of===int.is()#1339, this actually feels fine - this being a testing framework, it's good to be strict on this math stuff, while sensible on the NaN stuff.Fixes #1339
/cc @novemberborn, @sotojuan, @sindresorhus