Deep partial object match with samsam?

[thom-nic]

First, apologies for the samsam question here. The samsam repo doesn't have discussions enabled.

I'm trying to update chai-samsam which uses a very old version of samsam. I can't get @sinonjs/[email protected] (or v7,6,5) to do deep partial object matching.

// returns true as expected
match(
  {
    name: "Chris",
    profession: {job: "Programmer"},
  },
  {
    profession: {job :"Programmer"},
  }
);

// returns false because the value of 'profession' is not identical :(
match(
  {
    name: "Chris",
    profession: {job: "Programmer", foo:'bar'},
  },
  {
    profession: {job :"Programmer"},
  }
);

Documentation for "Object matcher" states

If the matcher does not have a test method, a recursive match is performed. If all properties of matcher matches corresponding properties in object, match returns true. Note that the object matcher does not care if the number of properties in the two objects are the same - only if all properties in the matcher recursively matches ones in object.

However object property values don't seem "recursively" match - they need to be identical.

Arrays "recursively" work as one would expect - so long as you don't recurse into an object apparently:

 // returns true as expected
match([1,2,3,4], [1,2,3])

// returns false :(
match({ count: [1,2,3,4] }, { count: [1,2,3] })

// but nested partial array matching works!
match([1,2,3,[4,5]], [1,2,3,[4]]);

// and nested partial objects inside an array work!
match([[1,2,3,{a:1,b:2}]], [[1,2,3]]);
match([[1,2,3,{a:1,b:2}]], [[1,2,3,{a:1}]]);

// but not partial arrays when they are an object property :((((
match([[1,2,3,{a:1,b:[2,3]}]], [[1,2,3,{a:1,b:[2]}]]);

So it seems this behavior has been around for a while, and probably why chai-samsam has never been updated to a newer version of samsam. That being the case: is there any way to get samsam to do actual recursive object matching?

[fatso83]

Looking at the commit history, this one might be in the domain of @mroderick, so maybe he knows if there has been a regression at some point. I am not all that familiar, but I do get why you wonder :)

[thom-nic]

@mroderick could you weigh in on this? I'm keen to use chai-samsam for mocha testing, but it uses a really old version of samsam, I think because of this discrepancy in behavior. TIA 🙏

[fatso83]

Ping? This should not be such a hard thing to fix (famous last words), especially if it worked earlier.

[thom-nic]

Hey! Very sorry I appreciate the follow up. chai-samsam uses samsam v1.3.0 which is before it was moved to the @sinonjs/samsam namespace and bumped to v2.0.0. I haven't done a full bisect but it's possible the behavior could have changed as early as the first release under @sinonjs/samsam.

I appreciate your willingness to look into this, I will try to do some more version control forensics and find when the behavior changed and get back to you!

[thom-nic]

Ok the breaking change is between @sinonjs/samsam v2.1.3 and v3.0.0

It looks like 3.0.0 deliberately introduced several breaking changes w/r/t match(): https://github.com/sinonjs/samsam/commits/v3.0.0/

So it seems the change was very deliberate, but it no longer matches the documentation. Also nested partial array matches still "work" which seems contradictory to the PR

It also references createMatcher which I see it is exported, but I can't find any reference in the documentation to that function. So it's not clear how I could use it to recreated recursive partial object matching.

[fatso83]

Ah, I think this is the reason I found samsam unusable for my own work. The recursive matching no longer works as expected. That this does not work:

        assert.isTrue(
            samsam.match(
                {
                    o: {
                        name: "Chris",
                        profession: "Programmer",
                    },
                },
                {
                    o: {
                        name: "Chris",
                    },
                },
            ),
        );

while this does, makes no sense to me:

        assert.isTrue(
            samsam.match(
                {
                    name: "Chris",
                    profession: "Programmer",
                },
                {
                    name: "Chris",
                },
            ),
        );

[thom-nic]

Yup partial matching but only at the top level object IMO doesn't make sense. Esp while partial array matching works at any depth.

[mantoni]

Yes, I remember introducing this due to the weird behaviour that the previous implementation had.

One example is the array matching as described in the referee issue mentioned by @thom-nic . But there are actually more cases. Here is an example with strings:

// This currently returns false, but it returns true with recursive matching
samsam.match({ test: 'something' }, { test: 'thing' })

I agree that with the examples given here, the current implementation looks broken. However, if deep recursive matching is really desired, I would rather introduce it as a new feature, e.g. samsam.deepMatch(a, b), since that would be less "magic".

[mantoni]

I just ran into some inconsistency in our implementations. All of these tests pass:

import samsam from '@sinonjs/samsam';
import { assert, refute, match } from '@sinonjs/referee';

describe('samsam vs referee-sinon', () => {
  const value = {
    parent: {
      child_a: 1,
      child_b: 2
    }
  };
  const expectation = {
    parent: {
      child_a: 1
    }
  };

  context('with samsam', () => {
    it('returns false for samsam.match', () => {
      assert.isFalse(samsam.match(value, expectation));
    });

    it('returns true for samsam.createMatcher', () => {
      const matcher = samsam.createMatcher(expectation, '');

      assert.isTrue(samsam.deepEqual(value, matcher));
    });
  });

  context('with referee', () => {
    it('passes refute.match', () => {
      // This is using samsam.match:
      refute.match(value, expectation);
    });

    it('passes assert.equals with match', () => {
      // This is using samsam.createMatcher:
      assert.equals(value, match(expectation));
    });
  });
});

I was expecting assert.match(value, expectation) and assert.equals(value, match(expectation)) to be equivalent, but they aren't. 🫠

[thom-nic]

Thanks @mantoni for looking at this. Yeah I understand the desire to not introduce breaking changes again since it's been this way for so long. A new function like deepMatch would be great.

w/r/t behavior for string values, they could be treated as exact match, and if you wanted a partial match the expectation could use a regex?

[fatso83]

Thanks for looking into this, Max. I am sure me and Thom can figure out a way of doing partial matching, now that it is established that deepEquals is not supposed be that function 😄 Too bad about the inconsistencies found, though 🙈