Immutable actuals

[matthewwithanm]

We're getting some false positives in our equals comparisons!

Turns out it's because Immutable.is is only used when the "actual" value is an Immutable collection. This fixes it.

[matthewwithanm]

Oops. The problem is actually when only the expected value is an Immutable collection. Updated my commit messages and test descriptions.

[astorije]

Hi @matthewwithanm, thanks for the contribution (and sorry for the delay, I was finally taking some vacations :-) ).
I am wondering, did you push the updates you are mentioning in your comment?

[matthewwithanm]

Np. Yeah, I amended the commit.

[astorije]

Ah right, thanks @matthewwithanm I am reviewing right now.

[astorije]

@matthewwithanm, I'm not sure I see why we need this.

Currently:

expect([]).to.not.equal(List.of(1, 2, 3)); // Passes
expect([]).to.equal(List.of(1, 2, 3)); // Fails with "AssertionError: expected [] to equal List [ 1, 2, 3 ]"

which seems like what we should expect: if obj is not a Collection, the equality assertion is deferred to the original assertion.

Is there something I am I missing here?

[matthewwithanm]

Hm, yeah it doesn't seem like we do…sorry, I can't remember anymore! I can remove it and see if we stumble on why again. Sound good?

[matthewwithanm]

Going to push a new PR…I think the real issue is that assert.equal is calling itself instead of the original version. Currently trying to figure out why we need to do that at all (it seems like Assertion should take care of that when it calls _super).