Skip to content

Commit

Permalink
feat: 🎸 new no-wait-snapshot rule
Browse files Browse the repository at this point in the history
✅ Closes: #214
  • Loading branch information
Gpx committed Aug 27, 2020
1 parent 636273a commit 143d454
Show file tree
Hide file tree
Showing 5 changed files with 188 additions and 0 deletions.
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@
[![Tweet][tweet-badge]][tweet-url]

<!-- ALL-CONTRIBUTORS-BADGE:START - Do not remove or modify this section -->

[![All Contributors](https://img.shields.io/badge/all_contributors-31-orange.svg?style=flat-square)](#contributors-)

<!-- ALL-CONTRIBUTORS-BADGE:END -->

## Installation
Expand Down Expand Up @@ -143,6 +145,7 @@ To enable this configuration use the `extends` property in your
| [no-manual-cleanup](docs/rules/no-manual-cleanup.md) | Disallow the use of `cleanup` | | |
| [no-render-in-setup](docs/rules/no-render-in-setup.md) | Disallow the use of `render` in setup functions | | |
| [no-wait-for-empty-callback](docs/rules/no-wait-for-empty-callback.md) | Disallow empty callbacks for `waitFor` and `waitForElementToBeRemoved` | | |
| [no-wait-for-snapshot](docs/rules/no-wait-for-snapshot.md) | Ensures no snapshot is generated inside of a `waitFor` call | | |
| [prefer-explicit-assert](docs/rules/prefer-explicit-assert.md) | Suggest using explicit assertions rather than just `getBy*` queries | | |
| [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][] |
| [prefer-presence-queries](docs/rules/prefer-presence-queries.md) | Enforce specific queries when checking element is present or not | | |
Expand Down Expand Up @@ -222,6 +225,7 @@ Thanks goes to these wonderful people ([emoji key](https://allcontributors.org/d

<!-- markdownlint-enable -->
<!-- prettier-ignore-end -->

<!-- ALL-CONTRIBUTORS-LIST:END -->

This project follows the [all-contributors](https://github.com/all-contributors/all-contributors) specification. Contributions of any kind welcome!
44 changes: 44 additions & 0 deletions docs/rules/no-wait-for-snapshot.test.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
# Ensures no snapshot is generated inside of a `wait` call' (no-wait-for-snapshot)

Ensure that no calls to `toMatchSnapshot` are made from within a `waitFor` method (or any of the other async utility methods).

## Rule Details

The `waitFor()` method runs in a timer loop. So it'll retry every n amount of time.
If a snapshot is generated inside the wait condition, jest will generate one snapshot per loop.

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.

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).

Examples of **incorrect** code for this rule:

```js
const foo = async () => {
// ...
await waitFor(() => expect(container).toMatchSnapshot());
// ...
};

const bar = async () => {
// ...
await wait(() => {
expect(container).toMatchSnapshot();
});
// ...
};
```

Examples of **correct** code for this rule:

```js
const foo = () => {
// ...
expect(container).toMatchSnapshot();
// ...
};
```

## Further Reading

- [Async Utilities](https://testing-library.com/docs/dom-testing-library/api-async)
2 changes: 2 additions & 0 deletions lib/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import noDomImport from './rules/no-dom-import';
import noManualCleanup from './rules/no-manual-cleanup';
import noRenderInSetup from './rules/no-render-in-setup';
import noWaitForEmptyCallback from './rules/no-wait-for-empty-callback';
import noWaitForSnapshot from './rules/no-wait-for-snapshot';
import preferExplicitAssert from './rules/prefer-explicit-assert';
import preferPresenceQueries from './rules/prefer-presence-queries';
import preferScreenQueries from './rules/prefer-screen-queries';
Expand All @@ -25,6 +26,7 @@ const rules = {
'no-manual-cleanup': noManualCleanup,
'no-render-in-setup': noRenderInSetup,
'no-wait-for-empty-callback': noWaitForEmptyCallback,
'no-wait-for-snapshot': noWaitForSnapshot,
'prefer-explicit-assert': preferExplicitAssert,
'prefer-find-by': preferFindBy,
'prefer-presence-queries': preferPresenceQueries,
Expand Down
67 changes: 67 additions & 0 deletions lib/rules/no-wait-for-snapshot.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import { ESLintUtils, TSESTree } from '@typescript-eslint/experimental-utils';
import { getDocsUrl, ASYNC_UTILS, LIBRARY_MODULES } from '../utils';
import { findClosestCallNode } from '../node-utils';

export const RULE_NAME = 'no-wait-for-snapshot';
export type MessageIds = 'noWaitForSnapshot';
type Options = [];

function isWithinImportedAsyncUtil(
importedAsyncUtils: string[],
node: TSESTree.Node
) {
return importedAsyncUtils.some(
asyncUtil => findClosestCallNode(node, asyncUtil) != null
);
}

export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
name: RULE_NAME,
meta: {
type: 'problem',
docs: {
description:
'Ensures no snapshot is generated inside of a `waitFor` call',
category: 'Best Practices',
recommended: 'warn',
},
messages: {
noWaitForSnapshot:
"A snapshot can't be generated inside of a `waitFor` call",
},
fixable: null,
schema: [],
},
defaultOptions: [],

create(context) {
const importedAsyncUtils: string[] = [];

return {
'ImportDeclaration > ImportSpecifier,ImportNamespaceSpecifier'(
node: TSESTree.Node
) {
const parent = node.parent as TSESTree.ImportDeclaration;

if (!LIBRARY_MODULES.includes(parent.source.value.toString())) return;

let name;
if (node.type === 'ImportSpecifier') {
name = node.imported.name;
}

if (node.type === 'ImportNamespaceSpecifier') {
name = node.local.name;
}

if (!ASYNC_UTILS.includes(name)) return;

importedAsyncUtils.push(name);
},
[`Identifier[name='toMatchSnapshot']`](node: TSESTree.Identifier) {
if (isWithinImportedAsyncUtil(importedAsyncUtils, node))
context.report({ node, messageId: 'noWaitForSnapshot' });
},
};
},
});
71 changes: 71 additions & 0 deletions tests/lib/rules/no-wait-for-snapshot.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
import { createRuleTester } from '../test-utils';
import rule, { RULE_NAME } from '../../../lib/rules/no-wait-for-snapshot';
import { ASYNC_UTILS } from '../../../lib/utils';

const ruleTester = createRuleTester();

ruleTester.run(RULE_NAME, rule, {
valid: [
...ASYNC_UTILS.map(asyncUtil => ({
code: `
test('snapshot calls outside of ${asyncUtil} are valid', () => {
expect(foo).toMatchSnapshot()
await ${asyncUtil}(() => expect(foo).toBeDefined())
expect(foo).toMatchSnapshot()
})
`,
})),
...ASYNC_UTILS.map(asyncUtil => ({
code: `
import { ${asyncUtil} } from '@testing-library/dom';
test('snapshot calls outside of ${asyncUtil} are valid', () => {
expect(foo).toMatchSnapshot()
await ${asyncUtil}(() => {
expect(foo).toBeDefined()
})
expect(foo).toMatchSnapshot()
})
`,
})),
...ASYNC_UTILS.map(asyncUtil => ({
code: `
import { ${asyncUtil} } from 'a-different-library';
test('snapshot calls within ${asyncUtil} are not valid', async () => {
await ${asyncUtil}(() => expect(foo).toMatchSnapshot());
});
`,
})),
...ASYNC_UTILS.map(asyncUtil => ({
code: `
import { ${asyncUtil} } from 'a-different-library';
test('snapshot calls within ${asyncUtil} are not valid', async () => {
await ${asyncUtil}(() => {
expect(foo).toMatchSnapshot()
});
});
`,
})),
],
invalid: [
...ASYNC_UTILS.map(asyncUtil => ({
code: `
import { ${asyncUtil} } from '@testing-library/dom';
test('snapshot calls within ${asyncUtil} are not valid', async () => {
await ${asyncUtil}(() => expect(foo).toMatchSnapshot());
});
`,
errors: [{ line: 4, messageId: 'noWaitForSnapshot' }],
})),
...ASYNC_UTILS.map(asyncUtil => ({
code: `
import { ${asyncUtil} } from '@testing-library/dom';
test('snapshot calls within ${asyncUtil} are not valid', async () => {
await ${asyncUtil}(() => {
expect(foo).toMatchSnapshot()
});
});
`,
errors: [{ line: 5, messageId: 'noWaitForSnapshot' }],
})),
],
});

0 comments on commit 143d454

Please sign in to comment.