From 619b8ebcc555e078e1c24e11f4627adbee2dba81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Wed, 2 Dec 2020 20:13:19 +0100 Subject: [PATCH 01/17] refactor(await-async-utils): create rule with custom creator --- tests/lib/rules/await-async-utils.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/lib/rules/await-async-utils.test.ts b/tests/lib/rules/await-async-utils.test.ts index 506f800f..c39bf486 100644 --- a/tests/lib/rules/await-async-utils.test.ts +++ b/tests/lib/rules/await-async-utils.test.ts @@ -4,6 +4,8 @@ import { ASYNC_UTILS } from '../../../lib/utils'; const ruleTester = createRuleTester(); +// FIXME: add cases for Promise.allSettled + ruleTester.run(RULE_NAME, rule, { valid: [ ...ASYNC_UTILS.map((asyncUtil) => ({ From 0222ddd56018e3b4e86da21a2e4fb8a481a0036e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Fri, 4 Dec 2020 16:40:40 +0100 Subject: [PATCH 02/17] docs(await-fire-event): update description --- README.md | 2 +- docs/rules/await-fire-event.md | 2 +- lib/rules/await-fire-event.ts | 9 ++++++--- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 46643a26..2f5e1636 100644 --- a/README.md +++ b/README.md @@ -129,7 +129,7 @@ To enable this configuration use the `extends` property in your | -------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------- | ----------------------------------------------------------------- | ------------------ | | [await-async-query](docs/rules/await-async-query.md) | Enforce promises from async queries to be handled | ![dom-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | | | [await-async-utils](docs/rules/await-async-utils.md) | Enforce async utils to be awaited properly | ![dom-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][] | | +| [await-fire-event](docs/rules/await-fire-event.md) | Enforce promises from fire event methods to be handled | ![vue-badge][] | | | [consistent-data-testid](docs/rules/consistent-data-testid.md) | Ensure `data-testid` values match a provided regex. | | | | [no-await-sync-events](docs/rules/no-await-sync-events.md) | Disallow unnecessary `await` for sync events | | | | [no-await-sync-query](docs/rules/no-await-sync-query.md) | Disallow unnecessary `await` for sync queries | ![dom-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | | diff --git a/docs/rules/await-fire-event.md b/docs/rules/await-fire-event.md index 1f464de5..2403d266 100644 --- a/docs/rules/await-fire-event.md +++ b/docs/rules/await-fire-event.md @@ -1,4 +1,4 @@ -# Enforce async fire event methods to be awaited (await-fire-event) +# Enforce promises from fire event methods to be handled (await-fire-event) Ensure that promises returned by `fireEvent` methods are awaited properly. diff --git a/lib/rules/await-fire-event.ts b/lib/rules/await-fire-event.ts index 84f7d877..dee72a8d 100644 --- a/lib/rules/await-fire-event.ts +++ b/lib/rules/await-fire-event.ts @@ -7,19 +7,22 @@ import { getDocsUrl } from '../utils'; import { isAwaited, hasChainedThen } from '../node-utils'; export const RULE_NAME = 'await-fire-event'; -export type MessageIds = 'awaitFireEvent'; +export type MessageIds = 'awaitFireEvent' | 'fireEventWrapper'; type Options = []; export default ESLintUtils.RuleCreator(getDocsUrl)({ name: RULE_NAME, meta: { type: 'problem', docs: { - description: 'Enforce async fire event methods to be awaited', + description: 'Enforce promises from fire event methods to be handled', category: 'Best Practices', recommended: false, }, messages: { - awaitFireEvent: 'async `fireEvent.{{ methodName }}` must be awaited', + awaitFireEvent: + 'Promise returned from `fireEvent.{{ methodName }}` must be handled', + fireEventWrapper: + 'Promise returned from `{{ wrapperName }}` wrapper over fire event method must be handled', }, fixable: null, schema: [], From 7fa934d8400f21861481e7206ba05ac0ef3e608a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Fri, 4 Dec 2020 17:37:45 +0100 Subject: [PATCH 03/17] refactor(await-fire-event): create rule with custom creator --- lib/rules/await-fire-event.ts | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/lib/rules/await-fire-event.ts b/lib/rules/await-fire-event.ts index dee72a8d..08280498 100644 --- a/lib/rules/await-fire-event.ts +++ b/lib/rules/await-fire-event.ts @@ -1,15 +1,12 @@ -import { - ESLintUtils, - TSESTree, - ASTUtils, -} from '@typescript-eslint/experimental-utils'; -import { getDocsUrl } from '../utils'; -import { isAwaited, hasChainedThen } from '../node-utils'; +import { ASTUtils, TSESTree } from '@typescript-eslint/experimental-utils'; +import { hasChainedThen, isAwaited } from '../node-utils'; +import { createTestingLibraryRule } from '../create-testing-library-rule'; export const RULE_NAME = 'await-fire-event'; export type MessageIds = 'awaitFireEvent' | 'fireEventWrapper'; type Options = []; -export default ESLintUtils.RuleCreator(getDocsUrl)({ + +export default createTestingLibraryRule({ name: RULE_NAME, meta: { type: 'problem', From 15328040709376996ce49f2ad8eb0f8a77aa52fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Fri, 4 Dec 2020 18:04:59 +0100 Subject: [PATCH 04/17] refactor(await-fire-event): replace manual promise checks by util --- lib/node-utils.ts | 2 +- lib/rules/await-fire-event.ts | 5 ++--- tests/lib/rules/await-fire-event.test.ts | 7 +++++++ 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/lib/node-utils.ts b/lib/node-utils.ts index 9e0168cb..1664b6b6 100644 --- a/lib/node-utils.ts +++ b/lib/node-utils.ts @@ -253,7 +253,7 @@ export function isPromisesArrayResolved(node: TSESTree.Node): boolean { * - it belongs to the `Promise.allSettled` method * - it's chained with the `then` method * - it's returned from a function - * - has `resolves` or `rejects` + * - has `resolves` or `rejects` jest methods */ export function isPromiseHandled(nodeIdentifier: TSESTree.Identifier): boolean { const closestCallExpressionNode = findClosestCallExpressionNode( diff --git a/lib/rules/await-fire-event.ts b/lib/rules/await-fire-event.ts index 08280498..411cde06 100644 --- a/lib/rules/await-fire-event.ts +++ b/lib/rules/await-fire-event.ts @@ -1,5 +1,5 @@ import { ASTUtils, TSESTree } from '@typescript-eslint/experimental-utils'; -import { hasChainedThen, isAwaited } from '../node-utils'; +import { isPromiseHandled } from '../node-utils'; import { createTestingLibraryRule } from '../create-testing-library-rule'; export const RULE_NAME = 'await-fire-event'; @@ -36,8 +36,7 @@ export default createTestingLibraryRule({ if ( ASTUtils.isIdentifier(fireEventMethodNode) && - !isAwaited(node.parent.parent.parent) && - !hasChainedThen(fireEventMethodNode.parent) + !isPromiseHandled(node) ) { context.report({ node: fireEventMethodNode, diff --git a/tests/lib/rules/await-fire-event.test.ts b/tests/lib/rules/await-fire-event.test.ts index dda5ed5f..65f9bc99 100644 --- a/tests/lib/rules/await-fire-event.test.ts +++ b/tests/lib/rules/await-fire-event.test.ts @@ -3,6 +3,13 @@ import rule, { RULE_NAME } from '../../../lib/rules/await-fire-event'; const ruleTester = createRuleTester(); +// TODO: add cases for fireEvent method destructured +// TODO: add import statement to all current cases +// TODO: add cases for fireEvent wildcard imported +// TODO: add cases for fireEvent method promise returned from function +// TODO: add cases for fireEvent not coming from TL +// TODO: iterate cases for more fireEvent method variants + ruleTester.run(RULE_NAME, rule, { valid: [ { From 4e0a897ae8e6898ebfae5283213b19df7f2f5986 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Sat, 5 Dec 2020 14:23:48 +0100 Subject: [PATCH 05/17] refactor: simplify isNodeComingFromTestingLibrary --- lib/detect-testing-library-utils.ts | 62 ++++++++--------------------- 1 file changed, 17 insertions(+), 45 deletions(-) diff --git a/lib/detect-testing-library-utils.ts b/lib/detect-testing-library-utils.ts index 580a7cbb..a606427e 100644 --- a/lib/detect-testing-library-utils.ts +++ b/lib/detect-testing-library-utils.ts @@ -4,16 +4,15 @@ import { TSESTree, } from '@typescript-eslint/experimental-utils'; import { - getImportModuleName, getAssertNodeInfo, - isLiteral, + getImportModuleName, ImportModuleNode, isImportDeclaration, isImportNamespaceSpecifier, isImportSpecifier, + isLiteral, + isMemberExpression, isProperty, - isCallExpression, - isObjectPattern, } from './node-utils'; import { ABSENCE_MATCHERS, ASYNC_UTILS, PRESENCE_MATCHERS } from './utils'; @@ -215,7 +214,8 @@ export function detectTestingLibraryUtils< }; /** - * Gets a string and verifies if it was imported/required by our custom module node + * Gets a string and verifies if it was imported/required by Testing Library + * related module. */ const findImportedUtilSpecifier: DetectionHelpers['findImportedUtilSpecifier'] = ( specifierName @@ -260,52 +260,24 @@ export function detectTestingLibraryUtils< }; /** * Takes a MemberExpression or an Identifier and verifies if its name comes from the import in TL - * @param node a MemberExpression (in "foo.property" it would be property) or an Identifier (it should be provided from a CallExpression, for example "foo()") + * @param node a MemberExpression (in "foo.property" it would be property) or an Identifier */ const isNodeComingFromTestingLibrary: DetectionHelpers['isNodeComingFromTestingLibrary'] = ( node: TSESTree.MemberExpression | TSESTree.Identifier ) => { - const importOrRequire = - getCustomModuleImportNode() ?? getTestingLibraryImportNode(); - if (!importOrRequire) { - return false; - } + let identifierName: string | undefined; + if (ASTUtils.isIdentifier(node)) { - if (isImportDeclaration(importOrRequire)) { - return importOrRequire.specifiers.some( - (s) => isImportSpecifier(s) && s.local.name === node.name - ); - } else { - return ( - ASTUtils.isVariableDeclarator(importOrRequire.parent) && - isObjectPattern(importOrRequire.parent.id) && - importOrRequire.parent.id.properties.some( - (p) => - isProperty(p) && - ASTUtils.isIdentifier(p.key) && - ASTUtils.isIdentifier(p.value) && - p.value.name === node.name - ) - ); - } - } else { - if (!ASTUtils.isIdentifier(node.object)) { - return false; - } - if (isImportDeclaration(importOrRequire)) { - return ( - isImportDeclaration(importOrRequire) && - isImportNamespaceSpecifier(importOrRequire.specifiers[0]) && - node.object.name === importOrRequire.specifiers[0].local.name - ); - } - return ( - isCallExpression(importOrRequire) && - ASTUtils.isVariableDeclarator(importOrRequire.parent) && - ASTUtils.isIdentifier(importOrRequire.parent.id) && - node.object.name === importOrRequire.parent.id.name - ); + identifierName = node.name; + } else if (ASTUtils.isIdentifier(node.object)) { + identifierName = node.object.name; } + + if (!identifierName) { + return; + } + + return !!findImportedUtilSpecifier(identifierName); }; const helpers = { From 4d65e1c1f09d3e0ddecc759d9df9d5f6bfe89e91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Sat, 5 Dec 2020 19:09:22 +0100 Subject: [PATCH 06/17] fix: call findClosestCallExpressionNode recursively keeping args --- lib/node-utils.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/node-utils.ts b/lib/node-utils.ts index 1664b6b6..88722272 100644 --- a/lib/node-utils.ts +++ b/lib/node-utils.ts @@ -27,7 +27,6 @@ const ValidLeftHandSideExpressions = [ AST_NODE_TYPES.ObjectExpression, AST_NODE_TYPES.ObjectPattern, AST_NODE_TYPES.Super, - AST_NODE_TYPES.TemplateLiteral, AST_NODE_TYPES.ThisExpression, AST_NODE_TYPES.TSNullKeyword, AST_NODE_TYPES.TaggedTemplateExpression, @@ -132,7 +131,7 @@ export function findClosestCallExpressionNode( return null; } - return findClosestCallExpressionNode(node.parent); + return findClosestCallExpressionNode(node.parent, shouldRestrictInnerScope); } export function findClosestCallNode( From 16253e55e7a9264c9ecfc07d64651581131bfe4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Sat, 5 Dec 2020 19:10:32 +0100 Subject: [PATCH 07/17] refactor(prefer-user-event): extract fire event helpers --- lib/detect-testing-library-utils.ts | 59 ++++++++++++++++++++++- lib/rules/prefer-user-event.ts | 46 ++++++------------ tests/lib/rules/prefer-user-event.test.ts | 54 +++++++++++++++++++-- 3 files changed, 124 insertions(+), 35 deletions(-) diff --git a/lib/detect-testing-library-utils.ts b/lib/detect-testing-library-utils.ts index a606427e..f78f4e52 100644 --- a/lib/detect-testing-library-utils.ts +++ b/lib/detect-testing-library-utils.ts @@ -53,6 +53,7 @@ export type DetectionHelpers = { isSyncQuery: (node: TSESTree.Identifier) => boolean; isAsyncQuery: (node: TSESTree.Identifier) => boolean; isAsyncUtil: (node: TSESTree.Identifier) => boolean; + isFireEventMethod: (node: TSESTree.Identifier) => boolean; isPresenceAssert: (node: TSESTree.MemberExpression) => boolean; isAbsenceAssert: (node: TSESTree.MemberExpression) => boolean; canReportErrors: () => boolean; @@ -66,6 +67,8 @@ export type DetectionHelpers = { const DEFAULT_FILENAME_PATTERN = '^.*\\.(test|spec)\\.[jt]sx?$'; +const FIRE_EVENT_NAME = 'fireEvent'; + /** * Enhances a given rule `create` with helpers to detect Testing Library utils. */ @@ -87,6 +90,15 @@ export function detectTestingLibraryUtils< context.settings['testing-library/filename-pattern'] ?? DEFAULT_FILENAME_PATTERN; + /** + * Determines whether aggressive reporting is enabled or not. + * + * Aggressive reporting is considered as enabled when: + * - custom module is not set (so we need to assume everything + * matching TL utils is related to TL no matter where it was imported from) + */ + const isAggressiveReportingEnabled = () => !customModule; + // Helpers for Testing Library detection. const getTestingLibraryImportNode: DetectionHelpers['getTestingLibraryImportNode'] = () => { return importedTestingLibraryNode; @@ -117,7 +129,7 @@ export function detectTestingLibraryUtils< * or custom module are imported. */ const isTestingLibraryImported: DetectionHelpers['isTestingLibraryImported'] = () => { - if (!customModule) { + if (isAggressiveReportingEnabled()) { return true; } @@ -175,6 +187,50 @@ export function detectTestingLibraryUtils< return ASYNC_UTILS.includes(node.name); }; + /** + * Determines whether a given node is fireEvent method or not + * @param node + */ + const isFireEventMethod: DetectionHelpers['isFireEventMethod'] = (node) => { + const fireEventUtil = findImportedUtilSpecifier(FIRE_EVENT_NAME); + let fireEventUtilName: string | undefined; + + if (fireEventUtil) { + fireEventUtilName = ASTUtils.isIdentifier(fireEventUtil) + ? fireEventUtil.name + : fireEventUtil.local.name; + } else if (isAggressiveReportingEnabled()) { + fireEventUtilName = FIRE_EVENT_NAME; + } + + if (!fireEventUtilName) { + return; + } + + const parentMemberExpression: + | TSESTree.MemberExpression + | undefined = isMemberExpression(node.parent) ? node.parent : undefined; + + if (parentMemberExpression) { + // checking fireEvent.click() usage + const regularCall = + ASTUtils.isIdentifier(parentMemberExpression.object) && + parentMemberExpression.object.name === fireEventUtilName; + + // checking testingLibraryUtils.fireEvent.click() usage + const wildcardCall = + isMemberExpression(parentMemberExpression.object) && + ASTUtils.isIdentifier(parentMemberExpression.object.object) && + parentMemberExpression.object.object.name === fireEventUtilName && + ASTUtils.isIdentifier(parentMemberExpression.object.property) && + parentMemberExpression.object.property.name === FIRE_EVENT_NAME; + + return regularCall || wildcardCall; + } + + return false; + }; + /** * Determines whether a given MemberExpression node is a presence assert * @@ -293,6 +349,7 @@ export function detectTestingLibraryUtils< isSyncQuery, isAsyncQuery, isAsyncUtil, + isFireEventMethod, isPresenceAssert, isAbsenceAssert, canReportErrors, diff --git a/lib/rules/prefer-user-event.ts b/lib/rules/prefer-user-event.ts index 0e5fc4f4..7e4b8350 100644 --- a/lib/rules/prefer-user-event.ts +++ b/lib/rules/prefer-user-event.ts @@ -1,6 +1,6 @@ -import { TSESTree, ASTUtils } from '@typescript-eslint/experimental-utils'; +import { TSESTree } from '@typescript-eslint/experimental-utils'; import { createTestingLibraryRule } from '../create-testing-library-rule'; -import { isMemberExpression } from '../node-utils'; +import { findClosestCallExpressionNode } from '../node-utils'; export const RULE_NAME = 'prefer-user-event'; @@ -22,7 +22,7 @@ export const UserEventMethods = [ ] as const; type UserEventMethodsType = typeof UserEventMethods[number]; -// maps fireEvent methods to userEvent. Those not found here, do not have an equivalet (yet) +// maps fireEvent methods to userEvent. Those not found here, do not have an equivalent (yet) export const MappingToUserEvent: Record = { click: ['click', 'type', 'selectOptions', 'deselectOptions'], change: ['upload', 'type', 'clear', 'selectOptions', 'deselectOptions'], @@ -72,7 +72,7 @@ export default createTestingLibraryRule({ }, messages: { preferUserEvent: - 'Prefer using {{userEventMethods}} over {{fireEventMethod}}()', + 'Prefer using {{userEventMethods}} over fireEvent.{{fireEventMethod}}()', }, schema: [ { @@ -88,50 +88,34 @@ export default createTestingLibraryRule({ create(context, [options], helpers) { const { allowedMethods } = options; - const sourceCode = context.getSourceCode(); return { - ['CallExpression > MemberExpression'](node: TSESTree.MemberExpression) { - const util = helpers.findImportedUtilSpecifier('fireEvent'); - if (!util) { - // testing library was imported, but fireEvent was not imported + 'CallExpression Identifier'(node: TSESTree.Identifier) { + if (!helpers.isFireEventMethod(node)) { return; } - const fireEventAliasOrWildcard = ASTUtils.isIdentifier(util) - ? util.name - : util.local.name; - const fireEventUsed = - ASTUtils.isIdentifier(node.object) && - node.object.name === fireEventAliasOrWildcard; + const closestCallExpression = findClosestCallExpressionNode(node, true); - const fireEventFromWildcardUsed = - isMemberExpression(node.object) && - ASTUtils.isIdentifier(node.object.object) && - node.object.object.name === fireEventAliasOrWildcard && - ASTUtils.isIdentifier(node.object.property) && - node.object.property.name === 'fireEvent'; - - if (!fireEventUsed && !fireEventFromWildcardUsed) { - // fireEvent was imported but it was not used + if (!closestCallExpression) { return; } + const fireEventMethodName: string = node.name; + if ( - !ASTUtils.isIdentifier(node.property) || - !fireEventMappedMethods.includes(node.property.name) || - allowedMethods.includes(node.property.name) + !fireEventMappedMethods.includes(fireEventMethodName) || + allowedMethods.includes(fireEventMethodName) ) { - // the fire event does not have an equivalent in userEvent, or it's excluded return; } context.report({ - node, + node: closestCallExpression.callee, messageId: 'preferUserEvent', data: { - userEventMethods: buildErrorMessage(node.property.name), - fireEventMethod: sourceCode.getText(node), + userEventMethods: buildErrorMessage(fireEventMethodName), + fireEventMethod: fireEventMethodName, }, }); }, diff --git a/tests/lib/rules/prefer-user-event.test.ts b/tests/lib/rules/prefer-user-event.test.ts index b1a4158a..171c1968 100644 --- a/tests/lib/rules/prefer-user-event.test.ts +++ b/tests/lib/rules/prefer-user-event.test.ts @@ -16,9 +16,7 @@ function createScenarioWithImport< T extends ValidTestCase | InvalidTestCase >(callback: (libraryModule: string, fireEventMethod: string) => T) { return LIBRARY_MODULES.reduce( - // can't find the right type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (acc: any, libraryModule) => + (acc: Array, libraryModule) => acc.concat( Object.keys(MappingToUserEvent).map((fireEventMethod) => callback(libraryModule, fireEventMethod) @@ -134,6 +132,17 @@ ruleTester.run(RULE_NAME, rule, { userEvent.${userEventMethod}(foo) `, })), + ...Object.keys(MappingToUserEvent).map((fireEventMethod: string) => ({ + settings: { + 'testing-library/module': 'test-utils', + }, + code: ` + // fireEvent method used but not imported from TL related module + // (aggressive reporting opted out) + import { fireEvent } from 'somewhere-else' + fireEvent.${fireEventMethod}(foo) + `, + })), ...Object.keys(MappingToUserEvent).map((fireEventMethod: string) => ({ settings: { 'testing-library/module': 'test-utils', @@ -166,6 +175,15 @@ ruleTester.run(RULE_NAME, rule, { `, options: [{ allowedMethods: [fireEventMethod] }], })), + // edge case for coverage: + // valid use case without call expression + // so there is no innermost function scope found + ` + import { fireEvent } from '@testing-library/react'; + test('edge case for no innermost function scope', () => { + const click = fireEvent.click + }) + `, ], invalid: [ ...createScenarioWithImport>( @@ -231,6 +249,15 @@ ruleTester.run(RULE_NAME, rule, { `, errors: [{ messageId: 'preferUserEvent', line: 3, column: 9 }], })), + ...Object.keys(MappingToUserEvent).map((fireEventMethod: string) => ({ + code: ` + // same as previous group of test cases but without custom module set + // (aggressive reporting) + import { fireEvent } from 'test-utils' + fireEvent.${fireEventMethod}(foo) + `, + errors: [{ messageId: 'preferUserEvent', line: 5, column: 9 }], + })), ...Object.keys(MappingToUserEvent).map((fireEventMethod: string) => ({ settings: { 'testing-library/module': 'test-utils', @@ -241,5 +268,26 @@ ruleTester.run(RULE_NAME, rule, { `, errors: [{ messageId: 'preferUserEvent', line: 3, column: 9 }], })), + { + code: ` // simple test to check error in detail + import { fireEvent } from '@testing-library/react' + + fireEvent.click(element) + `, + errors: [ + { + messageId: 'preferUserEvent', + line: 4, + endLine: 4, + column: 7, + endColumn: 22, + data: { + userEventMethods: + 'userEvent.click(), userEvent.type() or userEvent.deselectOptions()', + fireEventMethod: 'click', + }, + }, + ], + }, ], }); From 66ffe9f585c8cb1cf98573ae6d5404eeca3f02d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Sat, 5 Dec 2020 19:58:12 +0100 Subject: [PATCH 08/17] refactor(await-async-utils): remove unnecessary as expression --- lib/rules/await-async-utils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rules/await-async-utils.ts b/lib/rules/await-async-utils.ts index 75ff5384..c23a8877 100644 --- a/lib/rules/await-async-utils.ts +++ b/lib/rules/await-async-utils.ts @@ -74,7 +74,7 @@ export default createTestingLibraryRule({ ); if (references && references.length === 0) { - if (!isPromiseHandled(node as TSESTree.Identifier)) { + if (!isPromiseHandled(node)) { return context.report({ node, messageId: 'awaitAsyncUtil', From d78cd77895f91dc7dfd5120420784bb28d0692ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Sun, 6 Dec 2020 12:03:52 +0100 Subject: [PATCH 09/17] refactor(await-fire-event): reuse fire event detection helpers --- lib/detect-testing-library-utils.ts | 43 ++-- lib/rules/await-fire-event.ts | 69 +++-- tests/lib/rules/await-fire-event.test.ts | 309 ++++++++++++++++++----- 3 files changed, 314 insertions(+), 107 deletions(-) diff --git a/lib/detect-testing-library-utils.ts b/lib/detect-testing-library-utils.ts index f78f4e52..2363ae8b 100644 --- a/lib/detect-testing-library-utils.ts +++ b/lib/detect-testing-library-utils.ts @@ -189,7 +189,6 @@ export function detectTestingLibraryUtils< /** * Determines whether a given node is fireEvent method or not - * @param node */ const isFireEventMethod: DetectionHelpers['isFireEventMethod'] = (node) => { const fireEventUtil = findImportedUtilSpecifier(FIRE_EVENT_NAME); @@ -204,31 +203,39 @@ export function detectTestingLibraryUtils< } if (!fireEventUtilName) { - return; + return false; } const parentMemberExpression: | TSESTree.MemberExpression | undefined = isMemberExpression(node.parent) ? node.parent : undefined; - if (parentMemberExpression) { - // checking fireEvent.click() usage - const regularCall = - ASTUtils.isIdentifier(parentMemberExpression.object) && - parentMemberExpression.object.name === fireEventUtilName; - - // checking testingLibraryUtils.fireEvent.click() usage - const wildcardCall = - isMemberExpression(parentMemberExpression.object) && - ASTUtils.isIdentifier(parentMemberExpression.object.object) && - parentMemberExpression.object.object.name === fireEventUtilName && - ASTUtils.isIdentifier(parentMemberExpression.object.property) && - parentMemberExpression.object.property.name === FIRE_EVENT_NAME; - - return regularCall || wildcardCall; + if (!parentMemberExpression) { + return false; } - return false; + // make sure that given node it's not fireEvent util itself + if ( + ASTUtils.isIdentifier(parentMemberExpression.object) && + parentMemberExpression.object.name === node.name + ) { + return false; + } + + // check fireEvent.click() usage + const regularCall = + ASTUtils.isIdentifier(parentMemberExpression.object) && + parentMemberExpression.object.name === fireEventUtilName; + + // check testingLibraryUtils.fireEvent.click() usage + const wildcardCall = + isMemberExpression(parentMemberExpression.object) && + ASTUtils.isIdentifier(parentMemberExpression.object.object) && + parentMemberExpression.object.object.name === fireEventUtilName && + ASTUtils.isIdentifier(parentMemberExpression.object.property) && + parentMemberExpression.object.property.name === FIRE_EVENT_NAME; + + return regularCall || wildcardCall; }; /** diff --git a/lib/rules/await-fire-event.ts b/lib/rules/await-fire-event.ts index 411cde06..859a019c 100644 --- a/lib/rules/await-fire-event.ts +++ b/lib/rules/await-fire-event.ts @@ -1,5 +1,9 @@ -import { ASTUtils, TSESTree } from '@typescript-eslint/experimental-utils'; -import { isPromiseHandled } from '../node-utils'; +import { TSESTree } from '@typescript-eslint/experimental-utils'; +import { + findClosestCallExpressionNode, + getVariableReferences, + isPromiseHandled, +} from '../node-utils'; import { createTestingLibraryRule } from '../create-testing-library-rule'; export const RULE_NAME = 'await-fire-event'; @@ -19,32 +23,55 @@ export default createTestingLibraryRule({ awaitFireEvent: 'Promise returned from `fireEvent.{{ methodName }}` must be handled', fireEventWrapper: - 'Promise returned from `{{ wrapperName }}` wrapper over fire event method must be handled', + 'Promise returned from `fireEvent.{{ wrapperName }}` wrapper over fire event method must be handled', }, fixable: null, schema: [], }, defaultOptions: [], - create: function (context) { + create: function (context, _, helpers) { + function reportUnhandledNode( + node: TSESTree.Identifier, + closestCallExpressionNode: TSESTree.CallExpression, + messageId: MessageIds = 'awaitFireEvent' + ): void { + if (!isPromiseHandled(node)) { + context.report({ + node: closestCallExpressionNode.callee, + messageId, + data: { name: node.name }, + }); + } + } + return { - 'CallExpression > MemberExpression > Identifier[name=fireEvent]'( - node: TSESTree.Identifier - ) { - const memberExpression = node.parent as TSESTree.MemberExpression; - const fireEventMethodNode = memberExpression.property; - - if ( - ASTUtils.isIdentifier(fireEventMethodNode) && - !isPromiseHandled(node) - ) { - context.report({ - node: fireEventMethodNode, - messageId: 'awaitFireEvent', - data: { - methodName: fireEventMethodNode.name, - }, - }); + 'CallExpression Identifier'(node: TSESTree.Identifier) { + if (helpers.isFireEventMethod(node)) { + // TODO: detectFireEventMethodWrapper + + const closestCallExpression = findClosestCallExpressionNode( + node, + true + ); + + if (!closestCallExpression) { + return; + } + + const references = getVariableReferences( + context, + closestCallExpression.parent + ); + + if (!references || references.length === 0) { + return reportUnhandledNode(node, closestCallExpression); + } else { + for (const reference of references) { + const referenceNode = reference.identifier as TSESTree.Identifier; + return reportUnhandledNode(referenceNode, closestCallExpression); + } + } } }, }; diff --git a/tests/lib/rules/await-fire-event.test.ts b/tests/lib/rules/await-fire-event.test.ts index 65f9bc99..de7b1b2d 100644 --- a/tests/lib/rules/await-fire-event.test.ts +++ b/tests/lib/rules/await-fire-event.test.ts @@ -3,95 +3,268 @@ import rule, { RULE_NAME } from '../../../lib/rules/await-fire-event'; const ruleTester = createRuleTester(); -// TODO: add cases for fireEvent method destructured -// TODO: add import statement to all current cases -// TODO: add cases for fireEvent wildcard imported -// TODO: add cases for fireEvent method promise returned from function -// TODO: add cases for fireEvent not coming from TL -// TODO: iterate cases for more fireEvent method variants +const COMMON_FIRE_EVENT_METHODS: string[] = [ + 'click', + 'change', + 'focus', + 'blur', + 'keyDown', +]; ruleTester.run(RULE_NAME, rule, { valid: [ - { - code: `fireEvent.click`, - }, - { - code: `async () => { - await fireEvent.click(getByText('Click me')) - } - `, - }, - { - code: `async () => { - await fireEvent.focus(getByLabelText('username')) - await fireEvent.blur(getByLabelText('username')) - } - `, - }, - { - code: `done => { - fireEvent.click(getByText('Click me')).then(() => { done() }) - } - `, - }, - { - code: `done => { - fireEvent.focus(getByLabelText('username')).then(() => { - fireEvent.blur(getByLabelText('username')).then(() => { done() }) + ...COMMON_FIRE_EVENT_METHODS.map((fireEventMethod) => ({ + code: ` + import { fireEvent } from '@testing-library/vue' + test('fire event method not called is valid', () => { + fireEvent.${fireEventMethod} + }) + `, + })), + ...COMMON_FIRE_EVENT_METHODS.map((fireEventMethod) => ({ + code: ` + import { fireEvent } from '@testing-library/vue' + test('await promise from fire event method is valid', async () => { + await fireEvent.${fireEventMethod}(getByLabelText('username')) + }) + `, + })), + ...COMMON_FIRE_EVENT_METHODS.map((fireEventMethod) => ({ + code: ` + import { fireEvent } from '@testing-library/vue' + test('await several promises from fire event methods is valid', async () => { + await fireEvent.${fireEventMethod}(getByLabelText('username')) + await fireEvent.${fireEventMethod}(getByLabelText('username')) + }) + `, + })), + ...COMMON_FIRE_EVENT_METHODS.map((fireEventMethod) => ({ + code: ` + import { fireEvent } from '@testing-library/vue' + test('await promise kept in a var from fire event method is valid', async () => { + const promise = fireEvent.${fireEventMethod}(getByLabelText('username')) + await promise + }) + `, + })), + ...COMMON_FIRE_EVENT_METHODS.map((fireEventMethod) => ({ + code: ` + import { fireEvent } from '@testing-library/vue' + test('chain then method to promise from fire event method is valid', async (done) => { + fireEvent.${fireEventMethod}(getByLabelText('username')) + .then(() => { done() }) + }) + `, + })), + ...COMMON_FIRE_EVENT_METHODS.map((fireEventMethod) => ({ + code: ` + import { fireEvent } from '@testing-library/vue' + test('chain then method to several promises from fire event methods is valid', async (done) => { + fireEvent.${fireEventMethod}(getByLabelText('username')).then(() => { + fireEvent.${fireEventMethod}(getByLabelText('username')).then(() => { done() }) }) - } - `, - }, - { - code: `() => { - return fireEvent.click(getByText('Click me')) - } - `, - }, - { - code: `() => fireEvent.click(getByText('Click me')) - `, - }, - { - code: `function clickUtil() { - doSomething() - return fireEvent.click(getByText('Click me')) - } - `, - }, + }) + `, + })), + // TODO: this one should be valid + // `import { fireEvent } from '@testing-library/vue' + // + // test('fireEvent methods wrapped with Promise.all are valid', async () => { + // await Promise.all([ + // fireEvent.blur(getByText('Click me')), + // fireEvent.click(getByText('Click me')), + // ]) + // }) + // `, + ...COMMON_FIRE_EVENT_METHODS.map((fireEventMethod) => ({ + code: ` + import { fireEvent } from '@testing-library/vue' + test('return promise from fire event methods is valid', () => { + function triggerEvent() { + doSomething() + return fireEvent.${fireEventMethod}(getByLabelText('username')) + } + }) + `, + })), + ...COMMON_FIRE_EVENT_METHODS.map((fireEventMethod) => ({ + code: ` + import { fireEvent } from '@testing-library/vue' + test('await promise returned from function wrapping fire event method is valid', () => { + function triggerEvent() { + doSomething() + return fireEvent.${fireEventMethod}(getByLabelText('username')) + } + + await triggerEvent() + }) + `, + })), + ...COMMON_FIRE_EVENT_METHODS.map((fireEventMethod) => ({ + settings: { + 'testing-library/module': 'test-utils', + }, + code: ` + import { fireEvent } from 'somewhere-else' + test('unhandled promise from fire event not related to TL is valid', async () => { + fireEvent.${fireEventMethod}(getByLabelText('username')) + }) + `, + })), + ...COMMON_FIRE_EVENT_METHODS.map((fireEventMethod) => ({ + settings: { + 'testing-library/module': 'test-utils', + }, + code: ` + import { fireEvent } from 'test-utils' + test('await promise from fire event method imported from custom module is valid', async () => { + await fireEvent.${fireEventMethod}(getByLabelText('username')) + }) + `, + })), ], invalid: [ - { - code: `() => { - fireEvent.click(getByText('Click me')) - } + ...COMMON_FIRE_EVENT_METHODS.map((fireEventMethod) => ({ + code: ` + import { fireEvent } from '@testing-library/vue' + test('unhandled promise from fire event method is invalid', async () => { + fireEvent.${fireEventMethod}(getByLabelText('username')) + }) `, errors: [ { - column: 19, + line: 4, + column: 9, messageId: 'awaitFireEvent', + data: { name: fireEventMethod }, }, ], - }, - { - code: `() => { - fireEvent.focus(getByLabelText('username')) - fireEvent.blur(getByLabelText('username')) - } + })), + ...COMMON_FIRE_EVENT_METHODS.map((fireEventMethod) => ({ + code: ` + import { fireEvent } from '@testing-library/vue' + test('several unhandled promises from fire event methods is invalid', async () => { + fireEvent.${fireEventMethod}(getByLabelText('username')) + fireEvent.${fireEventMethod}(getByLabelText('username')) + }) `, errors: [ { - line: 2, - column: 19, + line: 4, + column: 9, messageId: 'awaitFireEvent', + data: { name: fireEventMethod }, }, { - line: 3, - column: 19, + line: 5, + column: 9, + messageId: 'awaitFireEvent', + data: { name: fireEventMethod }, + }, + ], + })), + ...COMMON_FIRE_EVENT_METHODS.map((fireEventMethod) => ({ + settings: { + 'testing-library/module': 'test-utils', + }, + code: ` + import { fireEvent } from '@testing-library/vue' + test('unhandled promise from fire event method with aggressive reporting opted-out is invalid', async () => { + fireEvent.${fireEventMethod}(getByLabelText('username')) + }) + `, + errors: [ + { + line: 4, + column: 9, + messageId: 'awaitFireEvent', + data: { name: fireEventMethod }, + }, + ], + })), + ...COMMON_FIRE_EVENT_METHODS.map((fireEventMethod) => ({ + settings: { + 'testing-library/module': 'test-utils', + }, + code: ` + import { fireEvent } from 'test-utils' + test( + 'unhandled promise from fire event method imported from custom module with aggressive reporting opted-out is invalid', + () => { + fireEvent.${fireEventMethod}(getByLabelText('username')) + }) + `, + errors: [ + { + line: 6, + column: 9, + messageId: 'awaitFireEvent', + data: { name: fireEventMethod }, + }, + ], + })), + ...COMMON_FIRE_EVENT_METHODS.map((fireEventMethod) => ({ + settings: { + 'testing-library/module': 'test-utils', + }, + code: ` + import { fireEvent } from '@testing-library/vue' + test( + 'unhandled promise from fire event method imported from default module with aggressive reporting opted-out is invalid', + () => { + fireEvent.${fireEventMethod}(getByLabelText('username')) + }) + `, + errors: [ + { + line: 6, + column: 9, + messageId: 'awaitFireEvent', + data: { name: fireEventMethod }, + }, + ], + })), + + ...COMMON_FIRE_EVENT_METHODS.map((fireEventMethod) => ({ + code: ` + import { fireEvent } from '@testing-library/vue' + test( + 'unhandled promise from fire event method kept in a var is invalid', + () => { + const promise = fireEvent.${fireEventMethod}(getByLabelText('username')) + }) + `, + errors: [ + { + line: 6, + column: 25, messageId: 'awaitFireEvent', + data: { name: fireEventMethod }, }, ], - }, + })), + // TODO: enable this one in next block of features + // ...COMMON_FIRE_EVENT_METHODS.map((fireEventMethod) => ({ + // code: ` + // import { fireEvent } from '@testing-library/vue' + // test('unhandled promise returned from function wrapping fire event method is invalid', () => { + // function triggerEvent() { + // doSomething() + // return fireEvent.${fireEventMethod}(getByLabelText('username')) + // } + // + // triggerEvent() + // }) + // `, + // errors: [ + // { + // line: 9, + // column: 9, + // messageId: 'fireEventWrapper', + // data: { name: fireEventMethod }, + // }, + // ], + // })), ], }); From 1b2110669602aca8ec4877c23cb039a679fa8a84 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Sun, 6 Dec 2020 12:46:28 +0100 Subject: [PATCH 10/17] feat(await-fire-event): detect functions wrapping fire event methods --- lib/rules/await-fire-event.ts | 31 ++++++++++++++++- tests/lib/rules/await-fire-event.test.ts | 43 ++++++++++++------------ 2 files changed, 51 insertions(+), 23 deletions(-) diff --git a/lib/rules/await-fire-event.ts b/lib/rules/await-fire-event.ts index 859a019c..85a8541a 100644 --- a/lib/rules/await-fire-event.ts +++ b/lib/rules/await-fire-event.ts @@ -1,6 +1,8 @@ import { TSESTree } from '@typescript-eslint/experimental-utils'; import { findClosestCallExpressionNode, + getFunctionName, + getInnermostReturningFunction, getVariableReferences, isPromiseHandled, } from '../node-utils'; @@ -31,6 +33,8 @@ export default createTestingLibraryRule({ defaultOptions: [], create: function (context, _, helpers) { + const functionWrappersNames: string[] = []; + function reportUnhandledNode( node: TSESTree.Identifier, closestCallExpressionNode: TSESTree.CallExpression, @@ -45,10 +49,18 @@ export default createTestingLibraryRule({ } } + function detectFireEventMethodWrapper(node: TSESTree.Identifier): void { + const innerFunction = getInnermostReturningFunction(context, node); + + if (innerFunction) { + functionWrappersNames.push(getFunctionName(innerFunction)); + } + } + return { 'CallExpression Identifier'(node: TSESTree.Identifier) { if (helpers.isFireEventMethod(node)) { - // TODO: detectFireEventMethodWrapper + detectFireEventMethodWrapper(node); const closestCallExpression = findClosestCallExpressionNode( node, @@ -72,6 +84,23 @@ export default createTestingLibraryRule({ return reportUnhandledNode(referenceNode, closestCallExpression); } } + } else if (functionWrappersNames.includes(node.name)) { + // report promise returned from function wrapping fire event method + // previously detected + const closestCallExpression = findClosestCallExpressionNode( + node, + true + ); + + if (!closestCallExpression) { + return; + } + + return reportUnhandledNode( + node, + closestCallExpression, + 'fireEventWrapper' + ); } }, }; diff --git a/tests/lib/rules/await-fire-event.test.ts b/tests/lib/rules/await-fire-event.test.ts index de7b1b2d..3d8c53a1 100644 --- a/tests/lib/rules/await-fire-event.test.ts +++ b/tests/lib/rules/await-fire-event.test.ts @@ -244,27 +244,26 @@ ruleTester.run(RULE_NAME, rule, { }, ], })), - // TODO: enable this one in next block of features - // ...COMMON_FIRE_EVENT_METHODS.map((fireEventMethod) => ({ - // code: ` - // import { fireEvent } from '@testing-library/vue' - // test('unhandled promise returned from function wrapping fire event method is invalid', () => { - // function triggerEvent() { - // doSomething() - // return fireEvent.${fireEventMethod}(getByLabelText('username')) - // } - // - // triggerEvent() - // }) - // `, - // errors: [ - // { - // line: 9, - // column: 9, - // messageId: 'fireEventWrapper', - // data: { name: fireEventMethod }, - // }, - // ], - // })), + ...COMMON_FIRE_EVENT_METHODS.map((fireEventMethod) => ({ + code: ` + import { fireEvent } from '@testing-library/vue' + test('unhandled promise returned from function wrapping fire event method is invalid', () => { + function triggerEvent() { + doSomething() + return fireEvent.${fireEventMethod}(getByLabelText('username')) + } + + triggerEvent() + }) + `, + errors: [ + { + line: 9, + column: 9, + messageId: 'fireEventWrapper', + data: { name: fireEventMethod }, + }, + ], + })), ], }); From f9b84f7250ffb57cca7bd24a9eeb8926bad5bdb1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Sun, 6 Dec 2020 13:12:36 +0100 Subject: [PATCH 11/17] fix(await-fire-event): detect more cases --- lib/detect-testing-library-utils.ts | 7 +-- lib/node-utils.ts | 19 ++++++--- tests/lib/rules/await-fire-event.test.ts | 54 +++++++++++++++++++----- 3 files changed, 61 insertions(+), 19 deletions(-) diff --git a/lib/detect-testing-library-utils.ts b/lib/detect-testing-library-utils.ts index 2363ae8b..f3066304 100644 --- a/lib/detect-testing-library-utils.ts +++ b/lib/detect-testing-library-utils.ts @@ -214,10 +214,11 @@ export function detectTestingLibraryUtils< return false; } - // make sure that given node it's not fireEvent util itself + // make sure that given node it's not fireEvent object itself if ( - ASTUtils.isIdentifier(parentMemberExpression.object) && - parentMemberExpression.object.name === node.name + [fireEventUtilName, FIRE_EVENT_NAME].includes(node.name) || + (ASTUtils.isIdentifier(parentMemberExpression.object) && + parentMemberExpression.object.name === node.name) ) { return false; } diff --git a/lib/node-utils.ts b/lib/node-utils.ts index 88722272..fcbc2f14 100644 --- a/lib/node-utils.ts +++ b/lib/node-utils.ts @@ -231,15 +231,22 @@ export function isPromiseAllSettled(node: TSESTree.CallExpression): boolean { ); } +/** + * Determines whether a given node belongs to handled Promise.all or Promise.allSettled + * array expression. + */ export function isPromisesArrayResolved(node: TSESTree.Node): boolean { - const parent = node.parent; + const closestCallExpression = findClosestCallExpressionNode(node, true); + + if (!closestCallExpression) { + return false; + } return ( - isCallExpression(parent) && - isArrayExpression(parent.parent) && - isCallExpression(parent.parent.parent) && - (isPromiseAll(parent.parent.parent) || - isPromiseAllSettled(parent.parent.parent)) + isArrayExpression(closestCallExpression.parent) && + isCallExpression(closestCallExpression.parent.parent) && + (isPromiseAll(closestCallExpression.parent.parent) || + isPromiseAllSettled(closestCallExpression.parent.parent)) ); } diff --git a/tests/lib/rules/await-fire-event.test.ts b/tests/lib/rules/await-fire-event.test.ts index 3d8c53a1..4746848d 100644 --- a/tests/lib/rules/await-fire-event.test.ts +++ b/tests/lib/rules/await-fire-event.test.ts @@ -66,16 +66,15 @@ ruleTester.run(RULE_NAME, rule, { }) `, })), - // TODO: this one should be valid - // `import { fireEvent } from '@testing-library/vue' - // - // test('fireEvent methods wrapped with Promise.all are valid', async () => { - // await Promise.all([ - // fireEvent.blur(getByText('Click me')), - // fireEvent.click(getByText('Click me')), - // ]) - // }) - // `, + `import { fireEvent } from '@testing-library/vue' + + test('fireEvent methods wrapped with Promise.all are valid', async () => { + await Promise.all([ + fireEvent.blur(getByText('Click me')), + fireEvent.click(getByText('Click me')), + ]) + }) + `, ...COMMON_FIRE_EVENT_METHODS.map((fireEventMethod) => ({ code: ` import { fireEvent } from '@testing-library/vue' @@ -136,6 +135,41 @@ ruleTester.run(RULE_NAME, rule, { { line: 4, column: 9, + endColumn: 19 + fireEventMethod.length, + messageId: 'awaitFireEvent', + data: { name: fireEventMethod }, + }, + ], + })), + ...COMMON_FIRE_EVENT_METHODS.map((fireEventMethod) => ({ + code: ` + import { fireEvent as testingLibraryFireEvent } from '@testing-library/vue' + test('unhandled promise from aliased fire event method is invalid', async () => { + testingLibraryFireEvent.${fireEventMethod}(getByLabelText('username')) + }) + `, + errors: [ + { + line: 4, + column: 9, + endColumn: 33 + fireEventMethod.length, + messageId: 'awaitFireEvent', + data: { name: fireEventMethod }, + }, + ], + })), + ...COMMON_FIRE_EVENT_METHODS.map((fireEventMethod) => ({ + code: ` + import * as testingLibrary from '@testing-library/vue' + test('unhandled promise from wildcard imported fire event method is invalid', async () => { + testingLibrary.fireEvent.${fireEventMethod}(getByLabelText('username')) + }) + `, + errors: [ + { + line: 4, + column: 9, + endColumn: 34 + fireEventMethod.length, messageId: 'awaitFireEvent', data: { name: fireEventMethod }, }, From 825cbe564df356c9f5c1dbc8f7d6d1116a1aaa77 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Sun, 6 Dec 2020 13:24:45 +0100 Subject: [PATCH 12/17] test(await-fire-event): increase coverage --- tests/lib/rules/await-fire-event.test.ts | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/lib/rules/await-fire-event.test.ts b/tests/lib/rules/await-fire-event.test.ts index 4746848d..2b62147a 100644 --- a/tests/lib/rules/await-fire-event.test.ts +++ b/tests/lib/rules/await-fire-event.test.ts @@ -121,6 +121,21 @@ ruleTester.run(RULE_NAME, rule, { }) `, })), + + // edge case for coverage: + // valid use case without call expression + // so there is no innermost function scope found + ` + import { fireEvent } from 'test-utils' + test('edge case for innermost function without call expression', async () => { + function triggerEvent() { + doSomething() + return fireEvent.focus(getByLabelText('username')) + } + + const reassignedFunction = triggerEvent + }) + `, ], invalid: [ From cc33199bac3c42f8c2327f4f747e1c874c3bb033 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Sun, 6 Dec 2020 13:39:30 +0100 Subject: [PATCH 13/17] docs(await-fire-event): update rule details and examples --- docs/rules/await-fire-event.md | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/docs/rules/await-fire-event.md b/docs/rules/await-fire-event.md index 2403d266..02805110 100644 --- a/docs/rules/await-fire-event.md +++ b/docs/rules/await-fire-event.md @@ -1,12 +1,14 @@ # Enforce promises from fire event methods to be handled (await-fire-event) -Ensure that promises returned by `fireEvent` methods are awaited +Ensure that promises returned by `fireEvent` methods are handled properly. ## Rule Details -This rule aims to prevent users from forgetting to await `fireEvent` -methods when they are async. +This rule aims to prevent users from forgetting to handle promise returned from `fireEvent` +methods. + +> ⚠️ `fireEvent` methods are async only on `@testing-library/vue` package Examples of **incorrect** code for this rule: @@ -15,6 +17,12 @@ fireEvent.click(getByText('Click me')); fireEvent.focus(getByLabelText('username')); fireEvent.blur(getByLabelText('username')); + +// wrap a fireEvent method within a function... +function triggerEvent() { + return fireEvent.click(button); +} +triggerEvent(); // ...but not handling promise from it is incorrect too ``` Examples of **correct** code for this rule: @@ -30,10 +38,19 @@ fireEvent.click(getByText('Click me')).then(() => { }); // return the promise within a function is correct too! -function clickMeRegularFn() { - return fireEvent.click(getByText('Click me')); -} const clickMeArrowFn = () => fireEvent.click(getByText('Click me')); + +// wrap a fireEvent method within a function... +function triggerEvent() { + return fireEvent.click(button); +} +await triggerEvent(); // ...and handling promise from it is correct also + +// using `Promise.all` or `Promise.allSettled` with an array of promises is valid +await Promise.all([ + fireEvent.focus(getByLabelText('username')), + fireEvent.blur(getByLabelText('username')), +]); ``` ## When Not To Use It From ab6da9caeadb2006a713f41f22772ba249222caf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Sun, 6 Dec 2020 14:17:44 +0100 Subject: [PATCH 14/17] test(await-async-utils): remove outdated comment --- tests/lib/rules/await-async-utils.test.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/lib/rules/await-async-utils.test.ts b/tests/lib/rules/await-async-utils.test.ts index c39bf486..506f800f 100644 --- a/tests/lib/rules/await-async-utils.test.ts +++ b/tests/lib/rules/await-async-utils.test.ts @@ -4,8 +4,6 @@ import { ASYNC_UTILS } from '../../../lib/utils'; const ruleTester = createRuleTester(); -// FIXME: add cases for Promise.allSettled - ruleTester.run(RULE_NAME, rule, { valid: [ ...ASYNC_UTILS.map((asyncUtil) => ({ From 20174d01757da4497751b73dd67c74964121191a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltr=C3=A1n=20Alarc=C3=B3n?= Date: Mon, 7 Dec 2020 09:19:12 +0100 Subject: [PATCH 15/17] docs(await-fire-event): update async note --- docs/rules/await-fire-event.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/rules/await-fire-event.md b/docs/rules/await-fire-event.md index 02805110..5ccff00c 100644 --- a/docs/rules/await-fire-event.md +++ b/docs/rules/await-fire-event.md @@ -8,7 +8,9 @@ properly. This rule aims to prevent users from forgetting to handle promise returned from `fireEvent` methods. -> ⚠️ `fireEvent` methods are async only on `@testing-library/vue` package +> ⚠️ `fireEvent` methods are async only on following Testing Library packages: +> - `@testing-library/vue` (supported by this plugin) +> - `@testing-library/svelte` (not supported yet by this plugin) Examples of **incorrect** code for this rule: @@ -55,7 +57,7 @@ await Promise.all([ ## When Not To Use It -`fireEvent` methods are only async in Vue Testing Library so if you are using another Testing Library module, you shouldn't use this rule. +`fireEvent` methods are not async on all Testing Library packages. If you are not using Testing Library package with async fire event, you shouldn't use this rule. ## Further Reading From 2047b43a10882faa479ecb3adff198e520000e01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Mon, 7 Dec 2020 10:36:59 +0100 Subject: [PATCH 16/17] style(await-fire-event): format rule doc --- docs/rules/await-fire-event.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/rules/await-fire-event.md b/docs/rules/await-fire-event.md index 5ccff00c..cff4509c 100644 --- a/docs/rules/await-fire-event.md +++ b/docs/rules/await-fire-event.md @@ -9,6 +9,7 @@ This rule aims to prevent users from forgetting to handle promise returned from methods. > ⚠️ `fireEvent` methods are async only on following Testing Library packages: +> > - `@testing-library/vue` (supported by this plugin) > - `@testing-library/svelte` (not supported yet by this plugin) From ba1628e81599976f64efdd27d554c2908a67744f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Mon, 7 Dec 2020 10:38:30 +0100 Subject: [PATCH 17/17] refactor(await-fire-event): remove unnecessary check --- lib/rules/await-fire-event.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rules/await-fire-event.ts b/lib/rules/await-fire-event.ts index 85a8541a..c127112a 100644 --- a/lib/rules/await-fire-event.ts +++ b/lib/rules/await-fire-event.ts @@ -76,7 +76,7 @@ export default createTestingLibraryRule({ closestCallExpression.parent ); - if (!references || references.length === 0) { + if (references.length === 0) { return reportUnhandledNode(node, closestCallExpression); } else { for (const reference of references) {