assert.isNumber(NaN) passes #271

Closed
nrabinowitz opened this Issue Jun 12, 2014 · 4 comments

Comments

Projects
None yet
2 participants

The title says it all. I can see that, according to the spec, this may be considered correct behavior, but it runs pretty contrary to developer expectations.

At very least, it would be great to see a note about this in the docs.

@ghost

ghost commented Jul 15, 2014

You can use expect(NaN).to.be.ok.

Owner

keithamus commented Oct 13, 2014

I'd have to disagree with your comment that it runs contrary to developer expectations. Assert.isNumber simply checks the type using _.type which for Numbers, effectively does typeof value === 'number'.

As @robert-spacejam suggested, you can use assert.ok(NaN) or assert.notEqual(NaN, NaN) or assert.isFalse(Number.isNaN(NaN)). Perhaps we could add an isFinite or isNotNaN - but isNumber certainly should not change.

If you'd like to, I'd encourage you to make a Pull Request adding isFinite or isNotNaN methods.

@keithamus keithamus closed this Oct 13, 2014

I take your point, but it certainly ran contrary to my assumptions as a developer, and to the intent of my assertions. I'm going to guess that in a significant portion of the use cases for this assertion, NaN would be considered an error by the developer, so an additional assertion would be required.

I will try to get a PR in for more detailed assertions. But I think the main issue here is a note about this in the documentation, as I do think it's a surprising case.

Owner

keithamus commented Oct 13, 2014

@nrabinowitz I agree that the documentation could better reflect this. Feel free to send a PR for that too! You can just edit the comment above isNumber and the site will get updated with the next release.

TheSavior added a commit to TheSavior/chai that referenced this issue Sep 1, 2016

keithamus added a commit that referenced this issue Sep 2, 2016

lucasfcosta added a commit to lucasfcosta/chai that referenced this issue Sep 15, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment