From bd2fcf437561c23dd03b08d4451f28f928a144c3 Mon Sep 17 00:00:00 2001 From: Nick Bolles Date: Sat, 23 Jul 2022 22:42:18 -0500 Subject: [PATCH 1/8] Support jest-extended toResolve and toReject as handling promises --- lib/node-utils/index.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/node-utils/index.ts b/lib/node-utils/index.ts index 138ca956..3feb1323 100644 --- a/lib/node-utils/index.ts +++ b/lib/node-utils/index.ts @@ -479,6 +479,7 @@ export function hasClosestExpectResolvesRejects(node: TSESTree.Node): boolean { return ( ASTUtils.isIdentifier(expectMatcher) && (expectMatcher.name === 'resolves' || expectMatcher.name === 'rejects') + (expectMatcher.name === 'toResolve' || expectMatcher.name === 'toReject') ); } From d31e3365dc63e605d3aacb16356fc41067e230ab Mon Sep 17 00:00:00 2001 From: Nick Bolles Date: Sat, 23 Jul 2022 22:50:34 -0500 Subject: [PATCH 2/8] add test cases for toResolve and toReject matchers --- tests/lib/rules/await-async-query.test.ts | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/lib/rules/await-async-query.test.ts b/tests/lib/rules/await-async-query.test.ts index 56b4818d..a5fb458a 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' }, From ca1203dc2a2d25036f408d73ef7b68de286a6931 Mon Sep 17 00:00:00 2001 From: Nick Bollesyarn Date: Sat, 23 Jul 2022 23:20:45 -0500 Subject: [PATCH 3/8] feat(await-async-query): support handling promises with jest-extendeds toResolve and toRejects --- lib/node-utils/index.ts | 14 +++++++++++--- tests/lib/rules/await-async-query.test.ts | 2 +- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/lib/node-utils/index.ts b/lib/node-utils/index.ts index 3feb1323..e887d946 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,8 +487,7 @@ export function hasClosestExpectResolvesRejects(node: TSESTree.Node): boolean { const expectMatcher = node.parent.property; return ( ASTUtils.isIdentifier(expectMatcher) && - (expectMatcher.name === 'resolves' || expectMatcher.name === 'rejects') - (expectMatcher.name === 'toResolve' || expectMatcher.name === 'toReject') + 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 a5fb458a..48cede4f 100644 --- a/tests/lib/rules/await-async-query.test.ts +++ b/tests/lib/rules/await-async-query.test.ts @@ -222,7 +222,7 @@ ruleTester.run(RULE_NAME, rule, { expect(wrappedQuery(${query}("foo"))).resolves.toBe("bar") ` ), - // async queries with toResolve matchers are valid + // async queries with toResolve matchers are valid ...createTestCase( (query) => ` expect(${query}("foo")).toResolve() From 5e45ebcef65693fe443134abfbc6f64bb4ec74c7 Mon Sep 17 00:00:00 2001 From: Nick Bollesyarn Date: Sat, 23 Jul 2022 23:34:23 -0500 Subject: [PATCH 4/8] docs(await-async-query): add docs for toResolve/toReject --- docs/rules/await-async-query.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docs/rules/await-async-query.md b/docs/rules/await-async-query.md index c0f8081f..db25c285 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'); From 7ed5f85a0fefb2d5584133b71d483768ac956692 Mon Sep 17 00:00:00 2001 From: Nick Bolles Date: Mon, 8 Aug 2022 14:23:25 -0500 Subject: [PATCH 5/8] Add jest-extended docs --- docs/rules/await-async-utils.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/docs/rules/await-async-utils.md b/docs/rules/await-async-utils.md index 9d23ab41..d3f19487 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,13 @@ 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(); + }); ``` From d1cba9cbfc2ab650667d3e1fa33b2c9e180927b1 Mon Sep 17 00:00:00 2001 From: Nick Bolles Date: Mon, 8 Aug 2022 14:23:30 -0500 Subject: [PATCH 6/8] fireEvent.blur(getByLabelText('username')) --- docs/rules/await-fire-event.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/docs/rules/await-fire-event.md b/docs/rules/await-fire-event.md index f88ca68f..c0b30778 100644 --- a/docs/rules/await-fire-event.md +++ b/docs/rules/await-fire-event.md @@ -7,6 +7,15 @@ 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 +64,13 @@ 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 From f08933c1faa7b5ebb0369cc4a99278635f990ffe Mon Sep 17 00:00:00 2001 From: Nick Bolles Date: Thu, 20 Oct 2022 12:46:42 -0500 Subject: [PATCH 7/8] run format --- docs/rules/await-async-utils.md | 1 - docs/rules/await-fire-event.md | 10 ++++------ lib/node-utils/index.ts | 8 ++++---- tests/lib/rules/await-async-query.test.ts | 18 +++++++++--------- 4 files changed, 17 insertions(+), 20 deletions(-) diff --git a/docs/rules/await-async-utils.md b/docs/rules/await-async-utils.md index 9204cc10..f27f92df 100644 --- a/docs/rules/await-async-utils.md +++ b/docs/rules/await-async-utils.md @@ -92,7 +92,6 @@ test('something correctly', async () => { // 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 b994258b..2a3fa4b0 100644 --- a/docs/rules/await-fire-event.md +++ b/docs/rules/await-fire-event.md @@ -16,7 +16,6 @@ The promise will be considered as handled when: - 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: > > - `@testing-library/vue` (supported by this plugin) @@ -65,12 +64,11 @@ await Promise.all([ fireEvent.blur(getByLabelText('username')), ]); +// Using jest resolves or rejects +expect(fireEvent.focus(getByLabelText('username'))).resolves.toBeUndefined(); - // 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(); +// 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 2281ded6..a132b924 100644 --- a/lib/node-utils/index.ts +++ b/lib/node-utils/index.ts @@ -464,10 +464,10 @@ export function getAssertNodeInfo( } const matcherNamesHandlePromise = [ - 'resolves', - 'rejects', - 'toResolve', - 'toReject', + 'resolves', + 'rejects', + 'toResolve', + 'toReject', ]; /** diff --git a/tests/lib/rules/await-async-query.test.ts b/tests/lib/rules/await-async-query.test.ts index e7beecbc..4a588f38 100644 --- a/tests/lib/rules/await-async-query.test.ts +++ b/tests/lib/rules/await-async-query.test.ts @@ -221,14 +221,14 @@ ruleTester.run(RULE_NAME, rule, { expect(${query}("foo")).resolves.toBe("bar") expect(wrappedQuery(${query}("foo"))).resolves.toBe("bar") ` - ), - // async queries with toResolve matchers are valid - ...createTestCase( - (query) => ` + ), + // 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( @@ -238,13 +238,13 @@ ruleTester.run(RULE_NAME, rule, { ` ), - // async queries with toReject matchers are valid - ...createTestCase( - (query) => ` + // 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) => ({ From 0f334a014ed77c5dbc99063a553efcb7fdcf7c12 Mon Sep 17 00:00:00 2001 From: Nick Bolles Date: Thu, 20 Oct 2022 12:58:34 -0500 Subject: [PATCH 8/8] add tests for await-async-utils and await-fire-event --- tests/lib/rules/await-async-utils.test.ts | 18 +++++++++++++ tests/lib/rules/await-fire-event.test.ts | 32 +++++++++++++++++++++++ 2 files changed, 50 insertions(+) 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'))