diff --git a/README.md b/README.md index 1a6a7e9a..0d393722 100644 --- a/README.md +++ b/README.md @@ -129,14 +129,15 @@ To enable this configuration use the `extends` property in your ## Supported Rules -| Rule | Description | Configurations | Fixable | -| -------------------------------------------------------------- | ---------------------------------------------- | ------------------------------------------------------------------------- | ------------------ | -| [await-async-query](docs/rules/await-async-query.md) | Enforce async queries to have proper `await` | ![recommended-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | | -| [await-fire-event](docs/rules/await-fire-event.md) | Enforce async fire event methods to be awaited | ![vue-badge][] | | -| [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][] | | -| [no-debug](docs/rules/no-debug.md) | Disallow the use of `debug` | ![angular-badge][] ![react-badge][] ![vue-badge][] | | -| [no-dom-import](docs/rules/no-dom-import.md) | Disallow importing from DOM Testing Library | ![angular-badge][] ![react-badge][] ![vue-badge][] | ![fixable-badge][] | -| [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][] | | +| Rule | Description | Configurations | Fixable | +| -------------------------------------------------------------- | ------------------------------------------------------------------- | ------------------------------------------------------------------------- | ------------------ | +| [await-async-query](docs/rules/await-async-query.md) | Enforce async queries to have proper `await` | ![recommended-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | | +| [await-fire-event](docs/rules/await-fire-event.md) | Enforce async fire event methods to be awaited | ![vue-badge][] | | +| [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][] | | +| [no-debug](docs/rules/no-debug.md) | Disallow the use of `debug` | ![angular-badge][] ![react-badge][] ![vue-badge][] | | +| [no-dom-import](docs/rules/no-dom-import.md) | Disallow importing from DOM Testing Library | ![angular-badge][] ![react-badge][] ![vue-badge][] | ![fixable-badge][] | +| [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][] | | +| [prefer-explicit-assert](docs/rules/prefer-explicit-assert.md) | Suggest using explicit assertions rather than just `getBy*` queries | | | [build-badge]: https://img.shields.io/travis/Belco90/eslint-plugin-testing-library?style=flat-square [build-url]: https://travis-ci.org/belco90/eslint-plugin-testing-library diff --git a/docs/rules/prefer-explicit-assert.md b/docs/rules/prefer-explicit-assert.md new file mode 100644 index 00000000..98dd7dda --- /dev/null +++ b/docs/rules/prefer-explicit-assert.md @@ -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(); +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(); +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) diff --git a/lib/index.js b/lib/index.js index faaf6432..21b3cd4d 100644 --- a/lib/index.js +++ b/lib/index.js @@ -7,6 +7,7 @@ const rules = { 'no-debug': require('./rules/no-debug'), 'no-dom-import': require('./rules/no-dom-import'), 'prefer-expect-query-by': require('./rules/prefer-expect-query-by'), + 'prefer-explicit-assert': require('./rules/prefer-explicit-assert'), }; const recommendedRules = { diff --git a/lib/rules/prefer-explicit-assert.js b/lib/rules/prefer-explicit-assert.js new file mode 100644 index 00000000..60331c67 --- /dev/null +++ b/lib/rules/prefer-explicit-assert.js @@ -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', + }); + } + }, + }; + }, +}; diff --git a/tests/lib/rules/prefer-explicit-assert.js b/tests/lib/rules/prefer-explicit-assert.js new file mode 100644 index 00000000..9ca26f79 --- /dev/null +++ b/tests/lib/rules/prefer-explicit-assert.js @@ -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', + }, + ], + }, + ], +});