feat: 🎸 new no-wait-for-snapshot rule

Gpx · Gpx · commit af9c8eeb0cb0 · 2020-08-27T18:05:04.000+02:00
✅ Closes: #214
diff --git a/README.md b/README.md
@@ -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
@@ -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
 
 <!-- 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!
diff --git a/docs/rules/no-wait-for-snapshot.test.md b/docs/rules/no-wait-for-snapshot.test.md
@@ -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)
diff --git 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
@@ -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
@@ -0,0 +1,115 @@
+import { ESLintUtils, TSESTree } from '@typescript-eslint/experimental-utils';
+import { getDocsUrl, ASYNC_UTILS, LIBRARY_MODULES } from '../utils';
+import { findClosestCallExpressionNode } 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('|')})$`);
+
+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 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='toMatchSnapshot']`](node: TSESTree.Identifier) {
+        snapshotUsage.push(node);
+      },
+      'Program:exit'() {
+        const testingLibraryUtilUsage = asyncUtilsUsage.filter(usage => {
+          if (usage.node.type === 'MemberExpression') {
+            const object = usage.node.object as TSESTree.Identifier;
+
+            return importedAsyncUtils.includes(object.name);
+          }
+
+          return importedAsyncUtils.includes(usage.name);
+        });
+
+        function isWithinAsyncUtil(
+          asyncUtilUsage: {
+            node: TSESTree.Identifier | TSESTree.MemberExpression;
+            name: string;
+          },
+          node: TSESTree.Node
+        ) {
+          let callExpression = findClosestCallExpressionNode(node);
+          while (callExpression != null) {
+            if (callExpression.callee === asyncUtilUsage.node) return true;
+            callExpression = findClosestCallExpressionNode(
+              callExpression.parent
+            );
+          }
+          return false;
+        }
+
+        snapshotUsage.forEach(node => {
+          testingLibraryUtilUsage.forEach(asyncUtilUsage => {
+            if (isWithinAsyncUtil(asyncUtilUsage, node)) {
+              context.report({ node, messageId: 'noWaitForSnapshot' });
+            }
+          });
+        });
+      },
+    };
+  },
+});
diff --git a/tests/lib/rules/no-wait-for-snapshot.test.ts b/tests/lib/rules/no-wait-for-snapshot.test.ts
@@ -0,0 +1,132 @@
+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).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 * 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).toMatchSnapshot()
+        })
+      `,
+    })),
+    ...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).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 { ${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()
+          });
+        });
+      `,
+    })),
+  ],
+  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' }],
+    })),
+  ],
+});

Original file line number	Diff line number	Diff line change
`@@ -84,6 +84,8 @@ export function findClosestCallExpressionNode(`
`84`	`84`	`return node;`
`85`	`85`	`}`
`86`	`86`
	`87`	`+ if(!node.parent) return null`
	`88`	`+`
`87`	`89`	`return findClosestCallExpressionNode(node.parent);`
`88`	`90`	`}`
`89`	`91`