Add a 'best practices' section to the Snapshot Testing Guide #6101
Conversation
Issue: jestjs#5812 Tools get even better when it's clear how they're meant to be used. Created a 'best practices' section where patterns that are known to improve the usefulness of Jest's snapshot feature can be explained for the community's benefit.
|
I think this is great, thanks! What do you think about adding a third item? 3. Use descriptive snapshot namesAlways strive to use descriptive test and/or snapshot names for snapshots. The best names describe the expected snapshot content. This makes it easier for reviewers to verify the snapshots during review, and for anyone to know whether or not an outdated snapshot is the correct behavior before updating. For example, compare: exports[`<UserName /> should handle some test case`] = `null`;
exports[`<UserName /> should handle some other test case`] = `
<div>
Alan Turing
</div>
`;To: exports[`<UserName /> should render null`] = `null`;
exports[`<UserName /> should render Alan Turing`] = `
<div>
Alan Turing
</div>
`;Since the later describes exactly what's expected in the output, it's easy to see when it's wrong: exports[`<UserName /> should render null`] = `
<div>
Alan Turing
</div>
`;
exports[`<UserName /> should render Alan Turing`] = `null` |
|
I linted the markdown for you 👌 |
Codecov Report
@@ Coverage Diff @@
## master #6101 +/- ##
======================================
Coverage 64.1% 64.1%
======================================
Files 217 217
Lines 8335 8335
Branches 4 3 -1
======================================
Hits 5343 5343
Misses 2991 2991
Partials 1 1Continue to review full report at Codecov.
|
There was a problem hiding this comment.
This is great, thanks! Left some minor comments, but it would be great to include the section @rickhanlonii added in a comment as well 🙂
| within your application – whether that interface is a UI, logs, or error | ||
| messages; however, they're not a cure-all, and, as with any testing strategy, | ||
| there are some best-practices you should be aware of, and guidelines you should | ||
| follow, in order to use them effectively. |
There was a problem hiding this comment.
this was a loooong sentence! Mind splitting it up? I think just swapping the ; for a . should be enough 🙂
| and using tools that enforce these stylistic conventions. | ||
|
|
||
| As mentioned previously, Jest uses | ||
| [pretty-format](https://github.com/facebook/jest/tree/master/packages/pretty-format) |
There was a problem hiding this comment.
Maybe link to https://www.npmjs.com/package/pretty-format instead?
There was a problem hiding this comment.
| [pretty-format](https://github.com/facebook/jest/tree/master/packages/pretty-format) | ||
| to make snapshots human-readable, but you may find it useful to introduce | ||
| additional tools, like | ||
| [`eslint-plugin-jest`](https://github.com/jest-community/eslint-plugin-jest/blob/master/docs/rules/no-large-snapshots.md) |
There was a problem hiding this comment.
Same, maybe linking to https://www.npmjs.com/package/eslint-plugin-jest
| to make snapshots human-readable, but you may find it useful to introduce | ||
| additional tools, like | ||
| [`eslint-plugin-jest`](https://github.com/jest-community/eslint-plugin-jest/blob/master/docs/rules/no-large-snapshots.md) | ||
| with its `'no-large-snapshots'` option, and |
There was a problem hiding this comment.
and here link to the specific rule
| @@ -158,17 +189,17 @@ Now, every time the snapshot test case runs, `Date.now()` will return | |||
| `1482363367071` consistently. This will result in the same snapshot being | |||
| generated for this component regardless of when the test is run. | |||
|
|
|||
| ### Snapshots are not written automatically on Continuous Integration systems (CI) | |||
There was a problem hiding this comment.
It was phrased like this because of SEO. I guess Google should handle this though
|
Can somebody make the changes @SimenB asked for? |
|
Thanks @TrustyKnave! |
|
This landed in |
|
@SimenB, PR submitted - no need to do 22.4 specifically as the snapshot doc is inheriting from 22.3 |
|
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. |
Fixes #5812
cc: @thymikee @SimenB
Summary
As explained in the issue referenced above (opened by @thymikee), Jest's Snapshot Testing Guide could benefit from some clearer usage guidelines.
The issue's wording specifically called out adding content that would help devs avoid common mistakes and misuses of snapshot testing. After giving the page a good read it seemed like the guide already had the nucleus of this content with the "Tests Should Be Deterministic" section.
Added some content that closely follows the recommendations outlined in the @kentcdodds's article Effective Snapshot Testing referenced in the issue, then grouped this and the existing "Tests Should Be Deterministic" section under a "Best Practices" heading.
Moved the note about snapshots not being automatically written on CI into the FAQ section, since it seemed more like an FYI than part of the guide's core content.
This is just a first-pass, so feedback is warmly welcomed! 😄
Test plan
N/A (this is an isolated docs change)