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.

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

fix(prefer-in-document): false positive on .toHaveLength(1) matcher with *AllBy* query #311

Merged

Conversation

EricKim987
Copy link
Contributor

@EricKim987 EricKim987 commented Jul 5, 2023

What:

Closes #171
Closes #273

Why:

It's valid to use .toHaveLength(1) with AllBy queries to check for "exactly one match".

See conclusion of discussions at #171 (comment)

How:

Implement the truth table as described in the link above.

Checklist:

  • Documentation
  • Tests
  • Ready to be merged

I basically just re-pushed #273 and made a few changes to the test suite.

@EricKim987 EricKim987 marked this pull request as ready for review July 5, 2023 12:41
@EricKim987 EricKim987 changed the title Erickim987/fix prefer in document fix(prefer-in-document): false positive on .toHaveLength(1) matcher with *AllBy* query Jul 5, 2023
Copy link
Collaborator

@G-Rath G-Rath left a comment

Choose a reason for hiding this comment

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

Looks good - just got a couple of nits I'd like addressed if possible

src/__tests__/lib/rules/prefer-in-document.js Show resolved Hide resolved
src/rules/prefer-in-document.js Outdated Show resolved Hide resolved
src/rules/prefer-in-document.js Outdated Show resolved Hide resolved
const matcherNode = node.property;

const matcherArguments = node.parent.arguments;

const expect = node.object.object;
check({
negatedMatcher: true,
queryNode: (queryNode && queryNode.callee) || queryNode,
queryNode: queryNode.callee,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because of the || operator the coverage was 99.64%. Now it is 100%.
Screen Shot 2023-07-07 at 20 42 09

@codecov
Copy link

codecov bot commented Aug 5, 2023

Codecov Report

Merging #311 (275f469) into main (c7ccdea) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main      #311   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           15        15           
  Lines          584       602   +18     
  Branches       168       172    +4     
=========================================
+ Hits           584       602   +18     
Files Changed Coverage Δ
src/rules/prefer-in-document.js 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@G-Rath G-Rath merged commit a7ee93c into testing-library:main Aug 5, 2023
63 checks passed
@github-actions
Copy link

github-actions bot commented Aug 5, 2023

🎉 This PR is included in version 5.0.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

@SevenOutman
Copy link
Contributor

@G-Rath Glad to see this finally getting merged! Could you please port this fix back to v4?

@G-Rath
Copy link
Collaborator

G-Rath commented Aug 7, 2023

@SevenOutman the only change in v5 is that we made @testing-library/dom a peer dependency, so there shouldn't be any reason you can't update to v5.

We don't have any infrastructure in place for doing backport releases

@SevenOutman
Copy link
Contributor

SevenOutman commented Aug 8, 2023

@SevenOutman the only change in v5 is that we made @testing-library/dom a peer dependency, so there shouldn't be any reason you can't update to v5.

@tesing-library/react < 12 has a peer dependency of @testing-library/dom < 8. But I've just upgraded them yesterday anyway.

We don't have any infrastructure in place for doing backport releases

Actually it's simple to do backporting with semantic-release. Just checkout a 4.x branch from the latest v4 tag and cherry-pick the changes to that branch, and semantic-release will handle the rest.
Reference https://semantic-release.gitbook.io/semantic-release/usage/workflow-configuration#pushing-to-a-maintenance-branch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

prefer-in-document: .toHaveLength(1) vs .toBeInTheDocument()
3 participants