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

toMatchSnapshot does not throw an exception on failure #9107

Closed
manu-st opened this issue Oct 28, 2019 · 8 comments
Closed

toMatchSnapshot does not throw an exception on failure #9107

manu-st opened this issue Oct 28, 2019 · 8 comments

Comments

@manu-st
Copy link

manu-st commented Oct 28, 2019

🐛 Bug Report

Whenever I use toMatchSnapshot () as in expect (val).toMatchSnapshot () and that it fails, no exception is being thrown as this is done for other matchers. Not sure if it matters, but I always use async calls.

To Reproduce

test ("Async snapshot", async () =>
{
   expect ("Some value not matching previous snapshot").toMatchSnapshot ();
}

Expected behavior

An exception is thrown so I can catch it like this is done for other matchers.

envinfo

Using jest v24.7.1 with ts-jest v24.1.0.

@thymikee
Copy link
Collaborator

cc @pedrottimark

@pedrottimark
Copy link
Contributor

The difference in behavior might be a design feature according to the following comment:

https://github.com/facebook/jest/blob/master/packages/expect/src/index.ts#L245-L250

@manu-st
Copy link
Author

manu-st commented Oct 28, 2019

It somewhat makes sense to get all the failures but the same reason would apply to any matcher.

Would it make sense to add an option to toMatchSnapshot to override that behavior and throw by default. Someone actually suggested to remove the line in question but that's not sustainable:
https://stackoverflow.com/questions/49285231/jest-tomatchsnapshot-not-throwing-an-exception

Referencing: #5802

@pedrottimark
Copy link
Contributor

Thank you for the links.

Because the code comment is 3 years old, I am not sure about the effect of a change in behavior.

Having said that, #9049 and #9104 throw some new matcher errors for failures caused by incorrect expected, received, or snapshot state

It is a hypothesis to explore whether a snapshot assertion fails or throws might affect the count appended to the name to make it unique. If throwing is a problem, then I have made it worse.

Research for this issue reminded me is snapshot assertions are added by expect.extend therefore external instead of internal. I’m not sure if that is right or wrong, considering this comment:

https://github.com/facebook/jest/blob/master/packages/expect/src/jestMatchersObject.ts#L16-L18

// Notes a built-in/internal Jest matcher.
// Jest may override the stack trace of Errors thrown by internal matchers.

@SimenB do you know about stack trace of Errors for snapshot versus other matchers?

Going a bit off issue, my first impression of #5503 is that it seems like an unintended side-effect if it creates asymmetric matchers corresponding to snapshot matchers (because they are external)

@pedrottimark
Copy link
Contributor

Here is minimal repro that expect.extend adds snapshot matchers as custom asymmetric matchers on expect but they don‘t work because they do not even have context let alone snapshotState

test('toEqual asymmetric toMatchSnapshot', () => {
  expect(true).toEqual(expect.toMatchSnapshot());
});
TypeError: Cannot read property 'dontThrow' of undefined

@cpojer Do you have any guidance how deeply for us to solve this side-effect of #5503

  • quick and dirty fix is blacklist 4 snapshot matchers from becoming asymmetric matchers
  • make snapshot matchers as similar as possible to internal matchers (for example, this issue about dontThrow on ordinary test failure) but still added separately? As evidence of ambiguity, their TypeScript declarations are in expect package because in Jest they seem like part of it.

@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 30 days.

@github-actions github-actions bot added the Stale label Feb 17, 2023
@github-actions
Copy link

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

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Mar 19, 2023
@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 Apr 22, 2023
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

4 participants