Skip to content

Commit 7a51c71

Browse files
committed
feat: 🎸 new no-wait-for-snapshot rule
✅ Closes: #214
1 parent 636273a commit 7a51c71

File tree

5 files changed

+188
-0
lines changed

5 files changed

+188
-0
lines changed

‎README.md

+4
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,9 @@
2323
[![Tweet][tweet-badge]][tweet-url]
2424

2525
<!-- ALL-CONTRIBUTORS-BADGE:START - Do not remove or modify this section -->
26+
2627
[![All Contributors](https://img.shields.io/badge/all_contributors-31-orange.svg?style=flat-square)](#contributors-)
28+
2729
<!-- ALL-CONTRIBUTORS-BADGE:END -->
2830

2931
## Installation
@@ -143,6 +145,7 @@ To enable this configuration use the `extends` property in your
143145
| [no-manual-cleanup](docs/rules/no-manual-cleanup.md) | Disallow the use of `cleanup` | | |
144146
| [no-render-in-setup](docs/rules/no-render-in-setup.md) | Disallow the use of `render` in setup functions | | |
145147
| [no-wait-for-empty-callback](docs/rules/no-wait-for-empty-callback.md) | Disallow empty callbacks for `waitFor` and `waitForElementToBeRemoved` | | |
148+
| [no-wait-for-snapshot](docs/rules/no-wait-for-snapshot.md) | Ensures no snapshot is generated inside of a `waitFor` call | | |
146149
| [prefer-explicit-assert](docs/rules/prefer-explicit-assert.md) | Suggest using explicit assertions rather than just `getBy*` queries | | |
147150
| [prefer-find-by](docs/rules/prefer-find-by.md) | Suggest using `findBy*` methods instead of the `waitFor` + `getBy` queries | ![recommended-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | ![fixable-badge][] |
148151
| [prefer-presence-queries](docs/rules/prefer-presence-queries.md) | Enforce specific queries when checking element is present or not | | |
@@ -222,6 +225,7 @@ Thanks goes to these wonderful people ([emoji key](https://allcontributors.org/d
222225

223226
<!-- markdownlint-enable -->
224227
<!-- prettier-ignore-end -->
228+
225229
<!-- ALL-CONTRIBUTORS-LIST:END -->
226230

227231
This project follows the [all-contributors](https://github.com/all-contributors/all-contributors) specification. Contributions of any kind welcome!
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
# Ensures no snapshot is generated inside of a `wait` call' (no-wait-for-snapshot)
2+
3+
Ensure that no calls to `toMatchSnapshot` are made from within a `waitFor` method (or any of the other async utility methods).
4+
5+
## Rule Details
6+
7+
The `waitFor()` method runs in a timer loop. So it'll retry every n amount of time.
8+
If a snapshot is generated inside the wait condition, jest will generate one snapshot per loop.
9+
10+
The problem then is the amount of loop ran until the condition is met will vary between different computers (or CI machines.) This leads to tests that will regenerate a lot of snapshots until the condition is match when devs run those tests locally updating the snapshots; e.g devs cannot run `jest -u` locally or it'll generate a lot of invalid snapshots who'll fail during CI.
11+
12+
Note that this lint rule prevents from generating a snapshot from within any of the [async utility methods](https://testing-library.com/docs/dom-testing-library/api-async).
13+
14+
Examples of **incorrect** code for this rule:
15+
16+
```js
17+
const foo = async () => {
18+
// ...
19+
await waitFor(() => expect(container).toMatchSnapshot());
20+
// ...
21+
};
22+
23+
const bar = async () => {
24+
// ...
25+
await wait(() => {
26+
expect(container).toMatchSnapshot();
27+
});
28+
// ...
29+
};
30+
```
31+
32+
Examples of **correct** code for this rule:
33+
34+
```js
35+
const foo = () => {
36+
// ...
37+
expect(container).toMatchSnapshot();
38+
// ...
39+
};
40+
```
41+
42+
## Further Reading
43+
44+
- [Async Utilities](https://testing-library.com/docs/dom-testing-library/api-async)

‎lib/index.ts

+2
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import noDomImport from './rules/no-dom-import';
88
import noManualCleanup from './rules/no-manual-cleanup';
99
import noRenderInSetup from './rules/no-render-in-setup';
1010
import noWaitForEmptyCallback from './rules/no-wait-for-empty-callback';
11+
import noWaitForSnapshot from './rules/no-wait-for-snapshot';
1112
import preferExplicitAssert from './rules/prefer-explicit-assert';
1213
import preferPresenceQueries from './rules/prefer-presence-queries';
1314
import preferScreenQueries from './rules/prefer-screen-queries';
@@ -25,6 +26,7 @@ const rules = {
2526
'no-manual-cleanup': noManualCleanup,
2627
'no-render-in-setup': noRenderInSetup,
2728
'no-wait-for-empty-callback': noWaitForEmptyCallback,
29+
'no-wait-for-snapshot': noWaitForSnapshot,
2830
'prefer-explicit-assert': preferExplicitAssert,
2931
'prefer-find-by': preferFindBy,
3032
'prefer-presence-queries': preferPresenceQueries,

‎lib/rules/no-wait-for-snapshot.ts

+67
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
import { ESLintUtils, TSESTree } from '@typescript-eslint/experimental-utils';
2+
import { getDocsUrl, ASYNC_UTILS, LIBRARY_MODULES } from '../utils';
3+
import { findClosestCallNode } from '../node-utils';
4+
5+
export const RULE_NAME = 'no-wait-for-snapshot';
6+
export type MessageIds = 'noWaitForSnapshot';
7+
type Options = [];
8+
9+
function isWithinImportedAsyncUtil(
10+
importedAsyncUtils: string[],
11+
node: TSESTree.Node
12+
) {
13+
return importedAsyncUtils.some(
14+
asyncUtil => findClosestCallNode(node, asyncUtil) != null
15+
);
16+
}
17+
18+
export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
19+
name: RULE_NAME,
20+
meta: {
21+
type: 'problem',
22+
docs: {
23+
description:
24+
'Ensures no snapshot is generated inside of a `waitFor` call',
25+
category: 'Best Practices',
26+
recommended: 'warn',
27+
},
28+
messages: {
29+
noWaitForSnapshot:
30+
"A snapshot can't be generated inside of a `waitFor` call",
31+
},
32+
fixable: null,
33+
schema: [],
34+
},
35+
defaultOptions: [],
36+
37+
create(context) {
38+
const importedAsyncUtils: string[] = [];
39+
40+
return {
41+
'ImportDeclaration > ImportSpecifier,ImportNamespaceSpecifier'(
42+
node: TSESTree.Node
43+
) {
44+
const parent = node.parent as TSESTree.ImportDeclaration;
45+
46+
if (!LIBRARY_MODULES.includes(parent.source.value.toString())) return;
47+
48+
let name;
49+
if (node.type === 'ImportSpecifier') {
50+
name = node.imported.name;
51+
}
52+
53+
if (node.type === 'ImportNamespaceSpecifier') {
54+
name = node.local.name;
55+
}
56+
57+
if (!ASYNC_UTILS.includes(name)) return;
58+
59+
importedAsyncUtils.push(name);
60+
},
61+
[`Identifier[name='toMatchSnapshot']`](node: TSESTree.Identifier) {
62+
if (isWithinImportedAsyncUtil(importedAsyncUtils, node))
63+
context.report({ node, messageId: 'noWaitForSnapshot' });
64+
},
65+
};
66+
},
67+
});
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
import { createRuleTester } from '../test-utils';
2+
import rule, { RULE_NAME } from '../../../lib/rules/no-wait-for-snapshot';
3+
import { ASYNC_UTILS } from '../../../lib/utils';
4+
5+
const ruleTester = createRuleTester();
6+
7+
ruleTester.run(RULE_NAME, rule, {
8+
valid: [
9+
...ASYNC_UTILS.map(asyncUtil => ({
10+
code: `
11+
test('snapshot calls outside of ${asyncUtil} are valid', () => {
12+
expect(foo).toMatchSnapshot()
13+
await ${asyncUtil}(() => expect(foo).toBeDefined())
14+
expect(foo).toMatchSnapshot()
15+
})
16+
`,
17+
})),
18+
...ASYNC_UTILS.map(asyncUtil => ({
19+
code: `
20+
import { ${asyncUtil} } from '@testing-library/dom';
21+
test('snapshot calls outside of ${asyncUtil} are valid', () => {
22+
expect(foo).toMatchSnapshot()
23+
await ${asyncUtil}(() => {
24+
expect(foo).toBeDefined()
25+
})
26+
expect(foo).toMatchSnapshot()
27+
})
28+
`,
29+
})),
30+
...ASYNC_UTILS.map(asyncUtil => ({
31+
code: `
32+
import { ${asyncUtil} } from 'a-different-library';
33+
test('snapshot calls within ${asyncUtil} are not valid', async () => {
34+
await ${asyncUtil}(() => expect(foo).toMatchSnapshot());
35+
});
36+
`,
37+
})),
38+
...ASYNC_UTILS.map(asyncUtil => ({
39+
code: `
40+
import { ${asyncUtil} } from 'a-different-library';
41+
test('snapshot calls within ${asyncUtil} are not valid', async () => {
42+
await ${asyncUtil}(() => {
43+
expect(foo).toMatchSnapshot()
44+
});
45+
});
46+
`,
47+
})),
48+
],
49+
invalid: [
50+
...ASYNC_UTILS.map(asyncUtil => ({
51+
code: `
52+
import { ${asyncUtil} } from '@testing-library/dom';
53+
test('snapshot calls within ${asyncUtil} are not valid', async () => {
54+
await ${asyncUtil}(() => expect(foo).toMatchSnapshot());
55+
});
56+
`,
57+
errors: [{ line: 4, messageId: 'noWaitForSnapshot' }],
58+
})),
59+
...ASYNC_UTILS.map(asyncUtil => ({
60+
code: `
61+
import { ${asyncUtil} } from '@testing-library/dom';
62+
test('snapshot calls within ${asyncUtil} are not valid', async () => {
63+
await ${asyncUtil}(() => {
64+
expect(foo).toMatchSnapshot()
65+
});
66+
});
67+
`,
68+
errors: [{ line: 5, messageId: 'noWaitForSnapshot' }],
69+
})),
70+
],
71+
});

0 commit comments

Comments
 (0)