From 957646292734c793e6d2f75b14a4ed265365a884 Mon Sep 17 00:00:00 2001 From: Nick Bolles Date: Thu, 20 Oct 2022 14:29:31 -0500 Subject: [PATCH] feat: support handling promises with jest-extended `.toResolve` & `.toRejects` (#612) * Support jest-extended toResolve and toReject as handling promises * add test cases for toResolve and toReject matchers * feat(await-async-query): support handling promises with jest-extendeds toResolve and toRejects * docs(await-async-query): add docs for toResolve/toReject * Add jest-extended docs * fireEvent.blur(getByLabelText('username')) * run format * add tests for await-async-utils and await-fire-event --- docs/rules/await-async-query.md | 7 +++++ docs/rules/await-async-utils.md | 7 +++++ docs/rules/await-fire-event.md | 14 ++++++++++ lib/node-utils/index.ts | 13 +++++++-- tests/lib/rules/await-async-query.test.ts | 15 +++++++++++ tests/lib/rules/await-async-utils.test.ts | 18 +++++++++++++ tests/lib/rules/await-fire-event.test.ts | 32 +++++++++++++++++++++++ 7 files changed, 104 insertions(+), 2 deletions(-) diff --git a/docs/rules/await-async-query.md b/docs/rules/await-async-query.md index 70e83aaf..608b9477 100644 --- a/docs/rules/await-async-query.md +++ b/docs/rules/await-async-query.md @@ -19,6 +19,7 @@ problems in the tests. The promise will be considered as handled when: - wrapped within `Promise.all` or `Promise.allSettled` methods - chaining the `then` method - chaining `resolves` or `rejects` from jest +- chaining `toResolve()` or `toReject()` from [jest-extended](https://github.com/jest-community/jest-extended#promise) - it's returned from a function (in this case, that particular function will be analyzed by this rule too) Examples of **incorrect** code for this rule: @@ -90,6 +91,12 @@ expect(findByTestId('alert')).resolves.toBe('Success'); expect(findByTestId('alert')).rejects.toBe('Error'); ``` +```js +// using a toResolve/toReject matcher is also correct +expect(findByTestId('alert')).toResolve(); +expect(findByTestId('alert')).toReject(); +``` + ```js // sync queries don't need to handle any promise const element = getByRole('role'); diff --git a/docs/rules/await-async-utils.md b/docs/rules/await-async-utils.md index c17a4cd8..f27f92df 100644 --- a/docs/rules/await-async-utils.md +++ b/docs/rules/await-async-utils.md @@ -20,6 +20,7 @@ problems in the tests. The promise will be considered as handled when: - wrapped within `Promise.all` or `Promise.allSettled` methods - chaining the `then` method - chaining `resolves` or `rejects` from jest +- chaining `toResolve()` or `toReject()` from [jest-extended](https://github.com/jest-community/jest-extended#promise) - it's returned from a function (in this case, that particular function will be analyzed by this rule too) Examples of **incorrect** code for this rule: @@ -85,6 +86,12 @@ test('something correctly', async () => { waitFor(() => getByLabelText('email')), waitForElementToBeRemoved(() => document.querySelector('div.getOuttaHere')), ]); + + // Using jest resolves or rejects + expect(waitFor(() => getByLabelText('email'))).resolves.toBeUndefined(); + + // Using jest-extended a toResolve/toReject matcher is also correct + expect(waitFor(() => getByLabelText('email'))).toResolve(); }); ``` diff --git a/docs/rules/await-fire-event.md b/docs/rules/await-fire-event.md index 54643c60..2a3fa4b0 100644 --- a/docs/rules/await-fire-event.md +++ b/docs/rules/await-fire-event.md @@ -7,6 +7,14 @@ properly. This rule aims to prevent users from forgetting to handle promise returned from `fireEvent` methods. +The promise will be considered as handled when: + +- using the `await` operator +- wrapped within `Promise.all` or `Promise.allSettled` methods +- chaining the `then` method +- chaining `resolves` or `rejects` from jest +- chaining `toResolve()` or `toReject()` from [jest-extended](https://github.com/jest-community/jest-extended#promise) +- it's returned from a function (in this case, that particular function will be analyzed by this rule too) > ⚠️ `fireEvent` methods are async only on following Testing Library packages: > @@ -55,6 +63,12 @@ await Promise.all([ fireEvent.focus(getByLabelText('username')), fireEvent.blur(getByLabelText('username')), ]); + +// Using jest resolves or rejects +expect(fireEvent.focus(getByLabelText('username'))).resolves.toBeUndefined(); + +// Using jest-extended a toResolve/toReject matcher is also correct +expect(waitFor(() => getByLabelText('email'))).toResolve(); ``` ## When Not To Use It diff --git a/lib/node-utils/index.ts b/lib/node-utils/index.ts index 33b8cce4..a132b924 100644 --- a/lib/node-utils/index.ts +++ b/lib/node-utils/index.ts @@ -191,6 +191,7 @@ export function isPromisesArrayResolved(node: TSESTree.Node): boolean { * - it's chained with the `then` method * - it's returned from a function * - has `resolves` or `rejects` jest methods + * - has `toResolve` or `toReject` jest-extended matchers */ export function isPromiseHandled(nodeIdentifier: TSESTree.Identifier): boolean { const closestCallExpressionNode = findClosestCallExpressionNode( @@ -462,9 +463,17 @@ export function getAssertNodeInfo( return { matcher, isNegated }; } +const matcherNamesHandlePromise = [ + 'resolves', + 'rejects', + 'toResolve', + 'toReject', +]; + /** * Determines whether a node belongs to an async assertion - * fulfilled by `resolves` or `rejects` properties. + * fulfilled by `resolves` or `rejects` properties or + * by `toResolve` or `toReject` jest-extended matchers * */ export function hasClosestExpectResolvesRejects(node: TSESTree.Node): boolean { @@ -478,7 +487,7 @@ export function hasClosestExpectResolvesRejects(node: TSESTree.Node): boolean { const expectMatcher = node.parent.property; return ( ASTUtils.isIdentifier(expectMatcher) && - (expectMatcher.name === 'resolves' || expectMatcher.name === 'rejects') + matcherNamesHandlePromise.includes(expectMatcher.name) ); } diff --git a/tests/lib/rules/await-async-query.test.ts b/tests/lib/rules/await-async-query.test.ts index 8054035a..4a588f38 100644 --- a/tests/lib/rules/await-async-query.test.ts +++ b/tests/lib/rules/await-async-query.test.ts @@ -222,6 +222,13 @@ ruleTester.run(RULE_NAME, rule, { expect(wrappedQuery(${query}("foo"))).resolves.toBe("bar") ` ), + // async queries with toResolve matchers are valid + ...createTestCase( + (query) => ` + expect(${query}("foo")).toResolve() + expect(wrappedQuery(${query}("foo"))).toResolve() + ` + ), // async queries with rejects matchers are valid ...createTestCase( @@ -231,6 +238,14 @@ ruleTester.run(RULE_NAME, rule, { ` ), + // async queries with toReject matchers are valid + ...createTestCase( + (query) => ` + expect(${query}("foo")).toReject() + expect(wrappedQuery(${query}("foo"))).toReject() + ` + ), + // unresolved async queries with aggressive reporting opted-out are valid ...ALL_ASYNC_COMBINATIONS_TO_TEST.map((query) => ({ settings: { 'testing-library/utils-module': 'test-utils' }, diff --git a/tests/lib/rules/await-async-utils.test.ts b/tests/lib/rules/await-async-utils.test.ts index 9552eba3..68d44d9f 100644 --- a/tests/lib/rules/await-async-utils.test.ts +++ b/tests/lib/rules/await-async-utils.test.ts @@ -40,6 +40,24 @@ ruleTester.run(RULE_NAME, rule, { doSomethingElse(); ${asyncUtil}(() => getByLabelText('email')).then(() => { console.log('done') }); }); + `, + })), + ...ASYNC_UTILS.map((asyncUtil) => ({ + code: ` + import { ${asyncUtil} } from '${testingFramework}'; + test('${asyncUtil} util expect chained with .resolves is valid', () => { + doSomethingElse(); + expect(${asyncUtil}(() => getByLabelText('email'))).resolves.toBe("foo"); + }); + `, + })), + ...ASYNC_UTILS.map((asyncUtil) => ({ + code: ` + import { ${asyncUtil} } from '${testingFramework}'; + test('${asyncUtil} util expect chained with .toResolve is valid', () => { + doSomethingElse(); + expect(${asyncUtil}(() => getByLabelText('email'))).toResolve(); + }); `, })), ...ASYNC_UTILS.map((asyncUtil) => ({ diff --git a/tests/lib/rules/await-fire-event.test.ts b/tests/lib/rules/await-fire-event.test.ts index 62b860e2..fe3aed93 100644 --- a/tests/lib/rules/await-fire-event.test.ts +++ b/tests/lib/rules/await-fire-event.test.ts @@ -36,6 +36,38 @@ ruleTester.run(RULE_NAME, rule, { ...COMMON_FIRE_EVENT_METHODS.map((fireEventMethod) => ({ code: ` import { fireEvent } from '${testingFramework}' + test('promise .resolves from fire event method is valid', async () => { + expect(fireEvent.${fireEventMethod}(getByLabelText('username'))).resolves.toBe("bar") + }) + `, + })), + ...COMMON_FIRE_EVENT_METHODS.map((fireEventMethod) => ({ + code: ` + import { fireEvent } from '${testingFramework}' + test('wrapped promise .resolves from fire event method is valid', async () => { + expect(wrapper(fireEvent.${fireEventMethod}(getByLabelText('username')))).resolves.toBe("bar") + }) + `, + })), + ...COMMON_FIRE_EVENT_METHODS.map((fireEventMethod) => ({ + code: ` + import { fireEvent } from '${testingFramework}' + test('promise .toResolve() from fire event method is valid', async () => { + expect(fireEvent.${fireEventMethod}(getByLabelText('username'))).toResolve() + }) + `, + })), + ...COMMON_FIRE_EVENT_METHODS.map((fireEventMethod) => ({ + code: ` + import { fireEvent } from '${testingFramework}' + test('promise .toResolve() from fire event method is valid', async () => { + expect(wrapper(fireEvent.${fireEventMethod}(getByLabelText('username')))).toResolve() + }) + `, + })), + ...COMMON_FIRE_EVENT_METHODS.map((fireEventMethod) => ({ + code: ` + import { fireEvent } from '${testingFramework}' test('await several promises from fire event methods is valid', async () => { await fireEvent.${fireEventMethod}(getByLabelText('username')) await fireEvent.${fireEventMethod}(getByLabelText('username'))