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

Updates SnapshotTesting.md to provide more info. on snapshot scope #6735

Merged
merged 3 commits into from Aug 7, 2018

Conversation

kalpeshsingh
Copy link
Contributor

Summary

The doc doesn't mention where to look for the bugs in the code. I have added extra info. which will clear things to find the bugs and why does test pass when you doesn't add any props in the component used in different files.

As a beginner I thought, test will fail if I change Link component props in the any file that usage test component (in this case Link component).

Test plan

cd website
yarn
yarn start

The doc doesn't mention where to look for the bugs in the code. I have added extra info. which will clear things to find the bugs and why does test pass when you doesn't add any props in the component used in different files.

As a beginner I thought, test will fail if I change Link component props in the any file that usage test component (in this case Link component).
The snapshot artifact should be committed alongside code changes, and reviewed as part of your code review process. Jest uses [pretty-format](https://github.com/facebook/jest/tree/master/packages/pretty-format) to make snapshots human-readable during code review. On subsequent test runs Jest will simply compare the rendered output with the previous snapshot. If they match, the test will pass. If they don't match, either the test runner found a bug in your code that should be fixed, or the implementation has changed and the snapshot needs to be updated.
The snapshot artifact should be committed alongside code changes, and reviewed as part of your code review process. Jest uses [pretty-format](https://github.com/facebook/jest/tree/master/packages/pretty-format) to make snapshots human-readable during code review. On subsequent test runs Jest will simply compare the rendered output with the previous snapshot. If they match, the test will pass. If they don't match, either the test runner found a bug in your code (in this case, it's `<Link>` component) that should be fixed, or the implementation has changed and the snapshot needs to be updated.

> Note: The snapshot is directly scoped to the data you render – in our example it's <Link /> component with page prop passed to it. This implies that even if any other file has missing props (Say, `App.js`) in the `<Link />` component, it will still pass the test as the test doesn't know the usage of `<Link />` component and it's scoped only to the `Link.react.js`.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"example it's " -> "example it's <Link />"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tsk-tsk! I missed it. :(
Fixed now.

Copy link
Collaborator

@thymikee thymikee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@thymikee
Copy link
Collaborator

Fix lint pls: yarn lint:md

@codecov-io
Copy link

codecov-io commented Jul 23, 2018

Codecov Report

Merging #6735 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #6735   +/-   ##
=======================================
  Coverage   63.68%   63.68%           
=======================================
  Files         235      235           
  Lines        9007     9007           
  Branches        4        4           
=======================================
  Hits         5736     5736           
  Misses       3270     3270           
  Partials        1        1

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 a10bc90...fa02b5c. Read the comment docs.

@kalpeshsingh
Copy link
Contributor Author

@rickhanlonii Do you need any other information on this?

@thymikee
Copy link
Collaborator

thymikee commented Aug 7, 2018

ping @rickhanlonii @SimenB ;). This is an obvious note to non-beginners, but if somebody finds this useful, I'm happy to get it in.

Copy link
Member

@rickhanlonii rickhanlonii left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@rickhanlonii rickhanlonii merged commit f66b74d into jestjs:master Aug 7, 2018
thymikee added a commit to rhysawilliams2010/jest that referenced this pull request Aug 8, 2018
* upstream/master: (122 commits)
  fix: don't report promises as open handles (jestjs#6716)
  support serializing `DocumentFragment` (jestjs#6705)
  Allow test titles to include array index (jestjs#6414)
  fix `toContain` suggest to contain equal message (jestjs#6810)
  fix: testMatch not working with negations (jestjs#6648)
  Add test cases for jestjs#6744 (jestjs#6772)
  print stack trace on calls to process.exit (jestjs#6714)
  Updates SnapshotTesting.md to provide more info. on snapshot scope (jestjs#6735)
  Mark snapshots as obsolete when moved to an inline snapshot (jestjs#6773)
  [Docs] Clarified the use of literal values as property matchers in toMatchSnapshot() (jestjs#6807)
  Update CHANGELOG.md (jestjs#6799)
  fix changelog entry that is not in 23.4.2 (jestjs#6796)
  Fix --coverage with --findRelatedTests overwriting collectCoverageFrom options (jestjs#6736)
  Update testURL default value from about:blank to localhost (jestjs#6792)
  fix: `matchInlineSnapshot` when prettier dependencies are mocked (jestjs#6776)
  Fix retryTimes and add e2e regression test (jestjs#6762)
  Release v23.4.2
  Docs/ExpectAPI: Correct docs for `objectContaining` (jestjs#6754)
  chore(packages/babel-jest) readme (jestjs#6746)
  docs: noted --coverage aliased by --collectCoverage (jestjs#6741)
  ...
@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 12, 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.

None yet

5 participants