-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
jest-snapshot: Remove report method and throw matcher errors #9049
Conversation
Matcher error if incorrect modified or invalid arguments: I included the following more improved alternative in this pull request What do you think about the following alternative with
Baseline incorrect behavior is to pass with error snapshot: Baseline above and improved below: But not if valid arguments: Residue: if it is correct to call |
Codecov Report
@@ Coverage Diff @@
## master #9049 +/- ##
=========================================
+ Coverage 64.05% 64.65% +0.6%
=========================================
Files 277 277
Lines 11676 11688 +12
Branches 2863 2867 +4
=========================================
+ Hits 7479 7557 +78
+ Misses 3572 3514 -58
+ Partials 625 617 -8
Continue to review full report at Codecov.
|
I included the more improved version in this pull request as prerequisite for the next pull request to improve colors of snapshot changes: color convention of Does color difference help you see which of adjacent arguments in hint is relevant to rest of report? Current improved is above and potentially more improved is below When snapshot is not updatable how about When snapshot is updatable how about |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I hope you didn't hide anything in the huge rename diffs ;)
Do we have anyone more familiar with jest-snapshot to review its significantly changed impl?
Tim, you have my apology about the rename mess. Sadly, it is up to us to become familiar. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regarding your question on propertiesOr*
- toMatchObject
also just takes object
, that's the same situation, isn't it? Don't think there is a better typing
Sorry that no one has enough time at the moment :| |
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Summary
Remove
report
method from returned object (added in Amend jest snapshots to tie in nicely with Ava #2130 for AVA, but ironically was not called then, nor called now by any dependent ofjest-snapshot
package)Throw matcher errors when assertion fails because invalid arguments:
Do not eagerly precompute
option
because it is not used if an assertion passesCall new
matcherHintFromConfig
function after matcher arguments have been validated:Call renamed
printSnapshotName
function whenever thecount
is known:Improve type annotation of
propertiesOrHint
andpropertiesOrSnapshot
Question: do y’all recommend something better thanobject
type?Make third argument optional in
matcherErrorMessage
fromjest-matcher-utils
Rename
print.ts
asprintSnapshot.ts
Residue
toThrowErrorMatchingInlineSnapshot
need to callstripAddedIndentation
?fail
instead of throw error?To minimize breaking changes in Jest 25, wait until next version:
|| hint
tofullTestName
and move it toutils.ts
Test plan
Rename
printDiffOrStringified.test.*
asprintSnapshot.test.*
describe
blockindex.ts
filetoMatchSnapshot
toThrowErrorMatchingInlineSnapshot
toThrowErrorMatchingSnapshot
See also pictures in following comments: baseline is at left and improved is at right