From 8606e246a18b5ac8ded24104058c29b3efc8e104 Mon Sep 17 00:00:00 2001 From: Josh Justice Date: Tue, 4 Apr 2023 14:43:29 -0400 Subject: [PATCH 1/9] feat: add prefer-query-matchers rule Co-authored-by: Dale Karp --- .../detect-testing-library-utils.ts | 16 +++ lib/rules/prefer-query-matchers.ts | 105 ++++++++++++++++++ 2 files changed, 121 insertions(+) create mode 100644 lib/rules/prefer-query-matchers.ts diff --git a/lib/create-testing-library-rule/detect-testing-library-utils.ts b/lib/create-testing-library-rule/detect-testing-library-utils.ts index cf65a0e4..9393a88b 100644 --- a/lib/create-testing-library-rule/detect-testing-library-utils.ts +++ b/lib/create-testing-library-rule/detect-testing-library-utils.ts @@ -85,6 +85,10 @@ type IsDebugUtilFn = ( validNames?: ReadonlyArray<(typeof DEBUG_UTILS)[number]> ) => boolean; type IsPresenceAssertFn = (node: TSESTree.MemberExpression) => boolean; +type IsMatchingAssertFn = ( + node: TSESTree.MemberExpression, + matcherName: string +) => boolean; type IsAbsenceAssertFn = (node: TSESTree.MemberExpression) => boolean; type CanReportErrorsFn = () => boolean; type FindImportedTestingLibraryUtilSpecifierFn = ( @@ -122,6 +126,7 @@ export interface DetectionHelpers { isActUtil: (node: TSESTree.Identifier) => boolean; isPresenceAssert: IsPresenceAssertFn; isAbsenceAssert: IsAbsenceAssertFn; + isMatchingAssert: IsMatchingAssertFn; canReportErrors: CanReportErrorsFn; findImportedTestingLibraryUtilSpecifier: FindImportedTestingLibraryUtilSpecifierFn; isNodeComingFromTestingLibrary: IsNodeComingFromTestingLibraryFn; @@ -819,6 +824,16 @@ export function detectTestingLibraryUtils< : ABSENCE_MATCHERS.includes(matcher); }; + const isMatchingAssert: IsMatchingAssertFn = (node, matcherName) => { + const { matcher } = getAssertNodeInfo(node); + + if (!matcher) { + return false; + } + + return matcher === matcherName; + }; + /** * Finds the import util specifier related to Testing Library for a given name. */ @@ -977,6 +992,7 @@ export function detectTestingLibraryUtils< isDebugUtil, isActUtil, isPresenceAssert, + isMatchingAssert, isAbsenceAssert, canReportErrors, findImportedTestingLibraryUtilSpecifier, diff --git a/lib/rules/prefer-query-matchers.ts b/lib/rules/prefer-query-matchers.ts new file mode 100644 index 00000000..932113a2 --- /dev/null +++ b/lib/rules/prefer-query-matchers.ts @@ -0,0 +1,105 @@ +import { TSESTree } from '@typescript-eslint/utils'; + +import { createTestingLibraryRule } from '../create-testing-library-rule'; +import { findClosestCallNode, isMemberExpression } from '../node-utils'; + +export const RULE_NAME = 'prefer-query-matchers'; +export type MessageIds = 'wrongQueryForMatcher'; +export type Options = [ + { + validEntries: { + query: 'get' | 'query'; + matcher: string; + }[]; + } +]; + +export default createTestingLibraryRule({ + name: RULE_NAME, + meta: { + docs: { + description: + 'Ensure the configured `get*`/`query*` query is used with the corresponding matchers', + recommendedConfig: { + dom: 'error', + angular: 'error', + react: 'error', + vue: 'error', + marko: 'error', + }, + }, + messages: { + wrongQueryForMatcher: 'Use `{{ query }}By*` queries for {{ matcher }}', + }, + schema: [ + { + type: 'object', + additionalProperties: false, + properties: { + validEntries: { + type: 'array', + items: { + type: 'object', + properties: { + query: { + type: 'string', + enum: ['get', 'query'] + }, + matcher: { + type: 'string', + }, + }, + }, + }, + }, + }, + ], + type: 'suggestion', + }, + defaultOptions: [ + { + validEntries: [{ query: 'get', matcher: 'toBeVisible' }], + }, + ], + + create(context, [{ validEntries }], helpers) { + return { + 'CallExpression Identifier'(node: TSESTree.Identifier) { + const expectCallNode = findClosestCallNode(node, 'expect'); + + if (!expectCallNode || !isMemberExpression(expectCallNode.parent)) { + return; + } + + // Sync queries (getBy and queryBy) and corresponding ones + // are supported. If none found, stop the rule. + if (!helpers.isSyncQuery(node)) { + return; + } + + const isGetBy = helpers.isGetQueryVariant(node); + const expectStatement = expectCallNode.parent; + for (const entry of validEntries) { + const { query, matcher } = entry; + const isMatchingAssertForThisEntry = helpers.isMatchingAssert( + expectStatement, + matcher + ); + + if (!isMatchingAssertForThisEntry) { + continue; + } + + const actualQuery = isGetBy ? 'get' : 'query'; + if (query !== actualQuery) { + context.report({ + node, + messageId: 'wrongQueryForMatcher', + data: { query, matcher }, + }); + } + } + }, + }; + }, +}); From 1ba34cfc42e261d36120af8577d14d66a0f20f28 Mon Sep 17 00:00:00 2001 From: Josh Justice Date: Tue, 4 Apr 2023 15:46:59 -0400 Subject: [PATCH 2/9] test: cover prefer-query-matchers with test Co-authored-by: Dale Karp --- tests/lib/rules/prefer-query-matchers.test.ts | 452 ++++++++++++++++++ 1 file changed, 452 insertions(+) create mode 100644 tests/lib/rules/prefer-query-matchers.test.ts diff --git a/tests/lib/rules/prefer-query-matchers.test.ts b/tests/lib/rules/prefer-query-matchers.test.ts new file mode 100644 index 00000000..6d6009ac --- /dev/null +++ b/tests/lib/rules/prefer-query-matchers.test.ts @@ -0,0 +1,452 @@ +import { TSESLint } from '@typescript-eslint/utils'; + +import rule, { + RULE_NAME, + MessageIds, + Options, +} from '../../../lib/rules/prefer-query-matchers'; +import { ALL_QUERIES_METHODS } from '../../../lib/utils'; +import { createRuleTester } from '../test-utils'; + +const ruleTester = createRuleTester(); + +const getByQueries = ALL_QUERIES_METHODS.map((method) => `get${method}`); +const getAllByQueries = ALL_QUERIES_METHODS.map((method) => `getAll${method}`); +const queryByQueries = ALL_QUERIES_METHODS.map((method) => `query${method}`); +const queryAllByQueries = ALL_QUERIES_METHODS.map( + (method) => `queryAll${method}` +); + +type RuleValidTestCase = TSESLint.ValidTestCase; +type RuleInvalidTestCase = TSESLint.InvalidTestCase; + +type AssertionFnParams = { + query: string; + matcher: string; + includeDefaultOptionsCase?: boolean; + options: Options; +}; + +const getValidAssertions = ({ + query, + matcher, + includeDefaultOptionsCase = true, + options, +}: AssertionFnParams): RuleValidTestCase[] => { + const code = `expect(${query}('Hello'))${matcher}`; + const screenCode = `expect(screen.${query}('Hello'))${matcher}`; + return [ + ...(includeDefaultOptionsCase + ? [ + { + name: `${code} with default options`, + code, + }, + ] + : []), + { + name: `${code} with provided options`, + code, + options, + }, + { + name: `${code} with no validEntries`, + code, + options: [{ validEntries: [] }], + }, + ...(includeDefaultOptionsCase + ? [ + { + name: `${screenCode} with default options`, + code, + }, + ] + : []), + { + name: `${screenCode} with provided options`, + code: screenCode, + options, + }, + { + name: `${screenCode} with no validEntries`, + code: screenCode, + options: [{ validEntries: [] }], + }, + ]; +}; + +const getInvalidAssertions = ({ + query, + matcher, + includeDefaultOptionsCase = true, + options, +}: AssertionFnParams): RuleInvalidTestCase[] => { + const code = `expect(${query}('Hello'))${matcher}`; + const screenCode = `expect(screen.${query}('Hello'))${matcher}`; + const messageId: MessageIds = 'wrongQueryForMatcher'; + const [ + { + validEntries: [validEntry], + }, + ] = options; + return [ + ...(includeDefaultOptionsCase + ? [ + { + name: `${code} with default options`, + code, + errors: [ + { + messageId, + line: 1, + column: 8, + data: { query: validEntry.query, matcher: validEntry.matcher }, + }, + ], + }, + ] + : []), + { + name: `${code} with provided options`, + code, + options, + errors: [ + { + messageId, + line: 1, + column: 8, + data: { query: validEntry.query, matcher: validEntry.matcher }, + }, + ], + }, + ...(includeDefaultOptionsCase + ? [ + { + name: `${screenCode} with default options`, + code: screenCode, + errors: [ + { + messageId, + line: 1, + column: 15, + data: { query: validEntry.query, matcher: validEntry.matcher }, + }, + ], + }, + ] + : []), + { + name: `${screenCode} with provided options`, + code: screenCode, + options, + errors: [ + { + messageId, + line: 1, + column: 15, + data: { query: validEntry.query, matcher: validEntry.matcher }, + }, + ], + }, + ]; +}; + +ruleTester.run(RULE_NAME, rule, { + valid: [ + // cases: methods not matching Testing Library queries pattern + `expect(queryElement('foo')).toBeVisible()`, + `expect(getElement('foo')).not.toBeVisible()`, + // cases: asserting with a configured allowed `[screen.]getBy*` query + ...getByQueries.reduce( + (validRules, queryName) => [ + ...validRules, + ...getValidAssertions({ + query: queryName, + matcher: '.toBeVisible()', + options: [ + { validEntries: [{ query: 'get', matcher: 'toBeVisible' }] }, + ], + }), + ...getValidAssertions({ + query: queryName, + matcher: '.toBeHelloWorld()', + options: [ + { validEntries: [{ query: 'get', matcher: 'toBeVisible' }] }, + ], + }), + ...getValidAssertions({ + query: queryName, + matcher: '.not.toBeVisible()', + options: [ + { validEntries: [{ query: 'get', matcher: 'toBeVisible' }] }, + ], + }), + ...getValidAssertions({ + query: queryName, + matcher: '.not.toBeHelloWorld()', + options: [ + { validEntries: [{ query: 'get', matcher: 'toBeVisible' }] }, + ], + }), + ...getValidAssertions({ + query: queryName, + matcher: '.toBeHelloWorld()', + options: [ + { validEntries: [{ query: 'query', matcher: 'toBeVisible' }] }, + ], + }), + ], + [] + ), + // cases: asserting with a configured allowed `[screen.]getAllBy*` query + ...getAllByQueries.reduce( + (validRules, queryName) => [ + ...validRules, + ...getValidAssertions({ + query: queryName, + matcher: '.toBeVisible()', + options: [ + { validEntries: [{ query: 'get', matcher: 'toBeVisible' }] }, + ], + }), + ...getValidAssertions({ + query: queryName, + matcher: '.toBeHelloWorld()', + options: [ + { validEntries: [{ query: 'get', matcher: 'toBeVisible' }] }, + ], + }), + ...getValidAssertions({ + query: queryName, + matcher: '.not.toBeVisible()', + options: [ + { validEntries: [{ query: 'get', matcher: 'toBeVisible' }] }, + ], + }), + ...getValidAssertions({ + query: queryName, + matcher: '.not.toBeHelloWorld()', + options: [ + { validEntries: [{ query: 'get', matcher: 'toBeVisible' }] }, + ], + }), + ...getValidAssertions({ + query: queryName, + matcher: '.toBeHelloWorld()', + options: [ + { validEntries: [{ query: 'query', matcher: 'toBeVisible' }] }, + ], + }), + ], + [] + ), + // cases: asserting with a configured allowed `[screen.]queryBy*` query + ...queryByQueries.reduce( + (validRules, queryName) => [ + ...validRules, + ...getValidAssertions({ + query: queryName, + matcher: '.toBeVisible()', + includeDefaultOptionsCase: false, + options: [ + { validEntries: [{ query: 'query', matcher: 'toBeVisible' }] }, + ], + }), + ...getValidAssertions({ + query: queryName, + matcher: '.toBeHelloWorld()', + includeDefaultOptionsCase: false, + options: [ + { validEntries: [{ query: 'query', matcher: 'toBeVisible' }] }, + ], + }), + ...getValidAssertions({ + query: queryName, + matcher: '.not.toBeVisible()', + includeDefaultOptionsCase: false, + options: [ + { validEntries: [{ query: 'query', matcher: 'toBeVisible' }] }, + ], + }), + ...getValidAssertions({ + query: queryName, + matcher: '.not.toBeHelloWorld()', + includeDefaultOptionsCase: false, + options: [ + { validEntries: [{ query: 'query', matcher: 'toBeVisible' }] }, + ], + }), + ...getValidAssertions({ + query: queryName, + matcher: '.toBeHelloWorld()', + options: [ + { validEntries: [{ query: 'get', matcher: 'toBeVisible' }] }, + ], + }), + ], + [] + ), + // cases: asserting with a configured allowed `[screen.]queryAllBy*` query + ...queryAllByQueries.reduce( + (validRules, queryName) => [ + ...validRules, + ...getValidAssertions({ + query: queryName, + matcher: '.toBeVisible()', + includeDefaultOptionsCase: false, + options: [ + { validEntries: [{ query: 'query', matcher: 'toBeVisible' }] }, + ], + }), + ...getValidAssertions({ + query: queryName, + matcher: '.toBeHelloWorld()', + includeDefaultOptionsCase: false, + options: [ + { validEntries: [{ query: 'query', matcher: 'toBeVisible' }] }, + ], + }), + ...getValidAssertions({ + query: queryName, + matcher: '.not.toBeVisible()', + includeDefaultOptionsCase: false, + options: [ + { validEntries: [{ query: 'query', matcher: 'toBeVisible' }] }, + ], + }), + ...getValidAssertions({ + query: queryName, + matcher: '.not.toBeHelloWorld()', + includeDefaultOptionsCase: false, + options: [ + { validEntries: [{ query: 'query', matcher: 'toBeVisible' }] }, + ], + }), + ...getValidAssertions({ + query: queryName, + matcher: '.toBeHelloWorld()', + options: [ + { validEntries: [{ query: 'get', matcher: 'toBeVisible' }] }, + ], + }), + ], + [] + ), + // case: getting outside an expectation + { + code: 'const el = getByText("button")', + }, + // case: querying outside an expectation + { + code: 'const el = queryByText("button")', + }, + // case: finding outside an expectation + { + code: `async () => { + const el = await findByText('button') + expect(el).toBeVisible() + }`, + }, + { + code: `// case: query an element with getBy but then check its absence after doing + // some action which makes it disappear. + + // submit button exists + const submitButton = screen.getByRole('button') + fireEvent.click(submitButton) + + // right after clicking submit button it disappears + expect(submitButton).toBeHelloWorld() + `, + options: [ + { validEntries: [{ query: 'query', matcher: 'toBeHelloWorld' }] }, + ], + }, + ], + invalid: [ + // cases: asserting with a disallowed `[screen.]getBy*` query + ...getByQueries.reduce( + (validRules, queryName) => [ + ...validRules, + ...getInvalidAssertions({ + query: queryName, + matcher: '.toBeHelloWorld()', + includeDefaultOptionsCase: false, + options: [ + { validEntries: [{ query: 'query', matcher: 'toBeHelloWorld' }] }, + ], + }), + ], + [] + ), + // cases: asserting with a disallowed `[screen.]getAllBy*` query + ...getAllByQueries.reduce( + (validRules, queryName) => [ + ...validRules, + ...getInvalidAssertions({ + query: queryName, + matcher: '.toBeHelloWorld()', + includeDefaultOptionsCase: false, + options: [ + { validEntries: [{ query: 'query', matcher: 'toBeHelloWorld' }] }, + ], + }), + ], + [] + ), + // cases: asserting with a disallowed `[screen.]getBy*` query + ...queryByQueries.reduce( + (invalidRules, queryName) => [ + ...invalidRules, + ...getInvalidAssertions({ + query: queryName, + matcher: '.toBeVisible()', + options: [ + { validEntries: [{ query: 'get', matcher: 'toBeVisible' }] }, + ], + }), + ], + [] + ), + // cases: asserting with a disallowed `[screen.]queryAllBy*` query + ...queryAllByQueries.reduce( + (invalidRules, queryName) => [ + ...invalidRules, + ...getInvalidAssertions({ + query: queryName, + matcher: '.toBeVisible()', + options: [ + { validEntries: [{ query: 'get', matcher: 'toBeVisible' }] }, + ], + }), + ], + [] + ), + // cases: indexing into an `AllBy` result within the expectation + { + code: 'expect(screen.queryAllByText("button")[1]).toBeVisible()', + errors: [ + { + messageId: 'wrongQueryForMatcher', + line: 1, + column: 15, + data: { query: 'get', matcher: 'toBeVisible' }, + }, + ], + }, + { + code: ` + // case: asserting presence incorrectly with custom queryBy* query + expect(queryByCustomQuery("button")).toBeVisible() + `, + errors: [ + { + messageId: 'wrongQueryForMatcher', + line: 3, + column: 12, + data: { query: 'get', matcher: 'toBeVisible' }, + }, + ], + }, + ], +}); From 99e0d2c4d1121833e63dcaea69189b232d36c30b Mon Sep 17 00:00:00 2001 From: Josh Justice Date: Mon, 10 Apr 2023 16:45:19 -0400 Subject: [PATCH 3/9] feat: set default options to no configured entries to check for Co-authored-by: Dale Karp --- lib/rules/prefer-query-matchers.ts | 2 +- tests/lib/rules/prefer-query-matchers.test.ts | 77 ++----------------- 2 files changed, 9 insertions(+), 70 deletions(-) diff --git a/lib/rules/prefer-query-matchers.ts b/lib/rules/prefer-query-matchers.ts index 932113a2..94f3108d 100644 --- a/lib/rules/prefer-query-matchers.ts +++ b/lib/rules/prefer-query-matchers.ts @@ -58,7 +58,7 @@ export default createTestingLibraryRule({ }, defaultOptions: [ { - validEntries: [{ query: 'get', matcher: 'toBeVisible' }], + validEntries: [], }, ], diff --git a/tests/lib/rules/prefer-query-matchers.test.ts b/tests/lib/rules/prefer-query-matchers.test.ts index 6d6009ac..879805e2 100644 --- a/tests/lib/rules/prefer-query-matchers.test.ts +++ b/tests/lib/rules/prefer-query-matchers.test.ts @@ -23,54 +23,34 @@ type RuleInvalidTestCase = TSESLint.InvalidTestCase; type AssertionFnParams = { query: string; matcher: string; - includeDefaultOptionsCase?: boolean; options: Options; }; const getValidAssertions = ({ query, matcher, - includeDefaultOptionsCase = true, options, }: AssertionFnParams): RuleValidTestCase[] => { const code = `expect(${query}('Hello'))${matcher}`; const screenCode = `expect(screen.${query}('Hello'))${matcher}`; return [ - ...(includeDefaultOptionsCase - ? [ - { - name: `${code} with default options`, - code, - }, - ] - : []), { - name: `${code} with provided options`, + name: `${code} with default options of empty validEntries`, code, - options, }, { - name: `${code} with no validEntries`, + name: `${code} with provided options`, code, - options: [{ validEntries: [] }], + options, }, - ...(includeDefaultOptionsCase - ? [ - { - name: `${screenCode} with default options`, - code, - }, - ] - : []), { - name: `${screenCode} with provided options`, + name: `${screenCode} with default options of empty validEntries`, code: screenCode, - options, }, { - name: `${screenCode} with no validEntries`, + name: `${screenCode} with provided options`, code: screenCode, - options: [{ validEntries: [] }], + options, }, ]; }; @@ -78,7 +58,6 @@ const getValidAssertions = ({ const getInvalidAssertions = ({ query, matcher, - includeDefaultOptionsCase = true, options, }: AssertionFnParams): RuleInvalidTestCase[] => { const code = `expect(${query}('Hello'))${matcher}`; @@ -90,22 +69,6 @@ const getInvalidAssertions = ({ }, ] = options; return [ - ...(includeDefaultOptionsCase - ? [ - { - name: `${code} with default options`, - code, - errors: [ - { - messageId, - line: 1, - column: 8, - data: { query: validEntry.query, matcher: validEntry.matcher }, - }, - ], - }, - ] - : []), { name: `${code} with provided options`, code, @@ -119,22 +82,6 @@ const getInvalidAssertions = ({ }, ], }, - ...(includeDefaultOptionsCase - ? [ - { - name: `${screenCode} with default options`, - code: screenCode, - errors: [ - { - messageId, - line: 1, - column: 15, - data: { query: validEntry.query, matcher: validEntry.matcher }, - }, - ], - }, - ] - : []), { name: `${screenCode} with provided options`, code: screenCode, @@ -247,7 +194,6 @@ ruleTester.run(RULE_NAME, rule, { ...getValidAssertions({ query: queryName, matcher: '.toBeVisible()', - includeDefaultOptionsCase: false, options: [ { validEntries: [{ query: 'query', matcher: 'toBeVisible' }] }, ], @@ -255,7 +201,6 @@ ruleTester.run(RULE_NAME, rule, { ...getValidAssertions({ query: queryName, matcher: '.toBeHelloWorld()', - includeDefaultOptionsCase: false, options: [ { validEntries: [{ query: 'query', matcher: 'toBeVisible' }] }, ], @@ -263,7 +208,6 @@ ruleTester.run(RULE_NAME, rule, { ...getValidAssertions({ query: queryName, matcher: '.not.toBeVisible()', - includeDefaultOptionsCase: false, options: [ { validEntries: [{ query: 'query', matcher: 'toBeVisible' }] }, ], @@ -271,7 +215,6 @@ ruleTester.run(RULE_NAME, rule, { ...getValidAssertions({ query: queryName, matcher: '.not.toBeHelloWorld()', - includeDefaultOptionsCase: false, options: [ { validEntries: [{ query: 'query', matcher: 'toBeVisible' }] }, ], @@ -293,7 +236,6 @@ ruleTester.run(RULE_NAME, rule, { ...getValidAssertions({ query: queryName, matcher: '.toBeVisible()', - includeDefaultOptionsCase: false, options: [ { validEntries: [{ query: 'query', matcher: 'toBeVisible' }] }, ], @@ -301,7 +243,6 @@ ruleTester.run(RULE_NAME, rule, { ...getValidAssertions({ query: queryName, matcher: '.toBeHelloWorld()', - includeDefaultOptionsCase: false, options: [ { validEntries: [{ query: 'query', matcher: 'toBeVisible' }] }, ], @@ -309,7 +250,6 @@ ruleTester.run(RULE_NAME, rule, { ...getValidAssertions({ query: queryName, matcher: '.not.toBeVisible()', - includeDefaultOptionsCase: false, options: [ { validEntries: [{ query: 'query', matcher: 'toBeVisible' }] }, ], @@ -317,7 +257,6 @@ ruleTester.run(RULE_NAME, rule, { ...getValidAssertions({ query: queryName, matcher: '.not.toBeHelloWorld()', - includeDefaultOptionsCase: false, options: [ { validEntries: [{ query: 'query', matcher: 'toBeVisible' }] }, ], @@ -371,7 +310,6 @@ ruleTester.run(RULE_NAME, rule, { ...getInvalidAssertions({ query: queryName, matcher: '.toBeHelloWorld()', - includeDefaultOptionsCase: false, options: [ { validEntries: [{ query: 'query', matcher: 'toBeHelloWorld' }] }, ], @@ -386,7 +324,6 @@ ruleTester.run(RULE_NAME, rule, { ...getInvalidAssertions({ query: queryName, matcher: '.toBeHelloWorld()', - includeDefaultOptionsCase: false, options: [ { validEntries: [{ query: 'query', matcher: 'toBeHelloWorld' }] }, ], @@ -425,6 +362,7 @@ ruleTester.run(RULE_NAME, rule, { // cases: indexing into an `AllBy` result within the expectation { code: 'expect(screen.queryAllByText("button")[1]).toBeVisible()', + options: [{ validEntries: [{ query: 'get', matcher: 'toBeVisible' }] }], errors: [ { messageId: 'wrongQueryForMatcher', @@ -439,6 +377,7 @@ ruleTester.run(RULE_NAME, rule, { // case: asserting presence incorrectly with custom queryBy* query expect(queryByCustomQuery("button")).toBeVisible() `, + options: [{ validEntries: [{ query: 'get', matcher: 'toBeVisible' }] }], errors: [ { messageId: 'wrongQueryForMatcher', From b212af4a18b7033c87fa70cb2297684abc7acf37 Mon Sep 17 00:00:00 2001 From: Josh Justice Date: Tue, 11 Apr 2023 07:41:14 -0400 Subject: [PATCH 4/9] docs: add query doc and supported rules table entry --- README.md | 1 + docs/rules/prefer-presence-queries.md | 1 + docs/rules/prefer-query-matchers.md | 85 +++++++++++++++++++++++++++ 3 files changed, 87 insertions(+) create mode 100644 docs/rules/prefer-query-matchers.md diff --git a/README.md b/README.md index de21469a..19ac8bcd 100644 --- a/README.md +++ b/README.md @@ -228,6 +228,7 @@ module.exports = { | [prefer-find-by](docs/rules/prefer-find-by.md) | Suggest using `find(All)By*` query instead of `waitFor` + `get(All)By*` to wait for elements | ![badge-angular][] ![badge-dom][] ![badge-marko][] ![badge-react][] ![badge-vue][] | 🔧 | | [prefer-presence-queries](docs/rules/prefer-presence-queries.md) | Ensure appropriate `get*`/`query*` queries are used with their respective matchers | ![badge-angular][] ![badge-dom][] ![badge-marko][] ![badge-react][] ![badge-vue][] | | | [prefer-query-by-disappearance](docs/rules/prefer-query-by-disappearance.md) | Suggest using `queryBy*` queries when waiting for disappearance | ![badge-angular][] ![badge-dom][] ![badge-marko][] ![badge-react][] ![badge-vue][] | | +| [prefer-query-matchers](docs/rules/prefer-query-matchers.md) | Ensure the configured `get*`/`query*` query is used with the corresponding matchers | | | | [prefer-screen-queries](docs/rules/prefer-screen-queries.md) | Suggest using `screen` while querying | ![badge-angular][] ![badge-dom][] ![badge-marko][] ![badge-react][] ![badge-vue][] | | | [prefer-user-event](docs/rules/prefer-user-event.md) | Suggest using `userEvent` over `fireEvent` for simulating user interactions | | | | [prefer-wait-for](docs/rules/prefer-wait-for.md) | Use `waitFor` instead of deprecated wait methods | | 🔧 | diff --git a/docs/rules/prefer-presence-queries.md b/docs/rules/prefer-presence-queries.md index af1df93c..6b773089 100644 --- a/docs/rules/prefer-presence-queries.md +++ b/docs/rules/prefer-presence-queries.md @@ -43,6 +43,7 @@ Examples of **correct** code for this rule: ```js test('some test', async () => { render(); + // check element is present with `getBy*` expect(screen.getByText('button')).toBeInTheDocument(); expect(screen.getAllByText('button')[9]).toBeTruthy(); diff --git a/docs/rules/prefer-query-matchers.md b/docs/rules/prefer-query-matchers.md new file mode 100644 index 00000000..eec3fa2f --- /dev/null +++ b/docs/rules/prefer-query-matchers.md @@ -0,0 +1,85 @@ +# Ensure the configured `get*`/`query*` query is used with the corresponding matchers (`testing-library/prefer-query-matchers`) + + + +The (DOM) Testing Library allows to query DOM elements using different types of queries such as `get*` and `query*`. Using `get*` throws an error in case the element is not found, while `query*` returns null instead of throwing (or empty array for `queryAllBy*` ones). + +It may be helpful to ensure that either `get*` or `query*` are always used for a given matcher. For example, `.toBeVisible()` and the negation `.not.toBeVisible()` both assume that an element exists in the DOM and will error if not. Using `get*` with `.toBeVisible()` ensures that if the element is not found the error thrown will offer better info than with `query*`. + +## Rule details + +This rule must be configured with a list of `validEntries`: for a given matcher, is `get*` or `query*` required. + +Assuming the following configuration: + +```json +{ + "testing-library/prefer-query-matchers": [ + 2, + { + "validEntries": [{ "matcher": "toBeVisible", "query": "get" }] + } + ] +} +``` + +Examples of **incorrect** code for this rule with the above configuration: + +```js +test('some test', () => { + render(); + + // use configured matcher with the disallowed `query*` + expect(screen.queryByText('button')).toBeVisible(); + expect(screen.queryByText('button')).not.toBeVisible(); + expect(screen.queryAllByText('button')[0]).toBeVisible(); + expect(screen.queryAllByText('button')[0]).not.toBeVisible(); +}); +``` + +Examples of **correct** code for this rule: + +```js +test('some test', async () => { + render(); + // use configured matcher with the allowed `get*` + expect(screen.getByText('button')).toBeVisible(); + expect(screen.getByText('button')).not.toBeVisible(); + expect(screen.getAllByText('button')[0]).toBeVisible(); + expect(screen.getAllByText('button')[0]).not.toBeVisible(); + + // use an unconfigured matcher with either `get* or `query* + expect(screen.getByText('button')).toBeEnabled(); + expect(screen.getAllByText('checkbox')[0]).not.toBeChecked(); + expect(screen.queryByText('button')).toHaveFocus(); + expect(screen.queryAllByText('button')[0]).not.toMatchMyCustomMatcher(); + + // `findBy*` queries are out of the scope for this rule + const button = await screen.findByText('submit'); + expect(button).toBeVisible(); +}); +``` + +## Options + +| Option | Required | Default | Details | +| -------------- | -------- | ------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| `validEntries` | No | `[]` | A list of objects with a `matcher` property (the name of any matcher, such as "toBeVisible") and a `query` property (either "get" or "query"). Indicates whether `get*` or `query*` are allowed with this matcher. | + +## Example + +```json +{ + "testing-library/prefer-query-matchers": [ + 2, + { + "validEntries": [{ "matcher": "toBeVisible", "query": "get" }] + } + ] +} +``` + +## Further Reading + +- [Testing Library queries cheatsheet](https://testing-library.com/docs/dom-testing-library/cheatsheet#queries) +- [jest-dom note about using `getBy` within assertions](https://testing-library.com/docs/ecosystem-jest-dom) From d08851d1c46a6bc0bf3ae084c4bf0a9eef6fd53d Mon Sep 17 00:00:00 2001 From: Josh Justice Date: Wed, 26 Apr 2023 06:02:11 -0400 Subject: [PATCH 5/9] style: prettify rule file --- lib/rules/prefer-query-matchers.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rules/prefer-query-matchers.ts b/lib/rules/prefer-query-matchers.ts index 94f3108d..c550c8d5 100644 --- a/lib/rules/prefer-query-matchers.ts +++ b/lib/rules/prefer-query-matchers.ts @@ -43,7 +43,7 @@ export default createTestingLibraryRule({ properties: { query: { type: 'string', - enum: ['get', 'query'] + enum: ['get', 'query'], }, matcher: { type: 'string', From 625c43868425ea8427a1bb3d77c3aca98732d9e2 Mon Sep 17 00:00:00 2001 From: Josh Justice Date: Wed, 26 Apr 2023 06:57:44 -0400 Subject: [PATCH 6/9] fix: do not include prefer-query-matchers by default --- lib/rules/prefer-query-matchers.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/rules/prefer-query-matchers.ts b/lib/rules/prefer-query-matchers.ts index c550c8d5..3f94a8e9 100644 --- a/lib/rules/prefer-query-matchers.ts +++ b/lib/rules/prefer-query-matchers.ts @@ -21,11 +21,11 @@ export default createTestingLibraryRule({ description: 'Ensure the configured `get*`/`query*` query is used with the corresponding matchers', recommendedConfig: { - dom: 'error', - angular: 'error', - react: 'error', - vue: 'error', - marko: 'error', + dom: false, + angular: false, + react: false, + vue: false, + marko: false, }, }, messages: { From 429b90c19939355dae480de1556e73243426a091 Mon Sep 17 00:00:00 2001 From: Josh Justice Date: Mon, 8 May 2023 06:09:13 -0400 Subject: [PATCH 7/9] fix: failing test --- tests/index.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/index.test.ts b/tests/index.test.ts index 6788a479..b3b535b2 100644 --- a/tests/index.test.ts +++ b/tests/index.test.ts @@ -3,7 +3,7 @@ import { resolve } from 'path'; import plugin from '../lib'; -const numberOfRules = 27; +const numberOfRules = 28; const ruleNames = Object.keys(plugin.rules); // eslint-disable-next-line jest/expect-expect From d0b9e6c508bb2409ace1ced0c91aac220b5bd9dd Mon Sep 17 00:00:00 2001 From: Josh Justice Date: Tue, 9 May 2023 14:27:04 -0400 Subject: [PATCH 8/9] fix: make generated tests more realistic AST --- tests/lib/rules/prefer-query-matchers.test.ts | 49 +++++++++++-------- 1 file changed, 29 insertions(+), 20 deletions(-) diff --git a/tests/lib/rules/prefer-query-matchers.test.ts b/tests/lib/rules/prefer-query-matchers.test.ts index 879805e2..98567c88 100644 --- a/tests/lib/rules/prefer-query-matchers.test.ts +++ b/tests/lib/rules/prefer-query-matchers.test.ts @@ -26,30 +26,39 @@ type AssertionFnParams = { options: Options; }; +const wrapExpectInTest = (expectStatement: string) => ` +import { render, screen } from '@testing-library/react' + +test('a fake test', () => { + render() + + ${expectStatement} +})`; + const getValidAssertions = ({ query, matcher, options, }: AssertionFnParams): RuleValidTestCase[] => { - const code = `expect(${query}('Hello'))${matcher}`; - const screenCode = `expect(screen.${query}('Hello'))${matcher}`; + const expectStatement = `expect(${query}('Hello'))${matcher}`; + const expectScreenStatement = `expect(screen.${query}('Hello'))${matcher}`; return [ { - name: `${code} with default options of empty validEntries`, - code, + name: `${expectStatement} with default options of empty validEntries`, + code: wrapExpectInTest(expectStatement), }, { - name: `${code} with provided options`, - code, + name: `${expectStatement} with provided options`, + code: wrapExpectInTest(expectStatement), options, }, { - name: `${screenCode} with default options of empty validEntries`, - code: screenCode, + name: `${expectScreenStatement} with default options of empty validEntries`, + code: wrapExpectInTest(expectScreenStatement), }, { - name: `${screenCode} with provided options`, - code: screenCode, + name: `${expectScreenStatement} with provided options`, + code: wrapExpectInTest(expectScreenStatement), options, }, ]; @@ -60,8 +69,8 @@ const getInvalidAssertions = ({ matcher, options, }: AssertionFnParams): RuleInvalidTestCase[] => { - const code = `expect(${query}('Hello'))${matcher}`; - const screenCode = `expect(screen.${query}('Hello'))${matcher}`; + const expectStatement = `expect(${query}('Hello'))${matcher}`; + const expectScreenStatement = `expect(screen.${query}('Hello'))${matcher}`; const messageId: MessageIds = 'wrongQueryForMatcher'; const [ { @@ -70,27 +79,27 @@ const getInvalidAssertions = ({ ] = options; return [ { - name: `${code} with provided options`, - code, + name: `${expectStatement} with provided options`, + code: wrapExpectInTest(expectStatement), options, errors: [ { messageId, - line: 1, - column: 8, + line: 7, + column: 10, data: { query: validEntry.query, matcher: validEntry.matcher }, }, ], }, { - name: `${screenCode} with provided options`, - code: screenCode, + name: `${expectScreenStatement} with provided options`, + code: wrapExpectInTest(expectScreenStatement), options, errors: [ { messageId, - line: 1, - column: 15, + line: 7, + column: 17, data: { query: validEntry.query, matcher: validEntry.matcher }, }, ], From 8073bf9bb80450caa0d1d7ab91b0c30ed739bd43 Mon Sep 17 00:00:00 2001 From: Josh Justice Date: Tue, 9 May 2023 14:45:29 -0400 Subject: [PATCH 9/9] fix: comment out test names --- tests/lib/rules/prefer-query-matchers.test.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/lib/rules/prefer-query-matchers.test.ts b/tests/lib/rules/prefer-query-matchers.test.ts index 98567c88..e16cf983 100644 --- a/tests/lib/rules/prefer-query-matchers.test.ts +++ b/tests/lib/rules/prefer-query-matchers.test.ts @@ -44,20 +44,20 @@ const getValidAssertions = ({ const expectScreenStatement = `expect(screen.${query}('Hello'))${matcher}`; return [ { - name: `${expectStatement} with default options of empty validEntries`, + // name: `${expectStatement} with default options of empty validEntries`, code: wrapExpectInTest(expectStatement), }, { - name: `${expectStatement} with provided options`, + // name: `${expectStatement} with provided options`, code: wrapExpectInTest(expectStatement), options, }, { - name: `${expectScreenStatement} with default options of empty validEntries`, + // name: `${expectScreenStatement} with default options of empty validEntries`, code: wrapExpectInTest(expectScreenStatement), }, { - name: `${expectScreenStatement} with provided options`, + // name: `${expectScreenStatement} with provided options`, code: wrapExpectInTest(expectScreenStatement), options, }, @@ -79,7 +79,7 @@ const getInvalidAssertions = ({ ] = options; return [ { - name: `${expectStatement} with provided options`, + // name: `${expectStatement} with provided options`, code: wrapExpectInTest(expectStatement), options, errors: [ @@ -92,7 +92,7 @@ const getInvalidAssertions = ({ ], }, { - name: `${expectScreenStatement} with provided options`, + // name: `${expectScreenStatement} with provided options`, code: wrapExpectInTest(expectScreenStatement), options, errors: [