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

jest-snapshot: Improve report when the matcher has properties #9104

Merged
merged 4 commits into from
Nov 4, 2019

Conversation

pedrottimark
Copy link
Contributor

@pedrottimark pedrottimark commented Oct 26, 2019

Summary

Borrow some features from toMatchObject for reports when snapshot matchers fail:

  1. Throw Matcher error: received value must be an object when the matcher has properties instead of

  2. In an ordinary test failure because received object does not match properties, display comparison lines if isLineDiffable

A similarity with toMatchObject that I did not include is getObjectSubset because:

  • it has known limitations
  • it is not a public interface
  • the long-term goal is to replace it with data-driven diff

But we decided that it is better to call getObjectSubset after all in #9198

Refactor for clearer distinction between snapshot and generic serialization:

  • call addExtraLineBreaks in State.ts for symmetry with removeExtraLineBreaks
  • change serialize not to call addExtraLineBreaks
  • add minify to call pretty-format with min: true and other serialize options
  • add printExpected and printReceived to call minify with colors
  • call stringify from jest-matcher-utils only for snapshotState error

Test plan

In printSnapshot.test.ts

  • add 1 snapshot for matcher error
  • add 1 snapshot for minified values if !isLineDiffable
  • update 3 snapshots for comparison lines if isLineDiffable
  • update 1 snapshot because printExpected uses snapshot serializer options
  • update 30 snapshot names to replace printDiffOrStringified with printSnapshotAndReceived

In utils.test.ts edit 1 toBe because serialize does not call addExtraLineBreaks

In e2e toMatchSnapshot.test.ts edit 2 toMatch for comparison lines if isLineDiffable

See also pictures in the following comment

@pedrottimark
Copy link
Contributor Author

pedrottimark commented Oct 26, 2019

Instead of ordinary test failure (first) throw matcher error (second)

1 received undefined

Compare the preceding pictures for evidence of potential future improvement to call fail method so some matcher errors can display the snapshot name

Instead of incorrect success (first) BREAKING throw matcher error (second)

2 received string

Instead of minified objects (first) display comparison lines (second)

3 isLineDiffable true

But continue to display minified objects if !isLineDiffable

4 isLineDiffable false

In the preceding picture, comparison lines of Error objects might be clearer

@codecov-io
Copy link

codecov-io commented Oct 26, 2019

Codecov Report

Merging #9104 into master will increase coverage by 0.03%.
The diff coverage is 95%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #9104      +/-   ##
==========================================
+ Coverage   64.73%   64.76%   +0.03%     
==========================================
  Files         277      277              
  Lines       11707    11720      +13     
  Branches     2874     2877       +3     
==========================================
+ Hits         7578     7591      +13     
  Misses       3512     3512              
  Partials      617      617
Impacted Files Coverage Δ
packages/jest-snapshot/src/State.ts 0% <0%> (ø) ⬆️
packages/jest-snapshot/src/printSnapshot.ts 100% <100%> (ø) ⬆️
packages/jest-snapshot/src/utils.ts 95.29% <100%> (+0.11%) ⬆️
packages/jest-snapshot/src/index.ts 73.23% <100%> (+0.38%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cc2c92b...8a0dc85. Read the comment docs.

@pedrottimark pedrottimark merged commit 7ece746 into jestjs:master Nov 4, 2019
@pedrottimark pedrottimark deleted the snapshot-properties branch November 4, 2019 19:13
@SimenB
Copy link
Member

SimenB commented Nov 5, 2019

Oh damn, this is really good!

@github-actions
Copy link

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

Successfully merging this pull request may close these issues.

4 participants