From 1b1a6619b14c7a2418e4100eb2d9c035fa6d64cb Mon Sep 17 00:00:00 2001 From: Julien Wajsberg Date: Mon, 16 Aug 2021 16:35:47 +0200 Subject: [PATCH] feat: Add an option to remove the matching of findBy queries by the prefer-explicit-assert rule A recent patch implemented matching findBy* queries by the prefer-explicit-assert rule. This patch makes it possible to disable it in cases where it's too distracting. Fixes #449 --- docs/rules/prefer-explicit-assert.md | 39 +++++++++-------- lib/rules/prefer-explicit-assert.ts | 42 ++++++++++--------- .../lib/rules/prefer-explicit-assert.test.ts | 17 ++++++++ 3 files changed, 62 insertions(+), 36 deletions(-) diff --git a/docs/rules/prefer-explicit-assert.md b/docs/rules/prefer-explicit-assert.md index 83c80280..1937dc30 100644 --- a/docs/rules/prefer-explicit-assert.md +++ b/docs/rules/prefer-explicit-assert.md @@ -13,7 +13,7 @@ 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. -Examples of **incorrect** code for this rule: +Examples of **incorrect** code for this rule with the default configuration: ```js // just calling `getBy*` query expecting not to throw an error as an @@ -23,9 +23,13 @@ getByText('foo'); const utils = render(); utils.getByText('foo'); + +// This is an incorrect code when `includeFindQueries` is `true`, which is the +// default. Set it to `false` to shut off all warnings about find* queries. +await findByText('foo'); ``` -Examples of **correct** code for this rule: +Examples of **correct** code for this rule with the default configuration: ```js // wrapping the get query within a `expect` and use some matcher for @@ -43,25 +47,26 @@ expect(queryByText('foo')).toBeInTheDocument(); await waitFor(() => getByText('foo')); fireEvent.click(getByText('bar')); const quxElement = getByText('qux'); + +expect(await findbyText('foo')).toBeTruthy(); +const myButton = await screen.findByRole('button', { name: /Accept/ }); ``` ## Options -This rule has one option: - -- `assertion`: this string allows defining the preferred assertion to use - with `getBy*` queries. By default, any assertion is valid (`toBeTruthy`, - `toBeDefined`, etc.). However, they all assert slightly different things. - This option ensures all `getBy*` assertions are consistent and use the same - assertion. This rule only allows defining a presence matcher - (`toBeInTheDocument`, `toBeTruthy`, or `toBeDefined`), but checks for both - presence and absence matchers (`not.toBeFalsy` and `not.toBeNull`). This means - other assertions such as `toHaveValue` or `toBeDisabled` will not trigger this - rule since these are valid uses with `getBy*`. - - ```js - "testing-library/prefer-explicit-assert": ["error", {"assertion": "toBeInTheDocument"}], - ``` +| Option | Required | Default | Details | Example | +| -------------------- | -------- | ------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------------------- | +| `assertion` | No | None | This string allows defining the preferred assertion to use with `getBy*` queries. By default, any assertion is valid (`toBeTruthy`, `toBeDefined`, etc.). However, they all assert slightly different things. This option ensures all `getBy*` assertions are consistent and use the same assertion. This rule only allows defining a presence matcher (`toBeInTheDocument`, `toBeTruthy`, or `toBeDefined`), but checks for both presence and absence matchers (`not.toBeFalsy` and `not.toBeNull`). This means other assertions such as `toHaveValue` or `toBeDisabled` will not trigger this rule since these are valid uses with `getBy*`. | `"toBeInTheDocument"` | +| `includeFindQueries` | No | `true` | This boolean controls whether queries such as `findByText` are also checked by this rule. | `false` | + +This is how you can use these options in eslint configuration: + +```js +"testing-library/prefer-explicit-assert": [ + "error", + { "assertion": "toBeInTheDocument", "includeFindQueries": false } +], +``` ## When Not To Use It diff --git a/lib/rules/prefer-explicit-assert.ts b/lib/rules/prefer-explicit-assert.ts index 20ffc076..a261b17f 100644 --- a/lib/rules/prefer-explicit-assert.ts +++ b/lib/rules/prefer-explicit-assert.ts @@ -15,6 +15,7 @@ export type MessageIds = type Options = [ { assertion?: string; + includeFindQueries?: boolean; } ]; @@ -91,13 +92,14 @@ export default createTestingLibraryRule({ type: 'string', enum: PRESENCE_MATCHERS, }, + includeFindQueries: { type: 'boolean' }, }, }, ], }, - defaultOptions: [{}], + defaultOptions: [{ includeFindQueries: true }], create(context, [options], helpers) { - const { assertion } = options; + const { assertion, includeFindQueries } = options; const getQueryCalls: TSESTree.Identifier[] = []; const findQueryCalls: TSESTree.Identifier[] = []; @@ -112,26 +114,28 @@ export default createTestingLibraryRule({ } }, 'Program:exit'() { - findQueryCalls.forEach((queryCall) => { - const memberExpression = isMemberExpression(queryCall.parent) - ? queryCall.parent - : queryCall; + if (includeFindQueries) { + findQueryCalls.forEach((queryCall) => { + const memberExpression = isMemberExpression(queryCall.parent) + ? queryCall.parent + : queryCall; - if ( - isVariableDeclaration(queryCall) || - !isAtTopLevel(memberExpression) - ) { - return; - } + if ( + isVariableDeclaration(queryCall) || + !isAtTopLevel(memberExpression) + ) { + return; + } - context.report({ - node: queryCall, - messageId: 'preferExplicitAssert', - data: { - queryType: 'findBy*', - }, + context.report({ + node: queryCall, + messageId: 'preferExplicitAssert', + data: { + queryType: 'findBy*', + }, + }); }); - }); + } getQueryCalls.forEach((queryCall) => { const node = isMemberExpression(queryCall.parent) diff --git a/tests/lib/rules/prefer-explicit-assert.test.ts b/tests/lib/rules/prefer-explicit-assert.test.ts index 2f40a380..e894e232 100644 --- a/tests/lib/rules/prefer-explicit-assert.test.ts +++ b/tests/lib/rules/prefer-explicit-assert.test.ts @@ -60,6 +60,23 @@ ruleTester.run(RULE_NAME, rule, { const quxElement = await find${queryMethod}('qux') }`, })), + ...COMBINED_QUERIES_METHODS.map((queryMethod) => ({ + code: ` + async () => { + expect(await find${queryMethod}('qux')).toBeInTheDocument(); + }`, + })), + ...COMBINED_QUERIES_METHODS.map((queryMethod) => ({ + code: ` + async () => { + await find${queryMethod}('foo') + }`, + options: [ + { + includeFindQueries: false, + }, + ], + })), ...COMBINED_QUERIES_METHODS.map((queryMethod) => ({ code: `const quxElement = find${queryMethod}('qux')`, })),