Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(await-async-query,await-async-utils,await-fire-event): support handling promises with jest-extended's .toResolve & .toRejects #612

Merged
merged 9 commits into from Oct 20, 2022
Merged

feat(await-async-query,await-async-utils,await-fire-event): support handling promises with jest-extended's .toResolve & .toRejects #612

merged 9 commits into from Oct 20, 2022

Conversation

NickBolles
Copy link
Contributor

@NickBolles NickBolles commented Jul 24, 2022 •
edited

Checks

  • I have read the contributing guidelines.
  • If some rule is added/updated/removed, I've regenerated the rules list (npm run generate:rules-list)
  • If some rule meta info is changed, I've regenerated the plugin shared configs (npm run generate:configs)

Changes

Add support for marking usage of jest-extendeds toResolve and toReject (docs) as valid handling of promises.

Context

@NickBolles NickBolles changed the title Support jest-extended toResolve and toReject as handling promises @NickBolles feat(await-async-query): support handling promises with jest-extendeds toResolve and toRejects Jul 24, 2022
@NickBolles NickBolles marked this pull request as ready for review July 24, 2022 04:37
@MichaelDeBoey MichaelDeBoey changed the title @NickBolles feat(await-async-query): support handling promises with jest-extendeds toResolve and toRejects feat(await-async-query): support handling promises with jest-extended's .toResolve & .toRejects Jul 24, 2022
@MichaelDeBoey
Copy link
Member

This ESLint plugin is only about @testing-library testing frameworks.
If you want to have linting for jest & jest-extended, you should have a look at eslint-plugin-jest & eslint-plugin-jest-extended

@Belco90
Copy link
Member

Belco90 commented Jul 29, 2022

@MichaelDeBoey Can we rethink this? I would say this is a legit update for the rule. Such matchers can't be extended for the rule in eslint-plugin-jest or eslint-plugin-jest-extended. Since the matchers mentioned in this PR belong to jest, and the rule is tied to jest, I would say this is a valid case and should be merged.

@Belco90 Belco90 reopened this Aug 8, 2022
@MichaelDeBoey
Copy link
Member

Reading the rule README again, I think I misunderstood what @NickBolles was trying to do.

I indeed think this has a place in our plugin! 👍

@Belco90
Copy link
Member

Belco90 commented Aug 8, 2022

@MichaelDeBoey Great! I'll review this later.

@Belco90 Belco90 self-requested a review August 8, 2022 16:40
Belco90
Belco90 requested changes Aug 8, 2022
View reviewed changes
Copy link
Member

@Belco90 Belco90 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change looks great! Since the change made to the hasClosestExpectResolvesRejects method affects to other rules, could you update their docs to mention those matchers are valid too? Those rules are:

  • await-async-utils
  • await-fire-event

@Belco90 Belco90 added the enhancement New feature or request label Aug 8, 2022
@NickBolles NickBolles requested a review from Belco90 August 8, 2022 19:23
@NickBolles
Copy link
Contributor Author

@Belco90 updated! (with a copy/paste accident in the message for d1cba9c :( )

@MichaelDeBoey MichaelDeBoey changed the title feat(await-async-query): support handling promises with jest-extended's .toResolve & .toRejects feat(await-async-query,await-asyn-utils,await-fire-event): support handling promises with jest-extended's .toResolve & .toRejects Aug 8, 2022
MichaelDeBoey
MichaelDeBoey previously requested changes Aug 8, 2022
View reviewed changes
Copy link
Member

@MichaelDeBoey MichaelDeBoey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@NickBolles Could you please also add tests for all these rules?

@MichaelDeBoey MichaelDeBoey changed the title feat(await-async-query,await-asyn-utils,await-fire-event): support handling promises with jest-extended's .toResolve & .toRejects feat(await-async-query,await-async-utils,await-fire-event): support handling promises with jest-extended's .toResolve & .toRejects Aug 8, 2022
@Belco90
Copy link
Member

Belco90 commented Aug 8, 2022

@NickBolles Could you please also add tests for all these rules?

Good catch, I missed this!

@Belco90
Copy link
Member

Belco90 commented Aug 18, 2022

@NickBolles Just a reminder you have some CI checks to fix!

@NickBolles
Copy link
Contributor Author

@Belco90 sorry about such the delay on this. I've added tests for await-async-utils and await-fire-event, merged the latest from main and fixed the format issues.

Belco90
Belco90 approved these changes Oct 20, 2022
View reviewed changes
Copy link
Member

@Belco90 Belco90 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@Belco90 Belco90 dismissed MichaelDeBoey’s stale review October 20, 2022 19:22

Requested changes have been submitted.

@Belco90 Belco90 merged commit 9576462 into testing-library:main Oct 20, 2022
@github-actions
Copy link

🎉 This PR is included in version 5.9.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@Belco90
Copy link
Member

Belco90 commented Oct 20, 2022

@all-contributors please add @NickBolles for code,test, and doc

@allcontributors allcontributors bot mentioned this pull request Oct 20, 2022
Merged
@allcontributors
Copy link
Contributor

@Belco90

I've put up a pull request to add @NickBolles! 🎉

@github-actions
Copy link

🎉 This PR is included in version 6.0.0-alpha.14 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Belco90 Belco90 Belco90 approved these changes

@MichaelDeBoey MichaelDeBoey MichaelDeBoey left review comments

Assignees

@NickBolles NickBolles

Labels
enhancement New feature or request hacktoberfest-accepted released on @alpha released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@NickBolles @MichaelDeBoey @Belco90