Sinon: Sinon calledWith fails comparing ImmutableJS arguments that it deems equal

[itsmichaelwang]

Describe the bug
Sinon maintains that an ImmutableJS list generated two different ways are equal, but then when comparing arguments in a spy it maintains that they are not equal.

it("fails", () => {
    const obj = { foo: "bar" };
    const immutable1 = Immutable.List().push(Immutable.fromJS(obj));
    const immutable2 = Immutable.fromJS([obj]);

    // this passes
    expect(immutable1).to.eq(immutable2);

    const baz = sinon.stub();
    baz(immutable1);

    // this fails, but weren't they equal?
    sinon.assert.calledWith(baz, immutable2);
});

Maps and fromJS do not suffer from this issue.

it("passes", () => {
    const obj = { foo: "bar" };
    const immutable1 = Immutable.Map(obj);
    const immutable2 = Immutable.fromJS(obj);

    // this passes
    expect(immutable1).to.eq(immutable2);

    const baz = sinon.stub();
    baz(immutable1);

    // this passes
    sinon.assert.calledWith(baz, immutable2);
});

Additionally, we get a really confusing stack trace:

AssertError: expected stub to be called with arguments 
List [ Map { "foo": "bar" } ]
/*
 * if you console log immutable1 or immutable2, you will get exactly this^ string, character for character
 */

Because both variables are serialized in the same way, it adds an additional layer of confusion!

To Reproduce
Steps to reproduce the behavior:

1. Install sinon and ImmutableJS
2. Copy/paste the above code and see that it fails

Expected behavior
It seems like if sinon agrees that the two objects are equal, that their argument assertions should also be equal.

Context (please complete the following information):

• Library version: Sinon 7.3.1 (does minor version count as latest?)
• Environment: OS X Mojave (10.14.5)
• Other libraries you are using: ImmutableJS (3.8.2)

[fatso83]

Good bug report. I assume finding out what the root cause is would require digging into the details of assertion and what constitutes equality for different types of objects.

With regards to the stack trace I don't think we can do much about that. That probably reliance on some basic tostring representation of each object. We don't make special customizations for specific libraries. That would be better suited for a plug-in or wrapper of some kind that specifically targets that library.

[mroderick]

What is the expectation library that you're using?

I suspect that the root cause of the problem comparing the values are in https://github.com/sinonjs/samsam/blob/master/lib/deep-equal.js somewhere

[fatso83]

Does the explanation at https://github.com/sinonjs/samsam/blob/master/lib/deep-equal.js#L31 make sense to you, Michael?

[itsmichaelwang]

@mroderick The assertion library I was using above was sinon assertions, but I also got it to fail using [email protected]:

describe('ImmutableJS fuckery #1', () => {
      it('fails', () => {
        const obj = { foo: 'bar' };
        const immutable1 = Immutable.List().push(Immutable.fromJS(obj));
        const immutable2 = Immutable.fromJS([obj]);

        // this passes
        expect(immutable1).to.eq(immutable2);

        const baz = sinon.stub();
        baz(immutable1);

        // this fails, but weren't they equal?
        sinon.assert.calledWith(baz, immutable2);

        // this also fails
        expect(baz).to.have.been.calledWithExactly(immutable2);
      });

      it('passes', () => {
        const obj = { foo: 'bar' };
        const immutable1 = Immutable.Map(obj);
        const immutable2 = Immutable.fromJS(obj);

        // this passes
        expect(immutable1).to.eq(immutable2);

        const baz = sinon.stub();
        baz(immutable1);

        // this passes
        sinon.assert.calledWith(baz, immutable2);

        // this also passes
        expect(baz).to.have.been.calledWithExactly(immutable2);
      });

[itsmichaelwang]

I can take a look at this after work. Does .eq and the matcher for calledWith not use the same equality comparator? I also tried doing .eql and it also says that they are equivalent, so I'm not sure if that means there's a third logic branch that's being used for argument comparison? I'll have to take a peek later.

[fatso83]

We meant what assertion library were you using (not the sinon assert) to write .eq(obj2), but since you mentioned sinon-chai, I deduce that you use chai :-)

The reason we have to ask is that we have no way of knowing how the eq method works in some external library works without checking its docs, and you certainly cannot assume it's equal to the one in Sinon's assertion. You have strict equality, as well as many variations of deep equality, so that's why we needed to know that you were using Chai.

I can understand why you don't understand why we keep mentioning samsam, but it's what Sinon uses under the hood for its equality checks! So that's why I asked if the JSDoc for the deep equals gave some insight into what was happening.

[fatso83]

I just tried verifiying your example, and it seems your description is incorrect, as it doesn't pass where your comments says it does. See https://runkit.com/fatso83/sinon-issue-2077

This fails:

// this passes
expect(immutable1).to.eq(immutable2);

which means it never gets to the sinon assertion.

The diff when run on the console is:

      -    "ownerID": {}
      +    "ownerID": [undefined]

So basically, because immutable1._tail.ownerID !== immutable2._tail.ownerID, they are not deeply equal. Not a problem with Sinon, I'm afraid.

[scraggo]

I ran into a similar problem and wrote a guide on using custom matchers with sinon to get around it https://www.scraggo.com/testing-immutable-js-with-sinon-custom-matchers/

[mroderick]

@scraggo @fatso83 I've added #2223 to link to that article from the external howtos section.