-
Notifications
You must be signed in to change notification settings - Fork 131
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
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
- Loading branch information
Showing
5 changed files
with
282 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,83 @@ | ||
| # 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'); | ||
|
|
||
| const utils = render(<Component />); | ||
| utils.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(); | ||
|
|
||
| const utils = render(<Component />); | ||
| expect(utils.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: | ||
|
|
||
| - `customQueryNames`: 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", {"customQueryNames": ["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 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,79 @@ | ||
| 'use strict'; | ||
|
|
||
| const { getDocsUrl, ALL_QUERIES_METHODS } = require('../utils'); | ||
|
|
||
| const ALL_GET_BY_QUERIES = ALL_QUERIES_METHODS.map( | ||
| queryMethod => `get${queryMethod}` | ||
| ); | ||
|
|
||
| const findCallExpressionParent = node => | ||
| node.type === 'CallExpression' ? node : findCallExpressionParent(node.parent); | ||
|
|
||
| const isValidQuery = (node, customQueryNames = []) => | ||
| ALL_GET_BY_QUERIES.includes(node.name) || | ||
| customQueryNames.includes(node.name); | ||
|
|
||
| const isDirectlyCalledByFunction = node => | ||
| node.parent.type === 'CallExpression'; | ||
|
|
||
| const isReturnedByArrowFunctionExpression = node => | ||
| node.parent.type === 'ArrowFunctionExpression'; | ||
|
|
||
| const isDeclared = node => node.parent.type === 'VariableDeclarator'; | ||
|
|
||
| const isReturnedByReturnStatement = node => | ||
| node.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: { | ||
| customQueryNames: { | ||
| type: 'array', | ||
| }, | ||
| }, | ||
| }, | ||
| ], | ||
| }, | ||
|
|
||
| create: function(context) { | ||
| return { | ||
| 'CallExpression Identifier'(node) { | ||
| const callExpressionNode = findCallExpressionParent(node); | ||
|
|
||
| let customQueryNames; | ||
| if (context.options && context.options.length > 0) { | ||
| [{ customQueryNames }] = context.options; | ||
| } | ||
|
|
||
| if ( | ||
| isValidQuery(node, customQueryNames) && | ||
| !isDirectlyCalledByFunction(callExpressionNode) && | ||
| !isReturnedByArrowFunctionExpression(callExpressionNode) && | ||
| !isDeclared(callExpressionNode) && | ||
| !isReturnedByReturnStatement(callExpressionNode) | ||
| ) { | ||
| context.report({ | ||
| node, | ||
| messageId: 'preferExplicitAssert', | ||
| }); | ||
| } | ||
| }, | ||
| }; | ||
| }, | ||
| }; |
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,110 @@ | ||
| '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`, | ||
| }, | ||
| { | ||
| code: `const utils = render() | ||
| utils.getByText | ||
| `, | ||
| }, | ||
| { | ||
| code: `expect(getByText('foo')).toBeDefined()`, | ||
| }, | ||
| { | ||
| code: `const utils = render() | ||
| expect(utils.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: `const utils = render() | ||
| utils.get${queryMethod}('foo')`, | ||
| errors: [ | ||
| { | ||
| messageId: 'preferExplicitAssert', | ||
| line: 3, | ||
| column: 13, | ||
| }, | ||
| ], | ||
| })), | ||
| ...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: [ | ||
| { | ||
| customQueryNames: ['getByIcon'], | ||
| }, | ||
| ], | ||
| errors: [ | ||
| { | ||
| messageId: 'preferExplicitAssert', | ||
| }, | ||
| ], | ||
| }, | ||
| ], | ||
| }); |