Skip to content

Commit 7da2b75

Browse files
Belco90Belco90
authored
feat: add prefer-explicit-assert rule (#29)
* feat: add no-get-by-assert rule * refactor: rename no-get-by-assert rule to prefer-explicit-assert * docs(prefer-explicit-assert): add references to prefer-expect-query-by * feat(prefer-explicit-assert): add option for custom queries * feat(prefer-explicit-assert): improve error message * refactor(prefer-explicit-assert): rename error message id * fix(prefer-explicit-assert): rename option argument * fix(prefer-explicit-assert): check queries when not destructured * docs(prefer-explicit-assert): add non-destructured queries examples
1 parent 22e88f5 commit 7da2b75

File tree

5 files changed

+282
-8
lines changed

5 files changed

+282
-8
lines changed

‎README.md

+9-8
Original file line numberDiff line numberDiff line change
@@ -129,14 +129,15 @@ To enable this configuration use the `extends` property in your
129129

130130
## Supported Rules
131131

132-
| Rule | Description | Configurations | Fixable |
133-
| -------------------------------------------------------------- | ---------------------------------------------- | ------------------------------------------------------------------------- | ------------------ |
134-
| [await-async-query](docs/rules/await-async-query.md) | Enforce async queries to have proper `await` | ![recommended-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | |
135-
| [await-fire-event](docs/rules/await-fire-event.md) | Enforce async fire event methods to be awaited | ![vue-badge][] | |
136-
| [no-await-sync-query](docs/rules/no-await-sync-query.md) | Disallow unnecessary `await` for sync queries | ![recommended-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | |
137-
| [no-debug](docs/rules/no-debug.md) | Disallow the use of `debug` | ![angular-badge][] ![react-badge][] ![vue-badge][] | |
138-
| [no-dom-import](docs/rules/no-dom-import.md) | Disallow importing from DOM Testing Library | ![angular-badge][] ![react-badge][] ![vue-badge][] | ![fixable-badge][] |
139-
| [prefer-expect-query-by](docs/rules/prefer-expect-query-by.md) | Disallow the use of `expect(getBy*)` | ![recommended-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | |
132+
| Rule | Description | Configurations | Fixable |
133+
| -------------------------------------------------------------- | ------------------------------------------------------------------- | ------------------------------------------------------------------------- | ------------------ |
134+
| [await-async-query](docs/rules/await-async-query.md) | Enforce async queries to have proper `await` | ![recommended-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | |
135+
| [await-fire-event](docs/rules/await-fire-event.md) | Enforce async fire event methods to be awaited | ![vue-badge][] | |
136+
| [no-await-sync-query](docs/rules/no-await-sync-query.md) | Disallow unnecessary `await` for sync queries | ![recommended-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | |
137+
| [no-debug](docs/rules/no-debug.md) | Disallow the use of `debug` | ![angular-badge][] ![react-badge][] ![vue-badge][] | |
138+
| [no-dom-import](docs/rules/no-dom-import.md) | Disallow importing from DOM Testing Library | ![angular-badge][] ![react-badge][] ![vue-badge][] | ![fixable-badge][] |
139+
| [prefer-expect-query-by](docs/rules/prefer-expect-query-by.md) | Disallow the use of `expect(getBy*)` | ![recommended-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | |
140+
| [prefer-explicit-assert](docs/rules/prefer-explicit-assert.md) | Suggest using explicit assertions rather than just `getBy*` queries | | |
140141

141142
[build-badge]: https://img.shields.io/travis/Belco90/eslint-plugin-testing-library?style=flat-square
142143
[build-url]: https://travis-ci.org/belco90/eslint-plugin-testing-library

‎docs/rules/prefer-explicit-assert.md

+83
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
# Suggest using explicit assertions rather than just `getBy*` queries (prefer-explicit-assert)
2+
3+
Testing Library `getBy*` queries throw an error if the element is not
4+
found. Some users like this behavior to use the query itself as an
5+
assert for the element existence in their tests, but other users don't
6+
and prefer to explicitly assert the element existence, so this rule is
7+
for users from the latter.
8+
9+
## Rule Details
10+
11+
This rule aims to encourage users to explicitly assert existence of
12+
elements in their tests rather than just use `getBy*` queries and expect
13+
it doesn't throw an error so it's easier to understand what's the
14+
expected behavior within the test.
15+
16+
> ⚠️ Please note that this rule is recommended to be used together with
17+
> [prefer-expect-query-by](docs/rules/prefer-expect-query-by.md), so the
18+
> combination of these two rules will force users to do 2 actual
19+
> changes: wrap `getBy*` with `expect` assertion and then use `queryBy*`
20+
> instead of `getBy*` for asserting.
21+
22+
Examples of **incorrect** code for this rule:
23+
24+
```js
25+
// just calling `getBy*` query expecting not to throw an error as an
26+
// assert-like method, without actually either using the returned element
27+
// or explicitly asserting
28+
getByText('foo');
29+
30+
const utils = render(<Component />);
31+
utils.getByText('foo');
32+
```
33+
34+
Examples of **correct** code for this rule:
35+
36+
```js
37+
// wrapping the get query within a `expect` and use some matcher for
38+
// making the assertion more explicit
39+
expect(getByText('foo')).toBeDefined();
40+
41+
const utils = render(<Component />);
42+
expect(utils.getByText('foo')).toBeDefined();
43+
44+
// ⚠️ `getBy*` should be replaced by `queryBy*` when combined with `prefer-expect-query-by` rule
45+
expect(queryByText('foo')).toBeDefined();
46+
47+
// even more explicit if you use `@testing-library/jest-dom` matcher
48+
// for checking the element is present in the document
49+
expect(queryByText('foo')).toBeInTheDocument();
50+
51+
// Doing something with the element returned without asserting is absolutely fine
52+
await waitForElement(() => getByText('foo'));
53+
fireEvent.click(getByText('bar'));
54+
const quxElement = getByText('qux');
55+
56+
// call directly something different than Testing Library query
57+
getByNonTestingLibraryVariant('foo');
58+
```
59+
60+
## Options
61+
62+
This rule accepts a single options argument:
63+
64+
- `customQueryNames`: this array option allows to extend default Testing
65+
Library queries with custom ones for including them into rule
66+
inspection.
67+
68+
```js
69+
"testing-library/prefer-expect-query-by": ["error", {"customQueryNames": ["getByIcon", "getBySomethingElse"]}],
70+
```
71+
72+
## When Not To Use It
73+
74+
If you prefer to use `getBy*` queries implicitly as an assert-like
75+
method itself, then this rule is not recommended.
76+
77+
## Related Rules
78+
79+
- [prefer-expect-query-by](docs/rules/prefer-expect-query-by.md)
80+
81+
## Further Reading
82+
83+
- [getBy query](https://testing-library.com/docs/dom-testing-library/api-queries#getby)

‎lib/index.js

+1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ const rules = {
77
'no-debug': require('./rules/no-debug'),
88
'no-dom-import': require('./rules/no-dom-import'),
99
'prefer-expect-query-by': require('./rules/prefer-expect-query-by'),
10+
'prefer-explicit-assert': require('./rules/prefer-explicit-assert'),
1011
};
1112

1213
const recommendedRules = {

‎lib/rules/prefer-explicit-assert.js

+79
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
'use strict';
2+
3+
const { getDocsUrl, ALL_QUERIES_METHODS } = require('../utils');
4+
5+
const ALL_GET_BY_QUERIES = ALL_QUERIES_METHODS.map(
6+
queryMethod => `get${queryMethod}`
7+
);
8+
9+
const findCallExpressionParent = node =>
10+
node.type === 'CallExpression' ? node : findCallExpressionParent(node.parent);
11+
12+
const isValidQuery = (node, customQueryNames = []) =>
13+
ALL_GET_BY_QUERIES.includes(node.name) ||
14+
customQueryNames.includes(node.name);
15+
16+
const isDirectlyCalledByFunction = node =>
17+
node.parent.type === 'CallExpression';
18+
19+
const isReturnedByArrowFunctionExpression = node =>
20+
node.parent.type === 'ArrowFunctionExpression';
21+
22+
const isDeclared = node => node.parent.type === 'VariableDeclarator';
23+
24+
const isReturnedByReturnStatement = node =>
25+
node.parent.type === 'ReturnStatement';
26+
27+
module.exports = {
28+
meta: {
29+
type: 'suggestion',
30+
docs: {
31+
description:
32+
'Suggest using explicit assertions rather than just `getBy*` queries',
33+
category: 'Best Practices',
34+
recommended: false,
35+
url: getDocsUrl('prefer-explicit-assert'),
36+
},
37+
messages: {
38+
preferExplicitAssert:
39+
'Wrap stand-alone `getBy*` query with `expect` function for better explicit assertion',
40+
},
41+
fixable: null,
42+
schema: [
43+
{
44+
type: 'object',
45+
properties: {
46+
customQueryNames: {
47+
type: 'array',
48+
},
49+
},
50+
},
51+
],
52+
},
53+
54+
create: function(context) {
55+
return {
56+
'CallExpression Identifier'(node) {
57+
const callExpressionNode = findCallExpressionParent(node);
58+
59+
let customQueryNames;
60+
if (context.options && context.options.length > 0) {
61+
[{ customQueryNames }] = context.options;
62+
}
63+
64+
if (
65+
isValidQuery(node, customQueryNames) &&
66+
!isDirectlyCalledByFunction(callExpressionNode) &&
67+
!isReturnedByArrowFunctionExpression(callExpressionNode) &&
68+
!isDeclared(callExpressionNode) &&
69+
!isReturnedByReturnStatement(callExpressionNode)
70+
) {
71+
context.report({
72+
node,
73+
messageId: 'preferExplicitAssert',
74+
});
75+
}
76+
},
77+
};
78+
},
79+
};

‎tests/lib/rules/prefer-explicit-assert.js

+110
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
'use strict';
2+
3+
const rule = require('../../../lib/rules/prefer-explicit-assert');
4+
const { ALL_QUERIES_METHODS } = require('../../../lib/utils');
5+
const RuleTester = require('eslint').RuleTester;
6+
7+
// ------------------------------------------------------------------------------
8+
// Tests
9+
// ------------------------------------------------------------------------------
10+
11+
const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 2018 } });
12+
ruleTester.run('prefer-explicit-assert', rule, {
13+
valid: [
14+
{
15+
code: `getByText`,
16+
},
17+
{
18+
code: `const utils = render()
19+
20+
utils.getByText
21+
`,
22+
},
23+
{
24+
code: `expect(getByText('foo')).toBeDefined()`,
25+
},
26+
{
27+
code: `const utils = render()
28+
29+
expect(utils.getByText('foo')).toBeDefined()
30+
`,
31+
},
32+
{
33+
code: `expect(getByText('foo')).toBeInTheDocument();`,
34+
},
35+
{
36+
code: `async () => { await waitForElement(() => getByText('foo')) }`,
37+
},
38+
{
39+
code: `fireEvent.click(getByText('bar'));`,
40+
},
41+
{
42+
code: `const quxElement = getByText('qux')`,
43+
},
44+
{
45+
code: `() => { return getByText('foo') }`,
46+
},
47+
{
48+
code: `function bar() { return getByText('foo') }`,
49+
},
50+
{
51+
code: `getByIcon('foo')`, // custom `getBy` query not extended through options
52+
},
53+
],
54+
55+
invalid: [
56+
...ALL_QUERIES_METHODS.map(queryMethod => ({
57+
code: `get${queryMethod}('foo')`,
58+
errors: [
59+
{
60+
messageId: 'preferExplicitAssert',
61+
},
62+
],
63+
})),
64+
...ALL_QUERIES_METHODS.map(queryMethod => ({
65+
code: `const utils = render()
66+
67+
utils.get${queryMethod}('foo')`,
68+
errors: [
69+
{
70+
messageId: 'preferExplicitAssert',
71+
line: 3,
72+
column: 13,
73+
},
74+
],
75+
})),
76+
...ALL_QUERIES_METHODS.map(queryMethod => ({
77+
code: `() => {
78+
get${queryMethod}('foo')
79+
doSomething()
80+
81+
get${queryMethod}('bar')
82+
const quxElement = get${queryMethod}('qux')
83+
}
84+
`,
85+
errors: [
86+
{
87+
messageId: 'preferExplicitAssert',
88+
line: 2,
89+
},
90+
{
91+
messageId: 'preferExplicitAssert',
92+
line: 5,
93+
},
94+
],
95+
})),
96+
{
97+
code: `getByIcon('foo')`, // custom `getBy` query extended through options
98+
options: [
99+
{
100+
customQueryNames: ['getByIcon'],
101+
},
102+
],
103+
errors: [
104+
{
105+
messageId: 'preferExplicitAssert',
106+
},
107+
],
108+
},
109+
],
110+
});

0 commit comments

Comments
 (0)