feat: add rule no-multiple-assertions-wait-for (#189)

* feat: add initial files for no-multiple-expect-wait-for rule * fix: add expect fields in test * feat: add no-multiple-assertion-wait-for logic * feat: add findClosestCalleName in node-utils * feat: add check for expect and rename file * docs: add no-multiple-assertions-wait-for rule doc * docs: add link for no-multiple-assertions-wait-for doc * docs: insert function example in no-multiple-assertions-wait-for * refactor: remove find closest call node from node-utils * fix: check expect based in total * docs: better english in no-multiple-assertions-wait-for rule details Co-authored-by: Tim Deschryver <[email protected]> * fix: use correct rule name in no-multiple-assertions-wait-for Co-authored-by: Tim Deschryver <[email protected]> * docs: improve docs for no-multiple-assertions-wait-for * fix: typo in no-multiple-assertions-wait-for * fix: better english in no-multiple-assertions-wait-for Co-authored-by: Tim Deschryver <[email protected]>
testing-library · Jun 30, 2020 · 11d67b2 · 11d67b2
1 parent 9db40ee
commit 11d67b2
Show file tree

Hide file tree

Showing 5 changed files with 233 additions and 0 deletions.
diff --git a/README.md b/README.md
@@ -136,6 +136,7 @@ To enable this configuration use the `extends` property in your
 | [no-debug](docs/rules/no-debug.md)                                     | Disallow the use of `debug`                                                | ![angular-badge][] ![react-badge][] ![vue-badge][]                |                    |
 | [no-dom-import](docs/rules/no-dom-import.md)                           | Disallow importing from DOM Testing Library                                | ![angular-badge][] ![react-badge][] ![vue-badge][]                | ![fixable-badge][] |
 | [no-manual-cleanup](docs/rules/no-manual-cleanup.md)                   | Disallow the use of `cleanup`                                              |                                                                   |                    |
+| [no-multiple-assertions-wait-for](docs/rules/no-multiple-assertions-wait-for.md) | Disallow the use of multiple expect inside `waitFor`             |                                                                   |                    |
 | [no-promise-in-fire-event](docs/rules/no-promise-in-fire-event.md)     | Disallow the use of promises passed to a `fireEvent` method                |                                                                   |                    |
 | [no-wait-for-empty-callback](docs/rules/no-wait-for-empty-callback.md) | Disallow empty callbacks for `waitFor` and `waitForElementToBeRemoved`     | ![dom-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] |                    |
 | [prefer-explicit-assert](docs/rules/prefer-explicit-assert.md)         | Suggest using explicit assertions rather than just `getBy*` queries        |                                                                   |                    |

diff --git a/docs/rules/no-multiple-assertions-wait-for.md b/docs/rules/no-multiple-assertions-wait-for.md
@@ -0,0 +1,52 @@
+# Multiple assertions inside `waitFor` are not preferred (no-multiple-assertions-wait-for)
+
+## Rule Details
+
+This rule aims to ensure the correct usage of `expect` inside `waitFor`, in the way that they're intended to be used.
+When using multiples assertions inside `waitFor`, if one fails, you have to wait for a timeout before seeing it failing.
+Putting one assertion, you can both wait for the UI to settle to the state you want to assert on,
+and also fail faster if one of the assertions do end up failing
+
+Example of **incorrect** code for this rule:
+
+```js
+const foo = async () => {
+  await waitFor(() => {
+    expect(a).toEqual('a');
+    expect(b).toEqual('b');
+  });
+
+  // or
+  await waitFor(function() {
+    expect(a).toEqual('a')
+    expect(b).toEqual('b');
+  })
+};
+```
+
+Examples of **correct** code for this rule:
+
+```js
+const foo = async () => {
+  await waitFor(() => expect(a).toEqual('a'));
+  expect(b).toEqual('b');
+
+  // or
+  await waitFor(function() {
+    expect(a).toEqual('a')
+  })
+  expect(b).toEqual('b');
+
+  // it only detects expect
+  // so this case doesn't generate warnings
+  await waitFor(() => {
+    fireEvent.keyDown(input, { key: 'ArrowDown' });
+    expect(b).toEqual('b');
+  });
+};
+```
+
+## Further Reading
+
+- [about `waitFor`](https://testing-library.com/docs/dom-testing-library/api-async#waitfor)
+- [inspiration for this rule](https://kentcdodds.com/blog/common-mistakes-with-react-testing-library#having-multiple-assertions-in-a-single-waitfor-callback)
diff --git a/lib/index.ts b/lib/index.ts
@@ -13,6 +13,7 @@ import preferExplicitAssert from './rules/prefer-explicit-assert';
 import preferPresenceQueries from './rules/prefer-presence-queries';
 import preferScreenQueries from './rules/prefer-screen-queries';
 import preferWaitFor from './rules/prefer-wait-for';
+import noMultipleAssertionsWaitFor from './rules/no-multiple-assertions-wait-for'
 import preferFindBy from './rules/prefer-find-by';
 
 const rules = {
@@ -25,6 +26,7 @@ const rules = {
   'no-debug': noDebug,
   'no-dom-import': noDomImport,
   'no-manual-cleanup': noManualCleanup,
+  'no-multiple-assertions-wait-for': noMultipleAssertionsWaitFor,
   'no-promise-in-fire-event': noPromiseInFireEvent,
   'no-wait-for-empty-callback': noWaitForEmptyCallback,
   'prefer-explicit-assert': preferExplicitAssert,

diff --git a/lib/rules/no-multiple-assertions-wait-for.ts b/lib/rules/no-multiple-assertions-wait-for.ts
@@ -0,0 +1,63 @@
+import { ESLintUtils, TSESTree } from '@typescript-eslint/experimental-utils'
+import { getDocsUrl } from '../utils'
+import { isBlockStatement, findClosestCallNode, isMemberExpression, isCallExpression, isIdentifier } from '../node-utils'
+
+export const RULE_NAME = 'no-multiple-assertions-wait-for';
+
+const WAIT_EXPRESSION_QUERY =
+  'CallExpression[callee.name=/^(waitFor)$/]';
+
+export type MessageIds = 'noMultipleAssertionWaitFor';
+type Options = [];
+
+export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
+  name: RULE_NAME,
+  meta: {
+    type: 'suggestion',
+    docs: {
+      description:
+        "It's preferred to avoid multiple assertions in `waitFor`",
+      category: 'Best Practices',
+      recommended: false,
+    },
+    messages: {
+      noMultipleAssertionWaitFor: 'Avoid using multiple assertions within `waitFor` callback',
+    },
+    fixable: null,
+    schema: [],
+  },
+  defaultOptions: [],
+  create: function(context) {
+    function reportMultipleAssertion(
+      node: TSESTree.BlockStatement
+    ) {
+      const totalExpect = (body: Array<TSESTree.Node>): Array<TSESTree.Node> =>
+        body.filter((node: TSESTree.ExpressionStatement) => {
+          if (
+            isCallExpression(node.expression) &&
+            isMemberExpression(node.expression.callee) &&
+            isCallExpression(node.expression.callee.object)
+          ) {
+            const object: TSESTree.CallExpression = node.expression.callee.object
+            const expressionName: string = isIdentifier(object.callee) && object.callee.name
+            return expressionName === 'expect'
+          } else {
+            return false
+          }
+        })
+
+      if (isBlockStatement(node) && totalExpect(node.body).length > 1) {
+        context.report({
+          node,
+          loc: node.loc.start,
+          messageId: 'noMultipleAssertionWaitFor',
+        });
+      }
+    }
+
+    return {
+      [`${WAIT_EXPRESSION_QUERY} > ArrowFunctionExpression > BlockStatement`]: reportMultipleAssertion,
+      [`${WAIT_EXPRESSION_QUERY} > FunctionExpression > BlockStatement`]: reportMultipleAssertion,
+    };
+  }
+})
diff --git a/tests/lib/rules/no-multiple-assertions-wait-for.test.ts b/tests/lib/rules/no-multiple-assertions-wait-for.test.ts
@@ -0,0 +1,115 @@
+import { createRuleTester } from '../test-utils';
+import rule, { RULE_NAME } from '../../../lib/rules/no-multiple-assertions-wait-for';
+
+const ruleTester = createRuleTester({
+  ecmaFeatures: {
+    jsx: true,
+  },
+});
+
+ruleTester.run(RULE_NAME, rule, {
+  valid: [
+    {
+      code: `
+        await waitFor(() => expect(a).toEqual('a'))
+      `,
+    },
+    {
+      code: `
+        await waitFor(function() {
+          expect(a).toEqual('a')
+        })
+      `,
+    },
+    // this needs to be check by other rule
+    {
+      code: `
+        await waitFor(() => {
+          fireEvent.keyDown(input, {key: 'ArrowDown'})
+          expect(b).toEqual('b')
+        })
+      `,
+    },
+    {
+      code: `
+        await waitFor(function() {
+          fireEvent.keyDown(input, {key: 'ArrowDown'})
+          expect(b).toEqual('b')
+        })
+      `,
+    },
+    {
+      code: `
+        await waitFor(() => {
+          console.log('testing-library')
+          expect(b).toEqual('b')
+        })
+      `,
+    },
+    {
+      code: `
+        await waitFor(function() {
+          console.log('testing-library')
+          expect(b).toEqual('b')
+        })
+      `,
+    },
+    {
+      code: `
+        await waitFor(() => {})
+      `,
+    },
+    {
+      code: `
+        await waitFor(function() {})
+      `,
+    },
+    {
+      code: `
+        await waitFor(() => {
+          // testing
+        })
+      `,
+    },
+  ],
+  invalid: [
+    {
+      code: `
+        await waitFor(() => {
+          expect(a).toEqual('a')
+          expect(b).toEqual('b')
+        })
+      `,
+      errors: [{ messageId: 'noMultipleAssertionWaitFor' }]
+    },
+    {
+      code: `
+        await waitFor(() => {
+          expect(a).toEqual('a')
+          console.log('testing-library')
+          expect(b).toEqual('b')
+        })
+      `,
+      errors: [{ messageId: 'noMultipleAssertionWaitFor' }]
+    },
+    {
+      code: `
+        await waitFor(function() {
+          expect(a).toEqual('a')
+          expect(b).toEqual('b')
+        })
+      `,
+      errors: [{ messageId: 'noMultipleAssertionWaitFor' }]
+    },
+    {
+      code: `
+        await waitFor(function() {
+          expect(a).toEqual('a')
+          console.log('testing-library')
+          expect(b).toEqual('b')
+        })
+      `,
+      errors: [{ messageId: 'noMultipleAssertionWaitFor' }]
+    }
+  ]
+})