Skip to content

Commit 3a0d125

Browse files
committed
Merge remote-tracking branch 'origin/master' into v4
# Conflicts: # .all-contributorsrc # .travis.yml # README.md # docs/rules/prefer-explicit-assert.md # lib/node-utils.ts # lib/rules/await-async-utils.ts # lib/rules/no-debug.ts # lib/rules/no-wait-for-snapshot.ts # lib/rules/prefer-explicit-assert.ts # lib/rules/prefer-screen-queries.ts # lib/utils.ts # package.json # tests/lib/rules/no-wait-for-snapshot.test.ts # tests/lib/rules/prefer-explicit-assert.test.ts # tests/lib/rules/prefer-find-by.test.ts
2 parents 9c2072d + f78720d commit 3a0d125

16 files changed

+220
-53
lines changed

.all-contributorsrc

+2-1
Original file line numberDiff line numberDiff line change
@@ -366,7 +366,8 @@
366366
"profile": "https://michaeldeboey.be",
367367
"contributions": [
368368
"code",
369-
"platform"
369+
"platform",
370+
"maintenance"
370371
]
371372
}
372373
],

README.md

+2-2
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
<!-- ALL-CONTRIBUTORS-BADGE:START - Do not remove or modify this section -->
2626

2727
[![All Contributors](https://img.shields.io/badge/all_contributors-34-orange.svg?style=flat-square)](#contributors-)
28-
2928
<!-- ALL-CONTRIBUTORS-BADGE:END -->
3029

3130
## Installation
@@ -143,6 +142,7 @@ To enable this configuration use the `extends` property in your
143142
| [no-side-effects-wait-for](docs/rules/no-side-effects-wait-for.md) | Disallow the use of side effects inside `waitFor` | | |
144143
| [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][] | |
145144
| [no-wait-for-snapshot](docs/rules/no-wait-for-snapshot.md) | Ensures no snapshot is generated inside of a `waitFor` call | | |
145+
| [no-wait-for-snapshot](docs/rules/no-wait-for-snapshot.md) | Ensures no snapshot is generated inside of a `waitFor` call | | |
146146
| [prefer-explicit-assert](docs/rules/prefer-explicit-assert.md) | Suggest using explicit assertions rather than just `getBy*` queries | | |
147147
| [prefer-find-by](docs/rules/prefer-find-by.md) | Suggest using `findBy*` methods instead of the `waitFor` + `getBy` queries | ![dom-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | ![fixable-badge][] |
148148
| [prefer-presence-queries](docs/rules/prefer-presence-queries.md) | Enforce specific queries when checking element is present or not | | |
@@ -221,7 +221,7 @@ Thanks goes to these wonderful people ([emoji key](https://allcontributors.org/d
221221
<td align="center"><a href="https://skovy.dev"><img src="https://avatars1.githubusercontent.com/u/5247455?v=4" width="100px;" alt=""/><br /><sub><b>Spencer Miskoviak</b></sub></a><br /><a href="https://github.com/testing-library/eslint-plugin-testing-library/commits?author=skovy" title="Code">💻</a> <a href="https://github.com/testing-library/eslint-plugin-testing-library/commits?author=skovy" title="Tests">⚠️</a> <a href="https://github.com/testing-library/eslint-plugin-testing-library/commits?author=skovy" title="Documentation">📖</a> <a href="#ideas-skovy" title="Ideas, Planning, & Feedback">🤔</a></td>
222222
<td align="center"><a href="https://twitter.com/Gpx"><img src="https://avatars0.githubusercontent.com/u/767959?v=4" width="100px;" alt=""/><br /><sub><b>Giorgio Polvara</b></sub></a><br /><a href="https://github.com/testing-library/eslint-plugin-testing-library/commits?author=Gpx" title="Code">💻</a> <a href="https://github.com/testing-library/eslint-plugin-testing-library/commits?author=Gpx" title="Tests">⚠️</a> <a href="https://github.com/testing-library/eslint-plugin-testing-library/commits?author=Gpx" title="Documentation">📖</a></td>
223223
<td align="center"><a href="https://github.com/jdanil"><img src="https://avatars0.githubusercontent.com/u/8342105?v=4" width="100px;" alt=""/><br /><sub><b>Josh David</b></sub></a><br /><a href="https://github.com/testing-library/eslint-plugin-testing-library/commits?author=jdanil" title="Documentation">📖</a></td>
224-
<td align="center"><a href="https://michaeldeboey.be"><img src="https://avatars3.githubusercontent.com/u/6643991?v=4" width="100px;" alt=""/><br /><sub><b>Michaël De Boey</b></sub></a><br /><a href="https://github.com/testing-library/eslint-plugin-testing-library/commits?author=MichaelDeBoey" title="Code">💻</a> <a href="#platform-MichaelDeBoey" title="Packaging/porting to new platform">📦</a></td>
224+
<td align="center"><a href="https://michaeldeboey.be"><img src="https://avatars3.githubusercontent.com/u/6643991?v=4" width="100px;" alt=""/><br /><sub><b>Michaël De Boey</b></sub></a><br /><a href="https://github.com/testing-library/eslint-plugin-testing-library/commits?author=MichaelDeBoey" title="Code">💻</a> <a href="#platform-MichaelDeBoey" title="Packaging/porting to new platform">📦</a> <a href="#maintenance-MichaelDeBoey" title="Maintenance">🚧</a></td>
225225
</tr>
226226
</table>
227227

docs/rules/await-async-utils.md

+6
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,12 @@ test('something correctly', async () => {
5959
// return the promise within a function is correct too!
6060
const makeCustomWait = () =>
6161
waitForElementToBeRemoved(() => document.querySelector('div.getOuttaHere'));
62+
63+
// using Promise.all combining the methods
64+
await Promise.all([
65+
waitFor(() => getByLabelText('email')),
66+
waitForElementToBeRemoved(() => document.querySelector('div.getOuttaHere')),
67+
]);
6268
});
6369
```
6470

docs/rules/prefer-explicit-assert.md

+5-1
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,11 @@ This rule has a few options:
5656
with `getBy*` queries. By default, any assertion is valid (`toBeTruthy`,
5757
`toBeDefined`, etc.). However, they all assert slightly different things.
5858
This option ensures all `getBy*` assertions are consistent and use the same
59-
assertion.
59+
assertion. This rule only allows defining a presence matcher
60+
(`toBeInTheDocument`, `toBeTruthy`, or `toBeDefined`), but checks for both
61+
presence and absence matchers (`not.toBeFalsy` and `not.toBeNull`). This means
62+
other assertions such as `toHaveValue` or `toBeDisabled` will not trigger this
63+
rule since these are valid uses with `getBy*`.
6064

6165
```js
6266
"testing-library/prefer-explicit-assert": ["error", {"assertion": "toBeInTheDocument"}],

jest.config.js

+7
Original file line numberDiff line numberDiff line change
@@ -10,5 +10,12 @@ module.exports = {
1010
lines: 100,
1111
statements: 100,
1212
},
13+
// TODO drop this custom threshold in v4
14+
"./lib/node-utils.ts": {
15+
branches: 90,
16+
functions: 90,
17+
lines: 90,
18+
statements: 90,
19+
}
1320
},
1421
};

lib/node-utils.ts

+12
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,12 @@ export function isImportSpecifier(
3737
return node && node.type === AST_NODE_TYPES.ImportSpecifier;
3838
}
3939

40+
export function isImportNamespaceSpecifier(
41+
node: TSESTree.Node
42+
): node is TSESTree.ImportNamespaceSpecifier {
43+
return node?.type === AST_NODE_TYPES.ImportNamespaceSpecifier
44+
}
45+
4046
export function isImportDefaultSpecifier(
4147
node: TSESTree.Node
4248
): node is TSESTree.ImportDefaultSpecifier {
@@ -136,6 +142,12 @@ export function isReturnStatement(
136142
return node && node.type === AST_NODE_TYPES.ReturnStatement;
137143
}
138144

145+
export function isArrayExpression(
146+
node: TSESTree.Node
147+
): node is TSESTree.ArrayExpression {
148+
return node?.type === AST_NODE_TYPES.ArrayExpression
149+
}
150+
139151
export function isAwaited(node: TSESTree.Node): boolean {
140152
return (
141153
isAwaitExpression(node) ||

lib/rules/await-async-utils.ts

+22-4
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,12 @@ import {
55
isAwaited,
66
isPromiseResolved,
77
getVariableReferences,
8+
isMemberExpression,
9+
isImportSpecifier,
10+
isImportNamespaceSpecifier,
11+
isCallExpression,
12+
isArrayExpression,
13+
isIdentifier
814
} from '../node-utils';
915

1016
export const RULE_NAME = 'await-async-utils';
@@ -13,6 +19,17 @@ type Options = [];
1319

1420
const ASYNC_UTILS_REGEXP = new RegExp(`^(${ASYNC_UTILS.join('|')})$`);
1521

22+
// verifies the CallExpression is Promise.all()
23+
function isPromiseAll(node: TSESTree.CallExpression) {
24+
return isMemberExpression(node.callee) && isIdentifier(node.callee.object) && node.callee.object.name === 'Promise' && isIdentifier(node.callee.property) && node.callee.property.name === 'all'
25+
}
26+
27+
// verifies the node is part of an array used in a CallExpression
28+
function isInPromiseAll(node: TSESTree.Node) {
29+
const parent = node.parent
30+
return isCallExpression(parent) && isArrayExpression(parent.parent) && isCallExpression(parent.parent.parent) && isPromiseAll(parent.parent.parent)
31+
}
32+
1633
export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
1734
name: RULE_NAME,
1835
meta: {
@@ -47,11 +64,11 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
4764
return;
4865
}
4966

50-
if (node.type === 'ImportSpecifier') {
67+
if (isImportSpecifier(node)) {
5168
importedAsyncUtils.push(node.imported.name);
5269
}
5370

54-
if (node.type === 'ImportNamespaceSpecifier') {
71+
if (isImportNamespaceSpecifier(node)) {
5572
importedAsyncUtils.push(node.local.name);
5673
}
5774
},
@@ -74,7 +91,7 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
7491
},
7592
'Program:exit'() {
7693
const testingLibraryUtilUsage = asyncUtilsUsage.filter((usage) => {
77-
if (usage.node.type === 'MemberExpression') {
94+
if (isMemberExpression(usage.node)) {
7895
const object = usage.node.object as TSESTree.Identifier;
7996

8097
return importedAsyncUtils.includes(object.name);
@@ -90,7 +107,8 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
90107
references &&
91108
references.length === 0 &&
92109
!isAwaited(node.parent.parent) &&
93-
!isPromiseResolved(node)
110+
!isPromiseResolved(node) &&
111+
!isInPromiseAll(node)
94112
) {
95113
context.report({
96114
node,

lib/rules/no-wait-for-snapshot.ts

+3-3
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
8181
snapshotUsage.push(node);
8282
},
8383
'Program:exit'() {
84-
const testingLibraryUtilUsage = asyncUtilsUsage.filter((usage) => {
84+
const testingLibraryUtilUsage = asyncUtilsUsage.filter(usage => {
8585
if (isMemberExpression(usage.node)) {
8686
const object = usage.node.object as TSESTree.Identifier;
8787

@@ -109,8 +109,8 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
109109
return null;
110110
}
111111

112-
snapshotUsage.forEach((node) => {
113-
testingLibraryUtilUsage.forEach((asyncUtilUsage) => {
112+
snapshotUsage.forEach(node => {
113+
testingLibraryUtilUsage.forEach(asyncUtilUsage => {
114114
const closestAsyncUtil = getClosestAsyncUtil(asyncUtilUsage, node);
115115
if (closestAsyncUtil != null) {
116116
let name;

lib/rules/prefer-explicit-assert.ts

+31-7
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,15 @@
11
import { ESLintUtils, TSESTree } from '@typescript-eslint/experimental-utils';
2-
import { getDocsUrl, ALL_QUERIES_METHODS } from '../utils';
3-
import { isMemberExpression } from '../node-utils';
2+
import {
3+
getDocsUrl,
4+
ALL_QUERIES_METHODS,
5+
PRESENCE_MATCHERS,
6+
ABSENCE_MATCHERS,
7+
} from '../utils';
8+
import {
9+
findClosestCallNode,
10+
isIdentifier,
11+
isMemberExpression,
12+
} from '../node-utils';
413

514
export const RULE_NAME = 'prefer-explicit-assert';
615
export type MessageIds =
@@ -48,6 +57,7 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
4857
properties: {
4958
assertion: {
5059
type: 'string',
60+
enum: PRESENCE_MATCHERS,
5161
},
5262
customQueryNames: {
5363
type: 'array',
@@ -84,15 +94,29 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
8494
messageId: 'preferExplicitAssert',
8595
});
8696
} else if (assertion) {
87-
const expectation = node.parent.parent.parent;
97+
const expectCallNode = findClosestCallNode(node, 'expect');
98+
99+
const expectStatement = expectCallNode.parent as TSESTree.MemberExpression;
100+
const property = expectStatement.property as TSESTree.Identifier;
101+
let matcher = property.name;
102+
let isNegatedMatcher = false;
88103

89104
if (
90-
expectation.type === 'MemberExpression' &&
91-
expectation.property.type === 'Identifier' &&
92-
expectation.property.name !== assertion
105+
matcher === 'not' &&
106+
isMemberExpression(expectStatement.parent) &&
107+
isIdentifier(expectStatement.parent.property)
93108
) {
109+
isNegatedMatcher = true;
110+
matcher = expectStatement.parent.property.name;
111+
}
112+
113+
const shouldEnforceAssertion =
114+
(!isNegatedMatcher && PRESENCE_MATCHERS.includes(matcher)) ||
115+
(isNegatedMatcher && ABSENCE_MATCHERS.includes(matcher));
116+
117+
if (shouldEnforceAssertion && matcher !== assertion) {
94118
context.report({
95-
node: expectation.property,
119+
node: property,
96120
messageId: 'preferExplicitAssertAssertion',
97121
data: {
98122
assertion,

lib/rules/prefer-presence-queries.ts

+1-3
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { ESLintUtils, TSESTree } from '@typescript-eslint/experimental-utils';
2-
import { getDocsUrl, ALL_QUERIES_METHODS } from '../utils';
2+
import { getDocsUrl, ALL_QUERIES_METHODS, PRESENCE_MATCHERS, ABSENCE_MATCHERS } from '../utils';
33
import {
44
findClosestCallNode,
55
isMemberExpression,
@@ -13,8 +13,6 @@ type Options = [];
1313
const QUERIES_REGEXP = new RegExp(
1414
`^(get|query)(All)?(${ALL_QUERIES_METHODS.join('|')})$`
1515
);
16-
const PRESENCE_MATCHERS = ['toBeInTheDocument', 'toBeTruthy', 'toBeDefined'];
17-
const ABSENCE_MATCHERS = ['toBeNull', 'toBeFalsy'];
1816

1917
function isThrowingQuery(node: TSESTree.Identifier) {
2018
return node.name.startsWith('get');

lib/rules/prefer-screen-queries.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ function usesContainerOrBaseElement(node: TSESTree.CallExpression) {
2424
return (
2525
isObjectExpression(secondArgument) &&
2626
secondArgument.properties.some(
27-
(property) =>
27+
property =>
2828
isProperty(property) &&
2929
isIdentifier(property.key) &&
3030
ALLOWED_RENDER_PROPERTIES_FOR_DESTRUCTURING.includes(property.key.name)

lib/utils.ts

+5
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,9 @@ const ALL_RETURNING_NODES = [
108108
...METHODS_RETURNING_NODES,
109109
];
110110

111+
const PRESENCE_MATCHERS = ['toBeInTheDocument', 'toBeTruthy', 'toBeDefined'];
112+
const ABSENCE_MATCHERS = ['toBeNull', 'toBeFalsy'];
113+
111114
export {
112115
getDocsUrl,
113116
hasTestingLibraryImportModule,
@@ -124,4 +127,6 @@ export {
124127
PROPERTIES_RETURNING_NODES,
125128
METHODS_RETURNING_NODES,
126129
ALL_RETURNING_NODES,
130+
PRESENCE_MATCHERS,
131+
ABSENCE_MATCHERS
127132
};

tests/lib/rules/await-async-utils.test.ts

+55-1
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,50 @@ ruleTester.run(RULE_NAME, rule, {
120120
});
121121
`,
122122
})),
123-
123+
...ASYNC_UTILS.map(asyncUtil => ({
124+
code: `
125+
import { ${asyncUtil} } from '@testing-library/dom';
126+
test('${asyncUtil} util used in with Promise.all() does not trigger an error', async () => {
127+
await Promise.all([
128+
${asyncUtil}(callback1),
129+
${asyncUtil}(callback2),
130+
]);
131+
});
132+
`,
133+
})),
134+
...ASYNC_UTILS.map(asyncUtil => ({
135+
code: `
136+
import { ${asyncUtil} } from '@testing-library/dom';
137+
test('${asyncUtil} util used in with Promise.all() with an await does not trigger an error', async () => {
138+
await Promise.all([
139+
await ${asyncUtil}(callback1),
140+
await ${asyncUtil}(callback2),
141+
]);
142+
});
143+
`,
144+
})),
145+
...ASYNC_UTILS.map(asyncUtil => ({
146+
code: `
147+
import { ${asyncUtil} } from '@testing-library/dom';
148+
test('${asyncUtil} util used in with Promise.all() with ".then" does not trigger an error', async () => {
149+
Promise.all([
150+
${asyncUtil}(callback1),
151+
${asyncUtil}(callback2),
152+
]).then(() => console.log('foo'));
153+
});
154+
`,
155+
})),
156+
{
157+
code: `
158+
import { waitFor, waitForElementToBeRemoved } from '@testing-library/dom';
159+
test('combining different async methods with Promise.all does not throw an error', async () => {
160+
await Promise.all([
161+
waitFor(() => getByLabelText('email')),
162+
waitForElementToBeRemoved(() => document.querySelector('div.getOuttaHere')),
163+
])
164+
});
165+
`
166+
},
124167
{
125168
code: `
126169
import { waitForElementToBeRemoved } from '@testing-library/dom';
@@ -139,6 +182,17 @@ ruleTester.run(RULE_NAME, rule, {
139182
});
140183
`,
141184
},
185+
{
186+
code: `
187+
test('using unrelated promises with Promise.all do not throw an error', async () => {
188+
await Promise.all([
189+
someMethod(),
190+
promise1,
191+
await foo().then(() => baz())
192+
])
193+
})
194+
`
195+
}
142196
],
143197
invalid: [
144198
...ASYNC_UTILS.map((asyncUtil) => ({

0 commit comments

Comments
 (0)