feat: add prefer-explicit-assert rule #29
There are no files selected for viewing
This file contains hidden or 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 hidden or 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,77 @@ | ||
| # Suggest using explicit assertions rather than just `getBy*` queries (prefer-explicit-assert) | ||
|
|
||
| Testing Library `getBy*` queries throw an error if the element is not | ||
| found. Some users like this behavior to use the query itself as an | ||
| assert for the element existence in their tests, but other users don't | ||
| and prefer to explicitly assert the element existence, so this rule is | ||
| for users from the latter. | ||
|
|
||
| ## Rule Details | ||
|
|
||
| This rule aims to encourage users to explicitly assert existence of | ||
| elements in their tests rather than just use `getBy*` queries and expect | ||
| it doesn't throw an error so it's easier to understand what's the | ||
| expected behavior within the test. | ||
|
|
||
| > ⚠️ Please note that this rule is recommended to be used together with | ||
| > [prefer-expect-query-by](docs/rules/prefer-expect-query-by.md), so the | ||
| > combination of these two rules will force users to do 2 actual | ||
| > changes: wrap `getBy*` with `expect` assertion and then use `queryBy*` | ||
| > instead of `getBy*` for asserting. | ||
|
|
||
| Examples of **incorrect** code for this rule: | ||
|
|
||
| ```js | ||
| // just calling `getBy*` query expecting not to throw an error as an | ||
| // assert-like method, without actually either using the returned element | ||
| // or explicitly asserting | ||
| getByText('foo'); | ||
| ``` | ||
|
|
||
| Examples of **correct** code for this rule: | ||
|
|
||
| ```js | ||
| // wrapping the get query within a `expect` and use some matcher for | ||
| // making the assertion more explicit | ||
| expect(getByText('foo')).toBeDefined(); | ||
|
|
||
| // ⚠️ `getBy*` should be replaced by `queryBy*` when combined with `prefer-expect-query-by` rule | ||
| expect(queryByText('foo')).toBeDefined(); | ||
|
|
||
| // even more explicit if you use `@testing-library/jest-dom` matcher | ||
| // for checking the element is present in the document | ||
| expect(queryByText('foo')).toBeInTheDocument(); | ||
|
|
||
| // Doing something with the element returned without asserting is absolutely fine | ||
| await waitForElement(() => getByText('foo')); | ||
| fireEvent.click(getByText('bar')); | ||
| const quxElement = getByText('qux'); | ||
|
|
||
| // call directly something different than Testing Library query | ||
| getByNonTestingLibraryVariant('foo'); | ||
| ``` | ||
|
|
||
| ## Options | ||
|
|
||
| This rule accepts a single options argument: | ||
|
|
||
| - `customQueries`: this array option allows to extend default Testing | ||
| Library queries with custom ones for including them into rule | ||
| inspection. | ||
|
|
||
| ```js | ||
| "testing-library/prefer-expect-query-by": ["error", {"customQueries": ["getByIcon", "getBySomethingElse"]}], | ||
| ``` | ||
|
|
||
| ## When Not To Use It | ||
|
|
||
| If you prefer to use `getBy*` queries implicitly as an assert-like | ||
| method itself, then this rule is not recommended. | ||
|
|
||
| ## Related Rules | ||
|
|
||
| - [prefer-expect-query-by](docs/rules/prefer-expect-query-by.md) | ||
|
|
||
| ## Further Reading | ||
|
|
||
| - [getBy query](https://testing-library.com/docs/dom-testing-library/api-queries#getby) | ||
This file contains hidden or 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 hidden or 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,73 @@ | ||
| 'use strict'; | ||
|
|
||
| const { getDocsUrl, ALL_QUERIES_METHODS } = require('../utils'); | ||
|
|
||
| const ALL_GET_BY_QUERIES = ALL_QUERIES_METHODS.map( | ||
| queryMethod => `get${queryMethod}` | ||
| ); | ||
|
|
||
| const isValidQuery = (node, customQueries = []) => | ||
| ALL_GET_BY_QUERIES.includes(node.name) || customQueries.includes(node.name); | ||
|
|
||
| const isDirectlyCalledByFunction = node => | ||
| node.parent.parent.type === 'CallExpression'; | ||
|
|
||
| const isReturnedByArrowFunctionExpression = node => | ||
| node.parent.parent.type === 'ArrowFunctionExpression'; | ||
|
|
||
| const isDeclared = node => node.parent.parent.type === 'VariableDeclarator'; | ||
|
|
||
| const isReturnedByReturnStatement = node => | ||
| node.parent.parent.type === 'ReturnStatement'; | ||
|
|
||
| module.exports = { | ||
| meta: { | ||
| type: 'suggestion', | ||
| docs: { | ||
| description: | ||
| 'Suggest using explicit assertions rather than just `getBy*` queries', | ||
| category: 'Best Practices', | ||
| recommended: false, | ||
| url: getDocsUrl('prefer-explicit-assert'), | ||
| }, | ||
| messages: { | ||
| preferExplicitAssert: | ||
| 'Wrap stand-alone `getBy*` query with `expect` function for better explicit assertion', | ||
| }, | ||
| fixable: null, | ||
| schema: [ | ||
| { | ||
| type: 'object', | ||
| properties: { | ||
| customQueries: { | ||
| type: 'array', | ||
| }, | ||
| }, | ||
| }, | ||
| ], | ||
| }, | ||
|
|
||
| create: function(context) { | ||
| return { | ||
| 'CallExpression > Identifier'(node) { | ||
| let customQueries; | ||
| if (context.options && context.options.length > 0) { | ||
| [{ customQueries }] = context.options; | ||
| } | ||
|
|
||
| if ( | ||
| isValidQuery(node, customQueries) && | ||
| !isDirectlyCalledByFunction(node) && | ||
| !isReturnedByArrowFunctionExpression(node) && | ||
| !isDeclared(node) && | ||
| !isReturnedByReturnStatement(node) | ||
| ) { | ||
| context.report({ | ||
| node, | ||
| messageId: 'preferExplicitAssert', | ||
| }); | ||
| } | ||
| }, | ||
| }; | ||
| }, | ||
| }; |
This file contains hidden or 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,86 @@ | ||
| 'use strict'; | ||
|
|
||
| const rule = require('../../../lib/rules/prefer-explicit-assert'); | ||
| const { ALL_QUERIES_METHODS } = require('../../../lib/utils'); | ||
| const RuleTester = require('eslint').RuleTester; | ||
|
|
||
| // ------------------------------------------------------------------------------ | ||
| // Tests | ||
| // ------------------------------------------------------------------------------ | ||
|
|
||
| const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 2018 } }); | ||
| ruleTester.run('prefer-explicit-assert', rule, { | ||
| valid: [ | ||
| { | ||
| code: `getByText`, | ||
|
Contributor
There was a problem hiding this comment. Can you also check for when queries are not destructured? const rendered = render(<App />)
rendered.getByText('...')
// ...
Member
Author
There was a problem hiding this comment. Nice spot! This actually found some issues in the rule, so I'm adding those tests and fixing the rule implementation.
Member
Author
There was a problem hiding this comment. Tests added, rule fixed and more examples added to the doc |
||
| }, | ||
| { | ||
| code: `expect(getByText('foo')).toBeDefined()`, | ||
| }, | ||
| { | ||
| code: `expect(getByText('foo')).toBeInTheDocument();`, | ||
| }, | ||
| { | ||
| code: `async () => { await waitForElement(() => getByText('foo')) }`, | ||
| }, | ||
| { | ||
| code: `fireEvent.click(getByText('bar'));`, | ||
| }, | ||
| { | ||
| code: `const quxElement = getByText('qux')`, | ||
| }, | ||
| { | ||
| code: `() => { return getByText('foo') }`, | ||
| }, | ||
| { | ||
| code: `function bar() { return getByText('foo') }`, | ||
| }, | ||
| { | ||
| code: `getByIcon('foo')`, // custom `getBy` query not extended through options | ||
| }, | ||
| ], | ||
|
|
||
| invalid: [ | ||
| ...ALL_QUERIES_METHODS.map(queryMethod => ({ | ||
| code: `get${queryMethod}('foo')`, | ||
| errors: [ | ||
| { | ||
| messageId: 'preferExplicitAssert', | ||
| }, | ||
| ], | ||
| })), | ||
| ...ALL_QUERIES_METHODS.map(queryMethod => ({ | ||
| code: `() => { | ||
| get${queryMethod}('foo') | ||
| doSomething() | ||
|
|
||
| get${queryMethod}('bar') | ||
| const quxElement = get${queryMethod}('qux') | ||
| } | ||
| `, | ||
| errors: [ | ||
| { | ||
| messageId: 'preferExplicitAssert', | ||
| line: 2, | ||
| }, | ||
| { | ||
| messageId: 'preferExplicitAssert', | ||
| line: 5, | ||
| }, | ||
| ], | ||
| })), | ||
| { | ||
| code: `getByIcon('foo')`, // custom `getBy` query extended through options | ||
| options: [ | ||
| { | ||
| customQueries: ['getByIcon'], | ||
| }, | ||
| ], | ||
| errors: [ | ||
| { | ||
| messageId: 'preferExplicitAssert', | ||
| }, | ||
| ], | ||
| }, | ||
| ], | ||
| }); | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.