diff --git a/docs/rules/no-wait-for-multiple-assertions.md b/docs/rules/no-wait-for-multiple-assertions.md index 83a3bd51..fae909ac 100644 --- a/docs/rules/no-wait-for-multiple-assertions.md +++ b/docs/rules/no-wait-for-multiple-assertions.md @@ -7,23 +7,27 @@ ## 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 multiple 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 + +If you use multiple assertions against the **same asynchronous target** inside `waitFor`, +you may have to wait for a timeout before seeing a test failure, which is inefficient. +Therefore, you should avoid using multiple assertions on the same async target inside a single `waitFor` callback. + +However, multiple assertions against **different async targets** (for example, independent state updates or different function calls) are allowed. +This avoids unnecessary verbosity and maintains readability, without increasing the risk of missing failures. Example of **incorrect** code for this rule: ```js const foo = async () => { await waitFor(() => { - expect(a).toEqual('a'); - expect(b).toEqual('b'); + expect(window.fetch).toHaveBeenCalledTimes(1); + expect(window.fetch).toHaveBeenCalledWith('/foo'); }); // or await waitFor(function () { - expect(a).toEqual('a'); - expect(b).toEqual('b'); + expect(window.fetch).toHaveBeenCalledTimes(1); + expect(window.fetch).toHaveBeenCalledWith('/foo'); }); }; ``` @@ -32,20 +36,26 @@ Examples of **correct** code for this rule: ```js const foo = async () => { - await waitFor(() => expect(a).toEqual('a')); - expect(b).toEqual('b'); + await waitFor(() => expect(window.fetch).toHaveBeenCalledTimes(1); + expect(window.fetch).toHaveBeenCalledWith('/foo'); // or await waitFor(function () { - expect(a).toEqual('a'); + expect(window.fetch).toHaveBeenCalledTimes(1); }); - expect(b).toEqual('b'); + expect(window.fetch).toHaveBeenCalledWith('/foo'); // it only detects expect // so this case doesn't generate warnings await waitFor(() => { fireEvent.keyDown(input, { key: 'ArrowDown' }); - expect(b).toEqual('b'); + expect(window.fetch).toHaveBeenCalledTimes(1); + }); + + // different async targets so the rule does not report it + await waitFor(() => { + expect(window.fetch).toHaveBeenCalledWith('/foo'); + expect(localStorage.setItem).toHaveBeenCalledWith('bar', 'baz'); }); }; ``` diff --git a/lib/rules/no-wait-for-multiple-assertions.ts b/lib/rules/no-wait-for-multiple-assertions.ts index 245bd9d1..bf6a1fc3 100644 --- a/lib/rules/no-wait-for-multiple-assertions.ts +++ b/lib/rules/no-wait-for-multiple-assertions.ts @@ -1,5 +1,10 @@ import { createTestingLibraryRule } from '../create-testing-library-rule'; -import { getPropertyIdentifierNode } from '../node-utils'; +import { + getPropertyIdentifierNode, + isCallExpression, + isMemberExpression, +} from '../node-utils'; +import { getSourceCode } from '../utils'; import type { TSESTree } from '@typescript-eslint/utils'; @@ -44,6 +49,24 @@ export default createTestingLibraryRule({ }) as Array; } + function getExpectArgument(expression: TSESTree.Expression) { + if (!isCallExpression(expression)) { + return null; + } + + const { callee } = expression; + if (!isMemberExpression(callee)) { + return null; + } + + const { object } = callee; + if (!isCallExpression(object) || object.arguments.length === 0) { + return null; + } + + return object.arguments[0]; + } + function reportMultipleAssertion(node: TSESTree.BlockStatement) { if (!node.parent) { return; @@ -62,14 +85,28 @@ export default createTestingLibraryRule({ const expectNodes = getExpectNodes(node.body); - if (expectNodes.length <= 1) { - return; + const expectArgumentMap = new Map< + string, + TSESTree.ExpressionStatement[] + >(); + + for (const expectNode of expectNodes) { + const argument = getExpectArgument(expectNode.expression); + if (!argument) { + continue; + } + + const argumentText = getSourceCode(context).getText(argument); + const existingNodes = expectArgumentMap.get(argumentText) ?? []; + const newTargetNodes = [...existingNodes, expectNode]; + expectArgumentMap.set(argumentText, newTargetNodes); } - for (let i = 0; i < expectNodes.length; i++) { - if (i !== 0) { + for (const expressionStatements of expectArgumentMap.values()) { + // Skip the first matched assertion; only report subsequent duplicates. + for (const expressionStatement of expressionStatements.slice(1)) { context.report({ - node: expectNodes[i], + node: expressionStatement, messageId: 'noWaitForMultipleAssertion', }); } diff --git a/tests/lib/rules/no-wait-for-multiple-assertions.test.ts b/tests/lib/rules/no-wait-for-multiple-assertions.test.ts index 0bd4e610..f316acf2 100644 --- a/tests/lib/rules/no-wait-for-multiple-assertions.test.ts +++ b/tests/lib/rules/no-wait-for-multiple-assertions.test.ts @@ -34,6 +34,22 @@ ruleTester.run(RULE_NAME, rule, { await waitFor(function() { expect(a).toEqual('a') }) + `, + }, + { + code: ` + await waitFor(function() { + expect(a).toEqual('a') + expect(b).toEqual('b') + }) + `, + }, + { + code: ` + await waitFor(function() { + expect(a).toEqual('a') + expect('a').toEqual('a') + }) `, }, { @@ -115,7 +131,18 @@ ruleTester.run(RULE_NAME, rule, { code: ` await waitFor(() => { expect(a).toEqual('a') - expect(b).toEqual('b') + expect(a).toEqual('a') + }) + `, + errors: [ + { line: 4, column: 11, messageId: 'noWaitForMultipleAssertion' }, + ], + }, + { + code: ` + await waitFor(() => { + expect(screen.getByTestId('a')).toHaveTextContent('a') + expect(screen.getByTestId('a')).toHaveTextContent('a') }) `, errors: [ @@ -129,7 +156,7 @@ ruleTester.run(RULE_NAME, rule, { import { waitFor } from '${testingFramework}' await waitFor(() => { expect(a).toEqual('a') - expect(b).toEqual('b') + expect(a).toEqual('a') }) `, errors: [ @@ -143,7 +170,7 @@ ruleTester.run(RULE_NAME, rule, { import { waitFor as renamedWaitFor } from 'test-utils' await renamedWaitFor(() => { expect(a).toEqual('a') - expect(b).toEqual('b') + expect(a).toEqual('a') }) `, errors: [ @@ -155,7 +182,7 @@ ruleTester.run(RULE_NAME, rule, { await waitFor(() => { expect(a).toEqual('a') console.log('testing-library') - expect(b).toEqual('b') + expect(a).toEqual('a') }) `, errors: [ @@ -168,7 +195,7 @@ ruleTester.run(RULE_NAME, rule, { await waitFor(() => { expect(a).toEqual('a') console.log('testing-library') - expect(b).toEqual('b') + expect(a).toEqual('a') }) }) `, @@ -181,7 +208,7 @@ ruleTester.run(RULE_NAME, rule, { await waitFor(async () => { expect(a).toEqual('a') await somethingAsync() - expect(b).toEqual('b') + expect(a).toEqual('a') }) `, errors: [ @@ -192,9 +219,9 @@ ruleTester.run(RULE_NAME, rule, { code: ` await waitFor(function() { expect(a).toEqual('a') - expect(b).toEqual('b') - expect(c).toEqual('c') - expect(d).toEqual('d') + expect(a).toEqual('a') + expect(a).toEqual('a') + expect(a).toEqual('a') }) `, errors: [ @@ -207,8 +234,22 @@ ruleTester.run(RULE_NAME, rule, { code: ` await waitFor(function() { expect(a).toEqual('a') - console.log('testing-library') + expect(a).toEqual('a') expect(b).toEqual('b') + expect(b).toEqual('b') + }) + `, + errors: [ + { line: 4, column: 11, messageId: 'noWaitForMultipleAssertion' }, + { line: 6, column: 11, messageId: 'noWaitForMultipleAssertion' }, + ], + }, + { + code: ` + await waitFor(function() { + expect(a).toEqual('a') + console.log('testing-library') + expect(a).toEqual('a') }) `, errors: [ @@ -220,8 +261,20 @@ ruleTester.run(RULE_NAME, rule, { await waitFor(async function() { expect(a).toEqual('a') const el = await somethingAsync() - expect(b).toEqual('b') + expect(a).toEqual('a') }) + `, + errors: [ + { line: 5, column: 11, messageId: 'noWaitForMultipleAssertion' }, + ], + }, + { + code: ` + await waitFor(() => { + expect(window.fetch).toHaveBeenCalledTimes(1); + expect(localStorage.setItem).toHaveBeenCalledWith('bar', 'baz'); + expect(window.fetch).toHaveBeenCalledWith('/foo'); + }); `, errors: [ { line: 5, column: 11, messageId: 'noWaitForMultipleAssertion' },