Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 22 additions & 12 deletions docs/rules/no-wait-for-multiple-assertions.md
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});
};
```
Expand All @@ -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');
});
};
```
Expand Down
49 changes: 43 additions & 6 deletions lib/rules/no-wait-for-multiple-assertions.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand Down Expand Up @@ -44,6 +49,24 @@ export default createTestingLibraryRule<Options, MessageIds>({
}) as Array<TSESTree.ExpressionStatement>;
}

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;
Expand All @@ -62,14 +85,28 @@ export default createTestingLibraryRule<Options, MessageIds>({

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',
});
}
Expand Down
75 changes: 64 additions & 11 deletions tests/lib/rules/no-wait-for-multiple-assertions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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')
})
`,
},
{
Expand Down Expand Up @@ -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: [
Expand All @@ -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: [
Expand All @@ -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: [
Expand All @@ -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: [
Expand All @@ -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')
})
})
`,
Expand All @@ -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: [
Expand All @@ -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: [
Expand All @@ -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: [
Expand All @@ -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' },
Expand Down