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

Support differentiating -0 and +0 (sameValue assertion) #1010

Open
leobalter opened this issue Jun 23, 2016 · 5 comments
Open

Support differentiating -0 and +0 (sameValue assertion) #1010

leobalter opened this issue Jun 23, 2016 · 5 comments
Labels
Category: API Component: Assert Type: Enhancement New idea or feature request. Type: Meta Seek input from maintainers and contributors.

Comments

@leobalter
Copy link
Member

ECMAScript has two internal abstractions to compare values called sameValue and sameValueZero.

They are different than == and === as they return true for NaN values and compare -0 and +0.

Based on my experience with test262, and considering some personal goals I have for QUnit (still seeking funding for it) I believe it's fair to ship assertions that does the same comparisons, while I believe it's dangerous to simply modify strictEqual.

Object.is returns the exact result for sameValue, but that's an ES6 feature and not available on supported browsers. The results are the same as === but with special checks for NaN and bit precision zeros.

Here goes some data on how they would work:

method NaN, NaN 0, -0 -0, -0 [1, 2], [1, 2]
equal fail pass pass fail
strictEqual fail pass pass fail
deepEqual pass pass pass pass
propEqual pass pass pass pass
_.isEqual pass pass pass pass
sameValue pass fail pass fail
sameValueZero pass pass pass fail

The following code can provide some results, as it uses lodash's isEqual as well:
cc @jdalton

QUnit.assert.sameValue = function(a, b, message) {
  var result;

  if (a === b) {
    // -0 vs +0
    result = (a !== 0 || 1/a === 1/b);
  } else

  // NaN
  if (a !== a && b !== b) {
    result = true;
  }

  this.pushResult({
    result,
    actual: a,
    expected: b,
    message
  });
};

QUnit.assert.sameValueZero = function(a, b, message) {
  var result;

  if (a === b) {
    result = true;
  } else 

  // NaN
  if (a !== a && b !== b) {
    result = true;
  }

  this.pushResult({
    result,
    actual: a,
    expected: b,
    message
  });
};


var {test} = QUnit;

test("NaN", function(t) {
  t.ok(_.isEqual(NaN, NaN), "lodash equals");
  t.equal(NaN, NaN, "equal");
  t.strictEqual(NaN, NaN, "strictEqual");
  t.deepEqual(NaN, NaN, "deepEqual");
  t.propEqual(NaN, NaN, "propEqual");
  t.sameValue(NaN, NaN, "sameValue");
  t.sameValueZero(NaN, NaN, "sameValueZero");
});

test("-0 vs +0", function(t) {
  t.ok(_.isEqual(0, -0), "lodash equals");
  t.equal(0, -0);
  t.strictEqual(0, -0);
  t.deepEqual(0, -0);
  t.propEqual(0, -0);
  t.sameValue(0, -0, "sameValue");
  t.sameValueZero(0, -0);
});

test("-0", function(t) {
  t.ok(_.isEqual(0, -0), "lodash equals");
  t.equal(-0, -0);
  t.strictEqual(-0, -0);
  t.propEqual(-0, -0);
  t.deepEqual(-0, -0);
  t.sameValue(-0, -0);
  t.sameValueZero(-0, -0);
});

test("arrays", function(t) {
  t.ok(_.isEqual([1, 2], [1, 2]), "lodash equals");
  t.equal([1, 2], [1, 2], "equal");
  t.strictEqual([1, 2], [1, 2], "strictEqual");
  t.propEqual([1, 2], [1, 2], "propEqual");
  t.deepEqual([1, 2], [1, 2], "deepEqual");
  t.sameValue([1, 2], [1, 2], "sameValue");
  t.sameValueZero([1, 2], [1, 2], "sameValueZero");
});
@jdalton
Copy link

jdalton commented Jun 23, 2016

🤘

@trentmwillis
Copy link
Member

@leobalter would the eventual goal here be to replace one or both of our current equality checks with these assertions? I'm concerned about overwhelming users with options that are very similar.

@leobalter
Copy link
Member Author

I want to approximate the API to the JS specs and test262. Maybe the
sameValueZero (ignores Diff from -0 and 0) is not necessary, but I would
like to add sameValue anyway.

I also have to admit I see them as good candidates for lodash as well.

One good argument for them is the lack of support for precisely comparing
NaN and bit precision zeros on current QUnit's api as actual and expected
values.

To check NaN values, the global isNaN is unreliable and Number.isNaN is
ES6+ and they mine the possibility of diffing results for failures.

On Tuesday, June 28, 2016, Trent Willis [email protected] wrote:

@leobalter https://github.com/leobalter would the eventual goal here be
to replace one or both of our current equality checks with these
assertions? I'm concerned about overwhelming users with options that are
very similar.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1010 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AASYkfCZ6JldjjaTYv4u1IqmqrdeYTgHks5qQVlygaJpZM4I9B0H
.

@trentmwillis
Copy link
Member

Okay, makes sense. I understood the logic for having them, but I wanted to clarify the long term goals

@Krinkle Krinkle added Type: Meta Seek input from maintainers and contributors. Type: Enhancement New idea or feature request. labels Jun 25, 2020
@Krinkle
Copy link
Member

Krinkle commented Jun 25, 2020

I think having both strictEqual and sameValue would be confusing long-term. I'm on the fence whether sameValue would be a good replacement for strictEqual long-term. It's worth considering for sure. I'm curious whether there are use cases for having code complex enough to produce negative 0 and yet not interested in asserting it.

For NaN, I can see an edge case where the current === behaviour might help catch a scenario where actual and expected both had something go wrong in one if its numerical properties and thus produced NaN. Tolerating this, as sameValue would, might mask such issue. On the other hand one could argue that's expected, especially since a test runner is not meant to look for problems in the expected value. Plus, if you use strictEqual to assert the result of a single numerical value, it should be rare for expected to not be a literal, and it should certainly be possible to assert it as NaN without needing a plugin or custom inline expressions.

Worth noting here is that assert.deepEqual already supports comparing of NaN:

QUnit.test('example', assert => {
 assert.strictEqual(NaN, NaN, 'fails');
 assert.deepEqual(NaN, NaN, 'passes');
 assert.deepEqual({ a: NaN }, { a: NaN }, 'passes');
});

And Node.js' assert.deepStrictEqual (docs) also uses the sameValue algorithm, and thus also supports matching NaN and differentiating 0 from -0.

I think that strikes a fairly good balance. For inline expressions of primitive values, we'd stick with strict equality for better parity and intuition of how JavaScript generally works. For recursive comparing of more complex structures, we'd use the sameValue logic for each individual value, thus supporting the matching NaN (as we do today) and differentiating of -0 from 0 (which we'd start doing). Noting that to get this behaviour for primitive/single values, the deepEqual method can be used directly. This can be surprising at first, but I think good documentation suffices for that. I'd prefer that over having more (similar) methods.

Would that leave any use cases uncovered? We'd land this in 3.0 to avoid breaking tests in the event 0/-1 equality is relied upon in deepEqual calls today.

@Krinkle Krinkle added this to the 3.0 milestone Jun 25, 2020
@Krinkle Krinkle changed the title New assertions sameValue and sameValueZero Support differentiating -0 and +0 (sameValue assertion) Jun 25, 2020
@Krinkle Krinkle removed this from the 3.0 release milestone Nov 7, 2020
@Krinkle Krinkle mentioned this issue Nov 7, 2020
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: API Component: Assert Type: Enhancement New idea or feature request. Type: Meta Seek input from maintainers and contributors.
Development

No branches or pull requests

4 participants