chai-stats icon indicating copy to clipboard operation
chai-stats copied to clipboard

Non numbers shouldn't be used for the almost-equal check

Open TN1ck opened this issue 9 years ago • 4 comments

Writing a assertion like the following, will fail, because the library expects only numbers. I expected it only looks at the numbers, not at strings or other objects.

expect({key: 0.8, bar: 'foo'}).to.almost.eql({key: 0.8, bar: 'foo'});

TN1ck avatar Apr 05 '16 13:04 TN1ck

Hey @TN1ck thanks for the issue.

I'm not sure we should change this behaviour. I think it'd be better for you to change your assertion to only assert on the numeric keys. Thoughts?

keithamus avatar Apr 05 '16 13:04 keithamus

Yes, it's not clear what the correct way should be. But it feels strange when equal values aren't considered as equal, a error-message with value {x} is not a number and is always considered unequal, only use this library to compare collections of numbers would help a lot, I think it's a common pattern to have some strings inside a numeric data-collection. For my usage it was really simple to provide a collection with the non-numbers omitted, so it was a non-issue for me after i knew what the problem was. And thanks for the lib, it's really useful!

TN1ck avatar Apr 05 '16 15:04 TN1ck

That error message sounds like it could indeed be useful. Do you fancy making a pull request to this effect?

keithamus avatar Apr 05 '16 15:04 keithamus

I had this issue but I wasn't able to find a good way to pass in only numbers/objects. I ended up patching the deepAlmostEqual function to cover the non-number and non-object cases because I really needed it to work with strings. I've created a branch called all-type-comparison with some tests for this.

exports.deepAlmostEqual = function(a,b,precision){
  var tol = tolerance(precision);
  function deepEql (act, exp) {
    if (Object(act) === act){
      for (var k in act) {
        if (typeof act[k] === typeof exp[k] && (typeof act[k] === 'number'
                                                || typeof act[k] === 'object')) {
          if (!(deepEql(act[k], exp[k]))) {
            return false;
          }
        } else {
          if (act[k] !== exp[k]) {
            return false;
          }
        }
      }
      return true;
    } else {
      return Math.abs(act - exp) < tol;
    }
  }

  return deepEql(a,b);
};

glen-nu avatar Feb 13 '17 07:02 glen-nu