Skip to content

Commit db8756c

Browse files
y-hsgwy-hsgw
authored
feat(no-wait-for-multiple-assertions): avoid reporting unrelated assertions (#1097)
Closes #1084
1 parent 95b88d5 commit db8756c

File tree

3 files changed

+129
-29
lines changed

3 files changed

+129
-29
lines changed

‎docs/rules/no-wait-for-multiple-assertions.md‎

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,23 +7,27 @@
77
## Rule Details
88

99
This rule aims to ensure the correct usage of `expect` inside `waitFor`, in the way that they're intended to be used.
10-
When using multiple assertions inside `waitFor`, if one fails, you have to wait for a timeout before seeing it failing.
11-
Putting one assertion, you can both wait for the UI to settle to the state you want to assert on,
12-
and also fail faster if one of the assertions do end up failing
10+
11+
If you use multiple assertions against the **same asynchronous target** inside `waitFor`,
12+
you may have to wait for a timeout before seeing a test failure, which is inefficient.
13+
Therefore, you should avoid using multiple assertions on the same async target inside a single `waitFor` callback.
14+
15+
However, multiple assertions against **different async targets** (for example, independent state updates or different function calls) are allowed.
16+
This avoids unnecessary verbosity and maintains readability, without increasing the risk of missing failures.
1317

1418
Example of **incorrect** code for this rule:
1519

1620
```js
1721
const foo = async () => {
1822
await waitFor(() => {
19-
expect(a).toEqual('a');
20-
expect(b).toEqual('b');
23+
expect(window.fetch).toHaveBeenCalledTimes(1);
24+
expect(window.fetch).toHaveBeenCalledWith('/foo');
2125
});
2226

2327
// or
2428
await waitFor(function () {
25-
expect(a).toEqual('a');
26-
expect(b).toEqual('b');
29+
expect(window.fetch).toHaveBeenCalledTimes(1);
30+
expect(window.fetch).toHaveBeenCalledWith('/foo');
2731
});
2832
};
2933
```
@@ -32,20 +36,26 @@ Examples of **correct** code for this rule:
3236

3337
```js
3438
const foo = async () => {
35-
await waitFor(() => expect(a).toEqual('a'));
36-
expect(b).toEqual('b');
39+
await waitFor(() => expect(window.fetch).toHaveBeenCalledTimes(1);
40+
expect(window.fetch).toHaveBeenCalledWith('/foo');
3741

3842
// or
3943
await waitFor(function () {
40-
expect(a).toEqual('a');
44+
expect(window.fetch).toHaveBeenCalledTimes(1);
4145
});
42-
expect(b).toEqual('b');
46+
expect(window.fetch).toHaveBeenCalledWith('/foo');
4347

4448
// it only detects expect
4549
// so this case doesn't generate warnings
4650
await waitFor(() => {
4751
fireEvent.keyDown(input, { key: 'ArrowDown' });
48-
expect(b).toEqual('b');
52+
expect(window.fetch).toHaveBeenCalledTimes(1);
53+
});
54+
55+
// different async targets so the rule does not report it
56+
await waitFor(() => {
57+
expect(window.fetch).toHaveBeenCalledWith('/foo');
58+
expect(localStorage.setItem).toHaveBeenCalledWith('bar', 'baz');
4959
});
5060
};
5161
```

‎lib/rules/no-wait-for-multiple-assertions.ts‎

Lines changed: 43 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
import { createTestingLibraryRule } from '../create-testing-library-rule';
2-
import { getPropertyIdentifierNode } from '../node-utils';
2+
import {
3+
getPropertyIdentifierNode,
4+
isCallExpression,
5+
isMemberExpression,
6+
} from '../node-utils';
7+
import { getSourceCode } from '../utils';
38

49
import type { TSESTree } from '@typescript-eslint/utils';
510

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

52+
function getExpectArgument(expression: TSESTree.Expression) {
53+
if (!isCallExpression(expression)) {
54+
return null;
55+
}
56+
57+
const { callee } = expression;
58+
if (!isMemberExpression(callee)) {
59+
return null;
60+
}
61+
62+
const { object } = callee;
63+
if (!isCallExpression(object) || object.arguments.length === 0) {
64+
return null;
65+
}
66+
67+
return object.arguments[0];
68+
}
69+
4770
function reportMultipleAssertion(node: TSESTree.BlockStatement) {
4871
if (!node.parent) {
4972
return;
@@ -62,14 +85,28 @@ export default createTestingLibraryRule<Options, MessageIds>({
6285

6386
const expectNodes = getExpectNodes(node.body);
6487

65-
if (expectNodes.length <= 1) {
66-
return;
88+
const expectArgumentMap = new Map<
89+
string,
90+
TSESTree.ExpressionStatement[]
91+
>();
92+
93+
for (const expectNode of expectNodes) {
94+
const argument = getExpectArgument(expectNode.expression);
95+
if (!argument) {
96+
continue;
97+
}
98+
99+
const argumentText = getSourceCode(context).getText(argument);
100+
const existingNodes = expectArgumentMap.get(argumentText) ?? [];
101+
const newTargetNodes = [...existingNodes, expectNode];
102+
expectArgumentMap.set(argumentText, newTargetNodes);
67103
}
68104

69-
for (let i = 0; i < expectNodes.length; i++) {
70-
if (i !== 0) {
105+
for (const expressionStatements of expectArgumentMap.values()) {
106+
// Skip the first matched assertion; only report subsequent duplicates.
107+
for (const expressionStatement of expressionStatements.slice(1)) {
71108
context.report({
72-
node: expectNodes[i],
109+
node: expressionStatement,
73110
messageId: 'noWaitForMultipleAssertion',
74111
});
75112
}

‎tests/lib/rules/no-wait-for-multiple-assertions.test.ts‎

Lines changed: 64 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,22 @@ ruleTester.run(RULE_NAME, rule, {
3434
await waitFor(function() {
3535
expect(a).toEqual('a')
3636
})
37+
`,
38+
},
39+
{
40+
code: `
41+
await waitFor(function() {
42+
expect(a).toEqual('a')
43+
expect(b).toEqual('b')
44+
})
45+
`,
46+
},
47+
{
48+
code: `
49+
await waitFor(function() {
50+
expect(a).toEqual('a')
51+
expect('a').toEqual('a')
52+
})
3753
`,
3854
},
3955
{
@@ -115,7 +131,18 @@ ruleTester.run(RULE_NAME, rule, {
115131
code: `
116132
await waitFor(() => {
117133
expect(a).toEqual('a')
118-
expect(b).toEqual('b')
134+
expect(a).toEqual('a')
135+
})
136+
`,
137+
errors: [
138+
{ line: 4, column: 11, messageId: 'noWaitForMultipleAssertion' },
139+
],
140+
},
141+
{
142+
code: `
143+
await waitFor(() => {
144+
expect(screen.getByTestId('a')).toHaveTextContent('a')
145+
expect(screen.getByTestId('a')).toHaveTextContent('a')
119146
})
120147
`,
121148
errors: [
@@ -129,7 +156,7 @@ ruleTester.run(RULE_NAME, rule, {
129156
import { waitFor } from '${testingFramework}'
130157
await waitFor(() => {
131158
expect(a).toEqual('a')
132-
expect(b).toEqual('b')
159+
expect(a).toEqual('a')
133160
})
134161
`,
135162
errors: [
@@ -143,7 +170,7 @@ ruleTester.run(RULE_NAME, rule, {
143170
import { waitFor as renamedWaitFor } from 'test-utils'
144171
await renamedWaitFor(() => {
145172
expect(a).toEqual('a')
146-
expect(b).toEqual('b')
173+
expect(a).toEqual('a')
147174
})
148175
`,
149176
errors: [
@@ -155,7 +182,7 @@ ruleTester.run(RULE_NAME, rule, {
155182
await waitFor(() => {
156183
expect(a).toEqual('a')
157184
console.log('testing-library')
158-
expect(b).toEqual('b')
185+
expect(a).toEqual('a')
159186
})
160187
`,
161188
errors: [
@@ -168,7 +195,7 @@ ruleTester.run(RULE_NAME, rule, {
168195
await waitFor(() => {
169196
expect(a).toEqual('a')
170197
console.log('testing-library')
171-
expect(b).toEqual('b')
198+
expect(a).toEqual('a')
172199
})
173200
})
174201
`,
@@ -181,7 +208,7 @@ ruleTester.run(RULE_NAME, rule, {
181208
await waitFor(async () => {
182209
expect(a).toEqual('a')
183210
await somethingAsync()
184-
expect(b).toEqual('b')
211+
expect(a).toEqual('a')
185212
})
186213
`,
187214
errors: [
@@ -192,9 +219,9 @@ ruleTester.run(RULE_NAME, rule, {
192219
code: `
193220
await waitFor(function() {
194221
expect(a).toEqual('a')
195-
expect(b).toEqual('b')
196-
expect(c).toEqual('c')
197-
expect(d).toEqual('d')
222+
expect(a).toEqual('a')
223+
expect(a).toEqual('a')
224+
expect(a).toEqual('a')
198225
})
199226
`,
200227
errors: [
@@ -207,8 +234,22 @@ ruleTester.run(RULE_NAME, rule, {
207234
code: `
208235
await waitFor(function() {
209236
expect(a).toEqual('a')
210-
console.log('testing-library')
237+
expect(a).toEqual('a')
211238
expect(b).toEqual('b')
239+
expect(b).toEqual('b')
240+
})
241+
`,
242+
errors: [
243+
{ line: 4, column: 11, messageId: 'noWaitForMultipleAssertion' },
244+
{ line: 6, column: 11, messageId: 'noWaitForMultipleAssertion' },
245+
],
246+
},
247+
{
248+
code: `
249+
await waitFor(function() {
250+
expect(a).toEqual('a')
251+
console.log('testing-library')
252+
expect(a).toEqual('a')
212253
})
213254
`,
214255
errors: [
@@ -220,8 +261,20 @@ ruleTester.run(RULE_NAME, rule, {
220261
await waitFor(async function() {
221262
expect(a).toEqual('a')
222263
const el = await somethingAsync()
223-
expect(b).toEqual('b')
264+
expect(a).toEqual('a')
224265
})
266+
`,
267+
errors: [
268+
{ line: 5, column: 11, messageId: 'noWaitForMultipleAssertion' },
269+
],
270+
},
271+
{
272+
code: `
273+
await waitFor(() => {
274+
expect(window.fetch).toHaveBeenCalledTimes(1);
275+
expect(localStorage.setItem).toHaveBeenCalledWith('bar', 'baz');
276+
expect(window.fetch).toHaveBeenCalledWith('/foo');
277+
});
225278
`,
226279
errors: [
227280
{ line: 5, column: 11, messageId: 'noWaitForMultipleAssertion' },

0 commit comments

Comments
 (0)