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

Throw on incorrect property matchers usage #7128

Closed
thymikee opened this issue Oct 9, 2018 · 7 comments
Closed

Throw on incorrect property matchers usage #7128

thymikee opened this issue Oct 9, 2018 · 7 comments

Comments

@thymikee
Copy link
Collaborator

thymikee commented Oct 9, 2018

πŸš€ Feature Proposal

I'd like to propose a new behavior to recently introduced "snapshot property matchers" – throwing when the object values are not asymmetric matchers.

We should also link to the guide from expect.toMatchSnapshot docs to avoid confusion (regardless this request is accepted or not).

Motivation

I've seen this code in the wild:

expect(user).toMatchSnapshot({
  createdAt: 'mocked-date',
  id: 'gen-id',
});

Which doesn't make much sense (maybe it is sometimes, but not really), because the snapshot file is going to have this very information anyway, resulting in duplicating code and reviewing effort. Such code can easily be replaced with toMatchObject or toMatchSnapshot without property matcher.

Example

We could throw something like:

Snapshot property matchers are intended to use with asymmetric matchers.
For details on how to use it properly, see: https://jestjs.io/docs/en/snapshot-testing#property-matchers

Pitch

It's an update to core matcher.

cc @rickhanlonii

@mattphillips
Copy link
Contributor

I agree this is odd! I wonder if this makes more sense as an eslint rule?

@rickhanlonii
Copy link
Member

FWIW the current behavior was what was proposed but I agree that it makes sense to only allow asymmetric matchers now that it's better understood πŸ‘Œ

@dortamiguel
Copy link

I like the current behaviour, if you have a big response from an api that you want to snapshot and the only thing that changes is a date, then you only need to specify the only field that changes.

@rickhanlonii
Copy link
Member

@ellipticaldoor we would keep that but would add the requirement that you specify a matcher

const user = {
  name: 'Pete Davidson',
  createdAt: Date.now(),
  id: uuid(),
};

// BAD, properties must be a matcher
expect(user).toMatchSnapshot({
  createdAt: 'mocked-date',
  id: 'gen-id',
});

// GOOD
expect(user).toMatchSnapshot({
  createdAt: expect.any(Date),
  id: expect.any(String),
});

@github-actions
Copy link

This issue is stale because it has been open for 1 year with no activity. Remove stale label or comment or this will be closed in 14 days.

@github-actions github-actions bot added the Stale label Feb 25, 2022
@github-actions
Copy link

This issue was closed because it has been stalled for 7 days with no activity. Please open a new issue if the issue is still relevant, linking to this one.

@github-actions
Copy link

github-actions bot commented May 2, 2022

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 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants