From 7e2f5e3c371eebeda15ba651a7523a52849706a0 Mon Sep 17 00:00:00 2001 From: Giorgio Polvara Date: Mon, 31 Aug 2020 10:46:46 +0200 Subject: [PATCH] feat: new no-wait-for-snapshot rule (#223) Closes: #214 --- README.md | 4 + docs/rules/no-wait-for-snapshot.test.md | 56 +++++ lib/index.ts | 2 + lib/node-utils.ts | 2 + lib/rules/no-wait-for-snapshot.ts | 132 ++++++++++++ tests/lib/rules/no-wait-for-snapshot.test.ts | 208 +++++++++++++++++++ 6 files changed, 404 insertions(+) create mode 100644 docs/rules/no-wait-for-snapshot.test.md create mode 100644 lib/rules/no-wait-for-snapshot.ts create mode 100644 tests/lib/rules/no-wait-for-snapshot.test.ts diff --git a/README.md b/README.md index df020730..ded5300b 100644 --- a/README.md +++ b/README.md @@ -23,7 +23,9 @@ [![Tweet][tweet-badge]][tweet-url] + [![All Contributors](https://img.shields.io/badge/all_contributors-31-orange.svg?style=flat-square)](#contributors-) + ## Installation @@ -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 | | | @@ -222,6 +225,7 @@ Thanks goes to these wonderful people ([emoji key](https://allcontributors.org/d + This project follows the [all-contributors](https://github.com/all-contributors/all-contributors) specification. Contributions of any kind welcome! diff --git a/docs/rules/no-wait-for-snapshot.test.md b/docs/rules/no-wait-for-snapshot.test.md new file mode 100644 index 00000000..8f35f2ab --- /dev/null +++ b/docs/rules/no-wait-for-snapshot.test.md @@ -0,0 +1,56 @@ +# Ensures no snapshot is generated inside of a `wait` call' (no-wait-for-snapshot) + +Ensure that no calls to `toMatchSnapshot` or `toMatchInlineSnapshot` 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 matched 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 waitFor(() => expect(container).toMatchInlineSnapshot()); + // ... +}; + +const baz = async () => { + // ... + await wait(() => { + expect(container).toMatchSnapshot(); + }); + // ... +}; +``` + +Examples of **correct** code for this rule: + +```js +const foo = () => { + // ... + expect(container).toMatchSnapshot(); + // ... +}; + +const bar = () => { + // ... + expect(container).toMatchInlineSnapshot(); + // ... +}; +``` + +## Further Reading + +- [Async Utilities](https://testing-library.com/docs/dom-testing-library/api-async) diff --git a/lib/index.ts b/lib/index.ts index 4b457bcf..2257bb2c 100644 --- a/lib/index.ts +++ b/lib/index.ts @@ -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'; @@ -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, diff --git a/lib/node-utils.ts b/lib/node-utils.ts index 94f6a7fc..cd8bc10c 100644 --- a/lib/node-utils.ts +++ b/lib/node-utils.ts @@ -84,6 +84,8 @@ export function findClosestCallExpressionNode( return node; } + if(!node.parent) return null + return findClosestCallExpressionNode(node.parent); } diff --git a/lib/rules/no-wait-for-snapshot.ts b/lib/rules/no-wait-for-snapshot.ts new file mode 100644 index 00000000..16235ac4 --- /dev/null +++ b/lib/rules/no-wait-for-snapshot.ts @@ -0,0 +1,132 @@ +import { ESLintUtils, TSESTree } from '@typescript-eslint/experimental-utils'; +import { getDocsUrl, ASYNC_UTILS, LIBRARY_MODULES } from '../utils'; +import { + findClosestCallExpressionNode, + isMemberExpression, +} from '../node-utils'; + +export const RULE_NAME = 'no-wait-for-snapshot'; +export type MessageIds = 'noWaitForSnapshot'; +type Options = []; + +const ASYNC_UTILS_REGEXP = new RegExp(`^(${ASYNC_UTILS.join('|')})$`); +const SNAPSHOT_REGEXP = /^(toMatchSnapshot|toMatchInlineSnapshot)$/; + +export default ESLintUtils.RuleCreator(getDocsUrl)({ + 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 `{{ name }}` call", + }, + fixable: null, + schema: [], + }, + defaultOptions: [], + + create(context) { + const asyncUtilsUsage: Array<{ + node: TSESTree.Identifier | TSESTree.MemberExpression; + name: string; + }> = []; + const importedAsyncUtils: string[] = []; + const snapshotUsage: TSESTree.Identifier[] = []; + + 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; + } + + importedAsyncUtils.push(name); + }, + [`CallExpression > Identifier[name=${ASYNC_UTILS_REGEXP}]`]( + node: TSESTree.Identifier + ) { + asyncUtilsUsage.push({ node, name: node.name }); + }, + [`CallExpression > MemberExpression > Identifier[name=${ASYNC_UTILS_REGEXP}]`]( + node: TSESTree.Identifier + ) { + const memberExpression = node.parent as TSESTree.MemberExpression; + const identifier = memberExpression.object as TSESTree.Identifier; + const memberExpressionName = identifier.name; + + asyncUtilsUsage.push({ + node: memberExpression, + name: memberExpressionName, + }); + }, + [`Identifier[name=${SNAPSHOT_REGEXP}]`](node: TSESTree.Identifier) { + snapshotUsage.push(node); + }, + 'Program:exit'() { + const testingLibraryUtilUsage = asyncUtilsUsage.filter(usage => { + if (isMemberExpression(usage.node)) { + const object = usage.node.object as TSESTree.Identifier; + + return importedAsyncUtils.includes(object.name); + } + + return importedAsyncUtils.includes(usage.name); + }); + + function getClosestAsyncUtil( + asyncUtilUsage: { + node: TSESTree.Identifier | TSESTree.MemberExpression; + name: string; + }, + node: TSESTree.Node + ) { + let callExpression = findClosestCallExpressionNode(node); + while (callExpression != null) { + if (callExpression.callee === asyncUtilUsage.node) + return asyncUtilUsage; + callExpression = findClosestCallExpressionNode( + callExpression.parent + ); + } + return null; + } + + snapshotUsage.forEach(node => { + testingLibraryUtilUsage.forEach(asyncUtilUsage => { + const closestAsyncUtil = getClosestAsyncUtil(asyncUtilUsage, node); + if (closestAsyncUtil != null) { + let name; + if (isMemberExpression(closestAsyncUtil.node)) { + name = (closestAsyncUtil.node.property as TSESTree.Identifier) + .name; + } else { + name = closestAsyncUtil.name; + } + context.report({ + node, + messageId: 'noWaitForSnapshot', + data: { name }, + }); + } + }); + }); + }, + }; + }, +}); diff --git a/tests/lib/rules/no-wait-for-snapshot.test.ts b/tests/lib/rules/no-wait-for-snapshot.test.ts new file mode 100644 index 00000000..5f489513 --- /dev/null +++ b/tests/lib/rules/no-wait-for-snapshot.test.ts @@ -0,0 +1,208 @@ +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: ` + import { ${asyncUtil} } from '@testing-library/dom'; + test('snapshot calls outside of ${asyncUtil} are valid', () => { + expect(foo).toMatchSnapshot() + await ${asyncUtil}(() => expect(foo).toBeDefined()) + expect(foo).toMatchInlineSnapshot() + }) + `, + })), + ...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).toMatchInlineSnapshot() + }) + `, + })), + ...ASYNC_UTILS.map(asyncUtil => ({ + code: ` + import * as asyncUtils from '@testing-library/dom'; + test('snapshot calls outside of ${asyncUtil} are valid', () => { + expect(foo).toMatchSnapshot() + await asyncUtils.${asyncUtil}(() => expect(foo).toBeDefined()) + expect(foo).toMatchInlineSnapshot() + }) + `, + })), + ...ASYNC_UTILS.map(asyncUtil => ({ + code: ` + import * as asyncUtils from '@testing-library/dom'; + test('snapshot calls outside of ${asyncUtil} are valid', () => { + expect(foo).toMatchSnapshot() + await asyncUtils.${asyncUtil}(() => { + expect(foo).toBeDefined() + }) + expect(foo).toMatchInlineSnapshot() + }) + `, + })), + ...ASYNC_UTILS.map(asyncUtil => ({ + code: ` + import { ${asyncUtil} } from 'some-other-library'; + test('snapshot calls within ${asyncUtil} are not valid', async () => { + await ${asyncUtil}(() => expect(foo).toMatchSnapshot()); + }); + `, + })), + ...ASYNC_UTILS.map(asyncUtil => ({ + code: ` + import { ${asyncUtil} } from 'some-other-library'; + test('snapshot calls within ${asyncUtil} are not valid', async () => { + await ${asyncUtil}(() => { + expect(foo).toMatchSnapshot() + }); + }); + `, + })), + ...ASYNC_UTILS.map(asyncUtil => ({ + code: ` + import * as asyncUtils from 'some-other-library'; + test('snapshot calls within ${asyncUtil} are not valid', async () => { + await asyncUtils.${asyncUtil}(() => expect(foo).toMatchSnapshot()); + }); + `, + })), + ...ASYNC_UTILS.map(asyncUtil => ({ + code: ` + import * as asyncUtils from 'some-other-library'; + test('snapshot calls within ${asyncUtil} are not valid', async () => { + await asyncUtils.${asyncUtil}(() => { + expect(foo).toMatchSnapshot() + }); + }); + `, + })), + ...ASYNC_UTILS.map(asyncUtil => ({ + code: ` + import { ${asyncUtil} } from 'some-other-library'; + test('snapshot calls within ${asyncUtil} are not valid', async () => { + await ${asyncUtil}(() => expect(foo).toMatchInlineSnapshot()); + }); + `, + })), + ...ASYNC_UTILS.map(asyncUtil => ({ + code: ` + import { ${asyncUtil} } from 'some-other-library'; + test('snapshot calls within ${asyncUtil} are not valid', async () => { + await ${asyncUtil}(() => { + expect(foo).toMatchInlineSnapshot() + }); + }); + `, + })), + ...ASYNC_UTILS.map(asyncUtil => ({ + code: ` + import * as asyncUtils from 'some-other-library'; + test('snapshot calls within ${asyncUtil} are not valid', async () => { + await asyncUtils.${asyncUtil}(() => expect(foo).toMatchInlineSnapshot()); + }); + `, + })), + ...ASYNC_UTILS.map(asyncUtil => ({ + code: ` + import * as asyncUtils from 'some-other-library'; + test('snapshot calls within ${asyncUtil} are not valid', async () => { + await asyncUtils.${asyncUtil}(() => { + expect(foo).toMatchInlineSnapshot() + }); + }); + `, + })), + ], + 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' }], + })), + ...ASYNC_UTILS.map(asyncUtil => ({ + code: ` + import * as asyncUtils from '@testing-library/dom'; + test('snapshot calls within ${asyncUtil} are not valid', async () => { + await asyncUtils.${asyncUtil}(() => expect(foo).toMatchSnapshot()); + }); + `, + errors: [{ line: 4, messageId: 'noWaitForSnapshot' }], + })), + ...ASYNC_UTILS.map(asyncUtil => ({ + code: ` + import * as asyncUtils from '@testing-library/dom'; + test('snapshot calls within ${asyncUtil} are not valid', async () => { + await asyncUtils.${asyncUtil}(() => { + expect(foo).toMatchSnapshot() + }); + }); + `, + errors: [{ line: 5, 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).toMatchInlineSnapshot()); + }); + `, + 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).toMatchInlineSnapshot() + }); + }); + `, + errors: [{ line: 5, messageId: 'noWaitForSnapshot' }], + })), + ...ASYNC_UTILS.map(asyncUtil => ({ + code: ` + import * as asyncUtils from '@testing-library/dom'; + test('snapshot calls within ${asyncUtil} are not valid', async () => { + await asyncUtils.${asyncUtil}(() => expect(foo).toMatchInlineSnapshot()); + }); + `, + errors: [{ line: 4, messageId: 'noWaitForSnapshot' }], + })), + ...ASYNC_UTILS.map(asyncUtil => ({ + code: ` + import * as asyncUtils from '@testing-library/dom'; + test('snapshot calls within ${asyncUtil} are not valid', async () => { + await asyncUtils.${asyncUtil}(() => { + expect(foo).toMatchInlineSnapshot() + }); + }); + `, + errors: [{ line: 5, messageId: 'noWaitForSnapshot' }], + })), + ], +});