Skip to content

Commit ea220a8

Browse files
committed
feat: add basic rule for no-get-by-for-absent-elements
1 parent 22636b1 commit ea220a8

File tree

4 files changed

+206
-0
lines changed

4 files changed

+206
-0
lines changed
Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
# Disallow the use of `expect(getBy*)` (prefer-expect-query-by)
2+
3+
The (DOM) Testing Library allows to query DOM elements using different types of queries such as `getBy*` and `queryBy*`. Using `getBy*` throws an error in case the element is not found. This is useful when:
4+
5+
- using method like `waitForElement`, which are `async` functions that will wait for the element to be found until a certain timeout, after that the test will fail.
6+
- using `getBy` queries as an assert itself, so if the element is not found the error thrown will work as the check itself within the test.
7+
8+
However, when trying to assert if an element is not present or disappearance, we can't use `getBy*` as the test will fail immediately. Instead it is recommended to use `queryBy*`, which does not throw and therefore we can assert that e.g. `expect(queryByText("Foo")).not.toBeInTheDocument()`.
9+
10+
> The same applies for the `getAll*` and `queryAll*` queries too.
11+
12+
## Rule details
13+
14+
This rule gives a notification whenever `expect` is used with one of the query functions that throw an error if the element is not found.
15+
16+
Examples of **incorrect** code for this rule:
17+
18+
```js
19+
test('some test', () => {
20+
const { getByText, getAllByText } = render(<App />);
21+
expect(getByText('Foo')).toBeInTheDocument();
22+
expect(getAllByText('Foo')[0]).toBeInTheDocument();
23+
expect(getByText('Foo')).not.toBeInTheDocument();
24+
expect(getAllByText('Foo')[0]).not.toBeInTheDocument();
25+
});
26+
```
27+
28+
```js
29+
test('some test', async () => {
30+
const utils = render(<App />);
31+
await wait(() => {
32+
expect(utils.getByText('Foo')).toBeInTheDocument();
33+
});
34+
await wait(() => {
35+
expect(utils.getAllByText('Foo')).toBeInTheDocument();
36+
});
37+
await wait(() => {
38+
expect(utils.getByText('Foo')).not.toBeInTheDocument();
39+
});
40+
await wait(() => {
41+
expect(utils.getAllByText('Foo')).not.toBeInTheDocument();
42+
});
43+
});
44+
```
45+
46+
Examples of **correct** code for this rule:
47+
48+
```js
49+
test('some test', () => {
50+
const { queryByText, queryAllByText } = render(<App />);
51+
expect(queryByText('Foo')).toBeInTheDocument();
52+
expect(queryAllByText('Foo')[0]).toBeInTheDocument();
53+
expect(queryByText('Foo')).not.toBeInTheDocument();
54+
expect(queryAllByText('Foo')[0]).not.toBeInTheDocument();
55+
});
56+
```
57+
58+
```js
59+
test('some test', async () => {
60+
const utils = render(<App />);
61+
await wait(() => {
62+
expect(utils.queryByText('Foo')).toBeInTheDocument();
63+
});
64+
await wait(() => {
65+
expect(utils.queryAllByText('Foo')).toBeInTheDocument();
66+
});
67+
await wait(() => {
68+
expect(utils.queryByText('Foo')).not.toBeInTheDocument();
69+
});
70+
await wait(() => {
71+
expect(utils.queryAllByText('Foo')).not.toBeInTheDocument();
72+
});
73+
});
74+
```
75+
76+
## Further Reading
77+
78+
- [Asserting elements are not present](https://testing-library.com/docs/guide-disappearance#asserting-elements-are-not-present)
79+
- [Waiting for disappearance](https://testing-library.com/docs/guide-disappearance#waiting-for-disappearance)
80+
- [jest-dom note about using `getBy` within assertions](https://testing-library.com/docs/ecosystem-jest-dom)
81+
- [Testing Library queries cheatsheet](https://testing-library.com/docs/dom-testing-library/cheatsheet#queries)

lib/index.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ const rules = {
77
'no-await-sync-query': require('./rules/no-await-sync-query'),
88
'no-debug': require('./rules/no-debug'),
99
'no-dom-import': require('./rules/no-dom-import'),
10+
'no-get-by-for-absent-elements': require('./rules/no-get-by-for-absent-elements'),
1011
'prefer-expect-query-by': require('./rules/prefer-expect-query-by'),
1112
'prefer-explicit-assert': require('./rules/prefer-explicit-assert'),
1213
};
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
'use strict';
2+
3+
const { getDocsUrl } = require('../utils');
4+
5+
const falsyMatchers = ['toBeNull', 'toBeFalsy', 'toBeUndefined'];
6+
7+
module.exports = {
8+
meta: {
9+
docs: {
10+
category: 'Best Practices',
11+
description:
12+
'Disallow using getBy* queries in expect calls when elements may not be present',
13+
recommended: 'error',
14+
url: getDocsUrl('no-get-by-for-absent-elements'),
15+
},
16+
messages: {
17+
expectQueryBy:
18+
'Use `expect(getBy*)` only when elements are present, otherwise use `expect(queryBy*)`.',
19+
},
20+
schema: [],
21+
type: 'suggestion',
22+
fixable: null,
23+
},
24+
25+
create: context => ({
26+
[`CallExpression > Identifier[name=/getBy|getAllBy/]`](node) {
27+
const expectCallNode = findClosestExpectStatement(node);
28+
29+
if (expectCallNode) {
30+
const expectStatement = expectCallNode.parent;
31+
const matcher = expectStatement.property.name;
32+
33+
if (matcher === 'not') {
34+
const negatedMatcher = expectStatement.parent.property.name;
35+
36+
if (!falsyMatchers.includes(negatedMatcher)) {
37+
return context.report({
38+
node,
39+
messageId: 'expectQueryBy',
40+
});
41+
}
42+
}
43+
44+
if (falsyMatchers.includes(matcher)) {
45+
return context.report({
46+
node,
47+
messageId: 'expectQueryBy',
48+
});
49+
}
50+
}
51+
},
52+
}),
53+
};
54+
55+
function findClosestExpectStatement(node) {
56+
if (!node.parent) {
57+
return false;
58+
}
59+
60+
if (node.type === 'CallExpression' && node.callee.name === 'expect') {
61+
return node;
62+
} else {
63+
return findClosestExpectStatement(node.parent);
64+
}
65+
}
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
'use strict';
2+
3+
const RuleTester = require('eslint').RuleTester;
4+
const rule = require('../../../lib/rules/no-get-by-for-absent-elements');
5+
const { ALL_QUERIES_METHODS } = require('../../../lib/utils');
6+
7+
const ruleTester = new RuleTester({
8+
parserOptions: { ecmaVersion: 2015, sourceType: 'module' },
9+
});
10+
11+
const queryByVariants = ALL_QUERIES_METHODS.reduce(
12+
(variants, method) => [
13+
...variants,
14+
...[`query${method}`, `queryAll${method}`],
15+
],
16+
[]
17+
);
18+
const getByVariants = ALL_QUERIES_METHODS.reduce(
19+
(variants, method) => [...variants, ...[`get${method}`, `getAll${method}`]],
20+
[]
21+
);
22+
23+
ruleTester.run('prefer-expect-query-by', rule, {
24+
valid: queryByVariants.reduce(
25+
(validRules, queryName) => [
26+
...validRules,
27+
{ code: `expect(${queryName}('Hello')).toBeInTheDocument()` },
28+
{ code: `expect(${queryName}('Hello')).toBe("foo")` },
29+
{ code: `expect(${queryName}('Hello')).toBeTruthy()` },
30+
],
31+
[]
32+
),
33+
invalid: getByVariants.reduce(
34+
(invalidRules, queryName) => [
35+
...invalidRules,
36+
{
37+
code: `expect(${queryName}('Hello')).not.toBeInTheDocument()`,
38+
errors: [{ messageId: 'expectQueryBy' }],
39+
},
40+
{
41+
code: `expect(${queryName}('Hello')).toBeNull()`,
42+
errors: [{ messageId: 'expectQueryBy' }],
43+
},
44+
{
45+
code: `expect(${queryName}('Hello')).not.toBeTruthy()`,
46+
errors: [{ messageId: 'expectQueryBy' }],
47+
},
48+
{
49+
code: `expect(${queryName}('Hello')).toBeUndefined()`,
50+
errors: [{ messageId: 'expectQueryBy' }],
51+
},
52+
{
53+
code: `expect(${queryName}('Hello')).toBeFalsy()`,
54+
errors: [{ messageId: 'expectQueryBy' }],
55+
},
56+
],
57+
[]
58+
),
59+
});

0 commit comments

Comments
 (0)