diff --git a/README.md b/README.md index 203d34ce..5ab81c5c 100644 --- 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 new file mode 100644 index 00000000..4259187f --- /dev/null +++ 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 index d9ea7a77..4b5149f2 100644 --- 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 new file mode 100644 index 00000000..0b05ade6 --- /dev/null +++ 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)({ + 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): Array => + 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 new file mode 100644 index 00000000..18b061bf --- /dev/null +++ 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' }] + } + ] +})