-
Notifications
You must be signed in to change notification settings - Fork 144
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
feat(rule): prefer expect queryBy (#22)
* feat(rule): prefer expect queryBy * chore: regenerate lockfile * refactor: improve docs and tests, include also findBy * refactor: remove occurrences of findBy * refactor: generate test cases for all query methods * refactor: disable autofix implementation * refactor: remove fixable badge * refactor: fixable option to null
- Loading branch information
Showing
7 changed files
with
236 additions
and
8 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| # Disallow the use of `expect(getBy*)` (prefer-expect-query-by) | ||
|
|
||
| The (DOM) Testing Library support three types of queries: `getBy*` and `queryBy*`. Using `getBy*` throws an error in case the element is not found. This is useful when 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. | ||
| However, when trying to assert if an element is not in the document, 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()`. | ||
|
|
||
| > The same applies for the `getAll*` and `queryAll*` queries. | ||
| ## Rule details | ||
|
|
||
| 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. | ||
|
|
||
| This rule is enabled by default. | ||
|
|
||
| Examples of **incorrect** code for this rule: | ||
|
|
||
| ```js | ||
| test('some test', () => { | ||
| const { getByText, getAllByText } = render(<App />); | ||
| expect(getByText('Foo')).toBeInTheDocument(); | ||
| expect(getAllByText('Foo')[0]).toBeInTheDocument(); | ||
| expect(getByText('Foo')).not.toBeInTheDocument(); | ||
| expect(getAllByText('Foo')[0]).not.toBeInTheDocument(); | ||
| }); | ||
| ``` | ||
|
|
||
| ```js | ||
| test('some test', () => { | ||
| const rendered = render(<App />); | ||
| expect(rendered.getByText('Foo')).toBeInTheDocument(); | ||
| expect(rendered.getAllByText('Foo')[0]).toBeInTheDocument(); | ||
| expect(rendered.getByText('Foo')).not.toBeInTheDocument(); | ||
| expect(rendered.getAllByText('Foo')[0]).not.toBeInTheDocument(); | ||
| }); | ||
| ``` | ||
|
|
||
| Examples of **correct** code for this rule: | ||
|
|
||
| ```js | ||
| test('some test', () => { | ||
| const { queryByText, queryAllByText } = render(<App />); | ||
| expect(queryByText('Foo')).toBeInTheDocument(); | ||
| expect(queryAllByText('Foo')[0]).toBeInTheDocument(); | ||
| expect(queryByText('Foo')).not.toBeInTheDocument(); | ||
| expect(queryAllByText('Foo')[0]).not.toBeInTheDocument(); | ||
| }); | ||
| ``` | ||
|
|
||
| ```js | ||
| test('some test', () => { | ||
| const rendered = render(<App />); | ||
| expect(rendered.queryByText('Foo')).toBeInTheDocument(); | ||
| expect(rendered.queryAllByText('Foo')[0]).toBeInTheDocument(); | ||
| expect(rendered.queryByText('Foo')).not.toBeInTheDocument(); | ||
| expect(rendered.queryAllByText('Foo')[0]).not.toBeInTheDocument(); | ||
| }); | ||
| ``` | ||
|
|
||
| ## Further Reading | ||
|
|
||
| - [Appearance and Disappearance](https://testing-library.com/docs/guide-disappearance#asserting-elements-are-not-present) | ||
| - [Testing Library queries cheatsheet](https://testing-library.com/docs/dom-testing-library/cheatsheet#queries) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,102 @@ | ||
| 'use strict'; | ||
|
|
||
| const { getDocsUrl } = require('../utils'); | ||
|
|
||
| const AST_NODE_TYPES = { | ||
| Identifier: 'Identifier', | ||
| MemberExpression: 'MemberExpression', | ||
| }; | ||
|
|
||
| function isIdentifier(node) { | ||
| return node.type === AST_NODE_TYPES.Identifier; | ||
| } | ||
|
|
||
| function isMemberExpression(node) { | ||
| return node.type === AST_NODE_TYPES.MemberExpression; | ||
| } | ||
|
|
||
| function isUsingWrongQueries(node) { | ||
| return node.name.startsWith('getBy') || node.name.startsWith('getAllBy'); | ||
| } | ||
|
|
||
| function isNotNullOrUndefined(input) { | ||
| return input != null; | ||
| } | ||
|
|
||
| function mapNodesForWrongGetByQuery(node) { | ||
| const nodeArguments = node.arguments; | ||
| return nodeArguments | ||
| .map(arg => { | ||
| if (!arg.callee) { | ||
| return null; | ||
| } | ||
| // Example: `expect(rendered.getBy*)` | ||
| if (isMemberExpression(arg.callee)) { | ||
| const node = arg.callee.property; | ||
| if (isIdentifier(node) && isUsingWrongQueries(node)) { | ||
| return node; | ||
| } | ||
| return null; | ||
| } | ||
|
|
||
| // Example: `expect(getBy*)` | ||
| if (isIdentifier(arg.callee) && isUsingWrongQueries(arg.callee)) { | ||
| return arg.callee; | ||
| } | ||
|
|
||
| return null; | ||
| }) | ||
| .filter(isNotNullOrUndefined); | ||
| } | ||
|
|
||
| function hasExpectWithWrongGetByQuery(node) { | ||
| if ( | ||
| node.callee && | ||
| node.callee.type === AST_NODE_TYPES.Identifier && | ||
| node.callee.name === 'expect' && | ||
| node.arguments | ||
| ) { | ||
| const nodesGetBy = mapNodesForWrongGetByQuery(node); | ||
| return nodesGetBy.length > 0; | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| module.exports = { | ||
| meta: { | ||
| docs: { | ||
| category: 'Best Practices', | ||
| description: 'Disallow using getBy* queries in expect calls', | ||
| recommended: 'error', | ||
| url: getDocsUrl('prefer-expect-query-by'), | ||
| }, | ||
| messages: { | ||
| expectQueryBy: | ||
| 'Using `expect(getBy*)` is not recommended, use `expect(queryBy*)` instead.', | ||
| }, | ||
| schema: [], | ||
| type: 'suggestion', | ||
| fixable: null, | ||
| }, | ||
|
|
||
| create: context => ({ | ||
| CallExpression(node) { | ||
| if (hasExpectWithWrongGetByQuery(node)) { | ||
| // const nodesGetBy = mapNodesForWrongGetByQuery(node); | ||
| context.report({ | ||
| node: node.callee, | ||
| messageId: 'expectQueryBy', | ||
| // TODO: we keep the autofixing disabled for now, until we figure out | ||
| // a better way to amend for the edge cases. | ||
| // See also the related discussion: https://github.com/Belco90/eslint-plugin-testing-library/pull/22#discussion_r335394402 | ||
| // fix(fixer) { | ||
| // return fixer.replaceText( | ||
| // nodesGetBy[0], | ||
| // nodesGetBy[0].name.replace(/^(get(All)?(.*))$/, 'query$2$3') | ||
| // ); | ||
| // }, | ||
| }); | ||
| } | ||
| }, | ||
| }), | ||
| }; |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| 'use strict'; | ||
|
|
||
| const RuleTester = require('eslint').RuleTester; | ||
| const rule = require('../../../lib/rules/prefer-expect-query-by'); | ||
| const { ALL_QUERIES_METHODS } = require('../../../lib/utils'); | ||
|
|
||
| const ruleTester = new RuleTester({ | ||
| parserOptions: { ecmaVersion: 2015, sourceType: 'module' }, | ||
| }); | ||
|
|
||
| const queryByVariants = ALL_QUERIES_METHODS.reduce( | ||
| (variants, method) => [ | ||
| ...variants, | ||
| ...[`query${method}`, `queryAll${method}`], | ||
| ], | ||
| [] | ||
| ); | ||
| const getByVariants = ALL_QUERIES_METHODS.reduce( | ||
| (variants, method) => [...variants, ...[`get${method}`, `getAll${method}`]], | ||
| [] | ||
| ); | ||
|
|
||
| ruleTester.run('prefer-expect-query-by', rule, { | ||
| valid: queryByVariants.reduce( | ||
| (validRules, queryName) => [ | ||
| ...validRules, | ||
| { code: `expect(${queryName}('Hello')).toBeInTheDocument()` }, | ||
| { code: `expect(rendered.${queryName}('Hello')).toBeInTheDocument()` }, | ||
| { code: `expect(${queryName}('Hello')).not.toBeInTheDocument()` }, | ||
| { | ||
| code: `expect(rendered.${queryName}('Hello')).not.toBeInTheDocument()`, | ||
| }, | ||
| ], | ||
| [] | ||
| ), | ||
| invalid: getByVariants.reduce( | ||
| (invalidRules, queryName) => [ | ||
| ...invalidRules, | ||
| { | ||
| code: `expect(${queryName}('Hello')).toBeInTheDocument()`, | ||
| errors: [{ messageId: 'expectQueryBy' }], | ||
| }, | ||
| { | ||
| code: `expect(rendered.${queryName}('Hello')).toBeInTheDocument()`, | ||
| errors: [{ messageId: 'expectQueryBy' }], | ||
| }, | ||
| { | ||
| code: `expect(${queryName}('Hello')).not.toBeInTheDocument()`, | ||
| errors: [{ messageId: 'expectQueryBy' }], | ||
| }, | ||
| { | ||
| code: `expect(rendered.${queryName}('Hello')).not.toBeInTheDocument()`, | ||
| errors: [{ messageId: 'expectQueryBy' }], | ||
| }, | ||
| ], | ||
| [] | ||
| ), | ||
| }); |