Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Are literal values deliberately allowed as property matchers in toMatchSnapshot()? #6774

Closed
InExtremaRes opened this issue Jul 28, 2018 · 7 comments · Fixed by #6807
Closed

Comments

@InExtremaRes
Copy link
Contributor

The docs say here:

For these cases, Jest allows providing an asymmetric matcher for any property. These matchers are checked before the snapshot is written or tested, and then saved to the snapshot file instead of the received value:

and here:

The optional propertyMatchers argument allows you to specify asymmetric matchers which are verified instead of the exact values.

Both only refers to asymmetric matchers. But if I do this everything works:

expect({ p: 'hi' }).toMatchSnapshot({ p: 'hi' });

Even if I'm not using an asymmetric matcher, but using a literal value, the expectation passes and the snapshot is written correctly.

So the question is: Is the fact that the value of a property matcher works with a literal value instead of an asymmetric matcher planned, even if the docs doesn't say anything about, or is it a coincidence and my code could be suddenly break and destroy all life as we know it?

I am asking this here because I think this is a design question or perhaps it shows that docs are incomplete. If this was not appropriate I apologize and understand if it gets closed.

Here a little context. Similar to #6455 (and maybe until #6528), it is not straightforward to put an asymmetric matcher on a nested property of an object or on an object inside and array. The workaround I'm using is something like this:

test('foo', () => {
    // The `anArray` property could be very large. Every object inside
    // has an `id` property with an autogenerated value.
    const obj = { anArray: [{ id: 'autogenerated', p: 1 }] };

    expect(obj).toMatchSnapshot({
        anArray: obj.anArray.map(a => ({
            ...a, // this "copies" every other property so will be written
                  // in the snapshot and compared the next time.
            id: expect.any(String),
        })),
    });
});

In that example the anArray property is using a literal value (the array returned from map) and the spreading of every property of every element in ...a. Nonetheless, and beyond this particular example, the question holds in general.

@thymikee
Copy link
Collaborator

AFAIK it's expected behavior. @rickhanlonii can you confirm that?

@rickhanlonii
Copy link
Member

Is the fact that the value of a property matcher works with a literal value instead of an asymmetric matcher planned

Great question @InExtremaRes. The short answer is yes and we should update the docs.

For some background, the proposal was that only the property matchers would be checked, but the whole object would be merged. This would allow you to specify overrides for certain values in the object, as in:

const user = {
  id: '58e2a3fc05649b0f00497fed',
  name: 'Mike',
};

expect(user).toMatchSnapshot({ id: 'generated-id' });

exports[`something 1`] = `
Object {
  "id": "generated-id",
  "name": "Mike",
}
`;

During implementation we realized this was confusing/unexpected behavior and that if you want to override what's in the snapshot you should use a matcher:

expect(user).toMatchSnapshot({ id: expect.any(String) });

exports[`something 1`] = `
Object {
  "id": Any<String>,
  "name": "Mike",
}
`;

When it came to what to do for non-matcher properties (if they weren't going to override) it seemed to make sense to match them exactly, (also much easier to implement) so the docs should be updated to say:

values will be matched exactly if not provided as a matcher

By the way, using custom asymmetric matchers, the above example can be written as:

expect(user).toMatchSnapshot({ id: expect.objectId() });

exports[`something 1`] = `
Object {
  "id": Any<ObjectID>,
  "name": "Mike",
}
`;

Which is probably the best case in terms of both readability and asserting values.

@InExtremaRes
Copy link
Contributor Author

InExtremaRes commented Jul 31, 2018

@rickhanlonii Thank you for your answer! My questions and sufferings are now gone. Feel free to close this issue or keep it open until the docs are modified. Now I need to open a new one for the typings of TypeScript, they only work for matchers...

@rickhanlonii
Copy link
Member

No sweat! Let's keep it open to update the docs - @InExtremaRes do you want to submit a PR?

@InExtremaRes
Copy link
Contributor Author

@rickhanlonii Sure! I'll do my best.

To clarify:

The initial proposal says (emphasis mine)

I propose that toMatchSnapshot accepts a new argument which is deeply merged into the received object before snapshotting.

That is at the moment not quiet true, isn't it? Not at least until #6528 I guess. I mean, and as far as I can understand, the property matcher object is not deeply merged at the moment the snapshot is written, but it is deeply checked with the expected object.

expect({
    deep: { a: 'hi', b: 'bye' }
}).toMatchSnapshot({
    deep: { b: 'bye' }
});


exports[`...`] = `
Object {
  "deep": Object {
    "b": "bye",
  },
}
`;

So the text

values will be matched exactly if not provided as a matcher

is only technically true for primitives, but for objects is, at least, a confusing statement IMO.

Could you explain to me the current functionality on that matter?

@rickhanlonii
Copy link
Member

@InExtremaRes yes, that's true - but update the docs as if it's deeply merged and we'll be sure to merge #6528 at the same time

@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants