Potential changes in assert.equals (needs feedback)

[Christian Johansen]

I am splitting out modules, renaming stuff, preparing for AMD in all of Buster's browser modules and generally preparing things for 1.0 according to our roadmap.

One of the things I recently did was to extract samsam, basically the interesting comparison methods from buster-assertions. It was just brought to my attention that Underscore.js actually implements about half of samsam, so I'm thinking of using it (actully lodash, which also has AMD support).

However, Underscore's isEqual has some differences, which would affect assert.equals:

• assert.equals(2, "2"); Passes today. but would fail with underscore/lodash
• assert.equals(0, ""); Passes today (falsy values), but would fail with underscore/lodash
•         var re1 = new RegExp("[a-z]+");
          var re2 = new RegExp("[a-z]+");
          re2.id = 42;
          assert.equals(re1, re2);
  This fails today, but would pass with underscore/lodash (it doesn't recognize the custom property). I may get a patch for that accepted.
• assert.equals with one arguments object and an array containing the same items passes today, but would fail with underscore/lodash. Same for array-like objects.

My question to you is: Does any of these changes matter? The more I think about it, the more I think "no". Especially because assert.match can be used in its place to get the above behavior (except for the regular expression case).

Christian

[Tiago Rodrigues]

I've always found it a bit confusing that assert.equals actually works the way it does currently and for me this would be a logical change.

But would this mean that assert.equals would behave as assert.same?

The reason why I ask this, is because I think the current behaviour of assert.equals is useful for comparing objects such as the ones on the assert.same documentation example of something that assert.equals should be used for:




{ name: "Chris", id: 42 } and { id: 42, name: "Chris" }

Would assert.equals still be able to do this?

[Christian Johansen]

I've always found it a bit confusing that assert.equals actually works the way it does currently and for me this would be a logical change.

I'm finding less and less positive things to say about coercion in general, so I'd like these changes too.
 

But would this mean that assert.equals would behave as assert.same?

No, assert.equals is a property-by-property deepEquals comparison.
 

The reason why I ask this, is because I think the current behaviour of assert.equals is useful for comparing objects such as the ones on the assert.same documentation example of something that assert.equals should be used for:




{ name: "Chris", id: 42 } and { id: 42, name: "Chris" }

Would assert.equals still be able to do this?

Most definitely!

Christian

[Magnar Sveen]

I prefer the new way in all cases, except for the last: I'd want to check array-likes without coercing them myself. And since assert.match now has set-semantics, how would you go about that in this scenario?

- Magnar

[Rod Vagg]

I'm cool with the changes. I'm always a fan of explicitness in tests (gives you permission to be clever in your tested code and use whatever magic you like), while coercion has its advantages it's certainly not explicit. And, I don't mind having to manually coerce arguments for the same reason, slice() isn't too hard and it's such a common pattern that you know what's going on when you see it.

[Christian Johansen]

I've played with it and come to the conclusion that I won't use Underscore/Lodash for the equals check. The reason is that `isEquals` is doing some underscore-specific checking of "chained" and "wrapped" objects, as well as has a hook for custom `isEquals`, which is cleverness I don't want in assert.equals.

In any case, I will be using lodash for quite a few other things, and I will change assert.equals to not do coercion any way, but I will keep the arguments/array "coercion".

Christian

--
MVH
Christian

[Christian Johansen]

Quick update. buster-assertions has successfully been transformed to Referee: https://github.com/busterjs/referee

A version is available in NPM, but it is not yet integrated with Buster. This version contains the coercion changes discussed in this thread. Referee is also AMD-compatible.

Christian