jest icon indicating copy to clipboard operation
jest copied to clipboard

[Bug]: 0 is not considered equal to -0?

Open Pomax opened this issue 3 years ago • 19 comments
trafficstars

Version

27.4.7

Steps to reproduce

it(`these are the same. No really `, () => {
  expect(0 === -0).toBe(true); 
  expect(0).toBe(-0);
})

First test passes, because it should. Second test fails. It very much should not.

Expected behavior

By both maths and JS spec definition, 0 is equal to -0

Actual behavior

    Expected: -0
    Received: 0

       7 |   it(`these are the same. No really `, () => {
       8 |     expect(0 === -0).toBe(true);
    >  9 |     expect(0).toBe(-0);
         |               ^
      10 |   })

Additional context

Floating point numbers result in hilarious edge cases.

Environment

Win 10 Pro x64, Node v17.3.0, Jest v27.4.7

Pomax avatar Jan 05 '22 23:01 Pomax

This is by design, we use Object.is. That said, I might be convinced that toEqual should report them as the same, but we have an explicit test today that they're not: https://github.com/facebook/jest/blob/5cd75f4e0b9f8b678fedee268676864013b12d81/packages/expect/src/tests/matchers.test.js#L452

@thymikee @cpojer thoughts?

SimenB avatar Jan 06 '22 07:01 SimenB

Given that I'm running into this as part of a maths library tests, 0 and -0 kind of need to be the same thing. The direction by which we arrive at zero is not part of the numerical equality. It's why 0 === -0 is true as per the JS language spec (ref: https://262.ecma-international.org/6.0/#sec-strict-equality-comparison, 7.2.13 step 4, points d and e) even if in the IEEE 754 spec the values +0 and -0 are provisioned as different bit patterns.

Using Object.is in this case gives you the wrong result because while theyre different internal values to JS (because otherwise it wouldn't be able to show 0 vs. -0) the JS spec says they're the same value in actual code. As such, treating them as not equal is quite literally a spec violation.

Of course, if a toStrictEqual or the like goes "hey these are different IEEE values" that makes a lot of sense, and even a maths library might benefit from that (e.g. to determine limiting behaviour) but the plain equals/toBe should most definitely say "yes, these are the same value".

Pomax avatar Jan 06 '22 17:01 Pomax

What about a custom .toBeMathematicallyEqual matcher? If .toBe matcher does not suite your project, why not to create a custom one?

Just an idea. I am not tying to argue (;

mrazauskas avatar Jan 06 '22 18:01 mrazauskas

@Pomax What you're saying does seem to make sense.. Are you implying then that the implementation of Object.is is incorrect? Or just saying that Object.is is not the correct function to use for the .toBe matcher?

danny-does-stuff avatar Jan 06 '22 20:01 danny-does-stuff

Good clarification question: using Object.is is in this case incorrect. For primitives, JS's strict equality (===) should be the authority on what is, and is not, "the same thing". (Except for values that the spec clearly indicates as never yielding true under === comparison, like NaN, Infinity, etc). Object.is is a low level mechanism for checking whether things are effectively pointers to the same data in memory (in this case, the standard library value table), which is too low level for testing user code (rather than testing whether an implementation of the JS interpreter itself is doing the right thing).

I haven't dug into the code to see how many things rely on Object.is but I would say that toBe should be based on ===, since it's for use with primitives, with equals based on arbitrarily deep object comparison. The Object.is comparison should only be used as part of the code for toStrictEqual.

Pomax avatar Jan 06 '22 21:01 Pomax

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 7 days.

github-actions[bot] avatar Feb 25 '22 12:02 github-actions[bot]

This issue should probably not go stale.

Pomax avatar Feb 25 '22 18:02 Pomax

I have no problem keeping the Object.is behavior with its 0 vs -0 inequality for toBe. Since 5/-0 is -Infinity while 5/0 is Infinity, the difference has implications that should be respected.

That being said, I think there very much needs to be some option to use === in toEqual, whether an additional parameter or an additional version of the matcher. Right now I'm testing a function that creates an array of numbers in which 0 and -0 need to be treated as equal, and the function could produce either depending on circumstances.

It's really pretty awkward to have to figure out case by case whether I need to expect, for example, .toEqual([-3, -2, -1, 0, 1, 2]) or .toEqual([-3, -2, -1, -0, 1, 2]) when the two are functionally equivalent for my use. And a subtle change to the function or inputs could start breaking tests that would be perfectly robust if I could just allow 0 and -0 to be compared with ===.

harshbarger avatar Mar 15 '22 21:03 harshbarger

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 30 days.

github-actions[bot] avatar Apr 14 '22 21:04 github-actions[bot]

obligatory "it would be nice to solve this" comment to prevent the stalebot from sniping this issue.

Pomax avatar Apr 14 '22 23:04 Pomax

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 30 days.

github-actions[bot] avatar May 14 '22 23:05 github-actions[bot]

And again.

Pomax avatar May 22 '22 15:05 Pomax

Any updates on this?

danielfarah54 avatar Jul 08 '22 20:07 danielfarah54

Have you tried expect(0).toBeCloseTo(-0);? It may work.

brunolnetto avatar Jan 24 '23 14:01 brunolnetto

It may, but "very close to zero" and "zero" are wildly different things.

Also as a remark on an earlier comment that asserted that 5/0 yields Infinity and 5/-0 yields -Infinity: while true, the language itself specifies that 0 is strictly equal to -0 and running 0 === -0 (not just 0 == -0) must evaluate to true, or you're not implementing JS (or JS testing).

So: if you need to know whether the effect of a statement involving a possibly negative zero is some specific value, you run the statement and test the result. That's not an argument for what tests on zero itself do.

In this case using Object.is does something so close to what it's being used for that it feels like it does the thing it's being used for, but it's not. It checks whether two things point to the same items on the heap. The problem with that is that Number is special, and there are different bit patterns, with different heap values, that by spec-definition must be considered the same value in isolation. Zero and negative zero are the same value, even if statements involving them yield different results depending on that sign.

Pomax avatar Jan 24 '23 16:01 Pomax

A solution may be:

const isZero = (result) => {
    if(typeof result !== 'number') {
        throw NaNError('Provided argument is not a number. A possible zero result is a number!);
} else return Number(result) === 0;

}

Jest offers matcher extension. It seems very incircunstancial, although expected to twist nose in such situation. Why is not expect(expectation === result).toBe(true) a solution? :-/

Does it make sense?

brunolnetto avatar Jan 25 '23 10:01 brunolnetto

Is it possible to make it optional toggle and allowed user to set it? Something could be like JEST_NEGATIVE_ZERO_COMPARISON_ENABLED=1 (or disable-ish similar).

t1anchen avatar Jan 23 '24 04:01 t1anchen

+1 especially in the case of expect(xs).toEqual(ys), where xs and ys are arrays or other deep structures, so the structural matching behavior of toEqual is still necessary but we still want standard equality semantics on -0. We're currently working around this with expect(xs.every((_, i) => xs[i] === ys[i])).toBe(true), but that has worse error messages and does not scale easily to more complicated structures. I agree that it makes sense for toBe to distinguish between the two, but it's surprising and unhelpful for toEqual to do so.

wchargin avatar Jul 10 '24 19:07 wchargin