Please provide an option to remove the matching of findBy queries by the prefer-explicit-assert rule

[julienw]

What rule do you want to change?

prefer-explicit-assert

Does this change cause the rule to produce more or fewer warnings?

Fewer warnings

How will the change be implemented?

An option matchFindByQueries will be added, true by default.
When it's set to false the rule won't match these queries.

Example code

await findByText(/no profile/i);

How does the current rule affect the code?

The current rule will report an error for such code, but I'd like to be able to exempt it easily.

How will the new rule affect the code?

When the new option is set to false, this code won't be flagged.
When the new option is absent or set to true the code will still be flagged as it is now, so it wouldn't be a breaking change.

Anything else?

My 2 cents: it's common I use findBy assertions in my test code, not to assert that they're here, but rather to ensure we're waiting for something to be displayed before moving forward, so that the test isn't flaky. Therefore adding expect calls in these cases brings distractions :/

References: #409 #421

Do you want to submit a pull request to change the rule?

Yes

[julienw]

Alternatively, as I see there's no option in the existing rules, maybe we can make these 2 use cases 2 separate rules? What do you think?

[Belco90]

Hi @julienw! Thanks for your suggestion, I think it definitely makes sense to have this as a rule option, but I'd like to mention something:

I use findBy assertions in my test code, not to assert that they're here, but rather to ensure we're waiting for something to be displayed before moving forward, so that the test isn't flaky. Therefore adding expect calls in these cases brings distractions

I think this is debatable, since could end up in the same problem as using get* variants as assertions. Quoting Common mistakes with RTL post from where this rule was originated: normally keep the assertion in there just to communicate to readers of the code that it's not just an old query hanging around after a refactor but that I'm explicitly asserting that it exists.

This could be applied to find* queries too so it's not just an old query hanging around after a refactor but that I'm explicitly waiting for an element to appear.

For this reason, I wouldn't say adding expect calls bring distractions (since they make assertion/awaiting explicit), but I definitely agree that standalone find* queries are more explicit than get*, hence the reason to include the option in case you are sure you want to use standalone find* with no assertion.

[julienw]

Yeah, I fully agree for the getBy* assertions :-)
But I very often use the findBy* as a waitFor rather than as a waitFor and fail if not here. When I looked at my codebase I found 2 places where I was using it as an assertion, but dozens where it was merely a waitFor.

Would you rather have it as an option or as 2 different rules? My concern with having an option is that as far as I know nothing is done yet for options, neither in the code or in the tests :-)

[Belco90]

I'd have it as a rule option enabled by default.

My concern with having an option is that as far as I know nothing is done yet for options, neither in the code or in the tests

I don't get this bit tho.

[julienw]

I'd have it as a rule option enabled by default.

My concern with having an option is that as far as I know nothing is done yet for options, neither in the code or in the tests

I don't get this bit tho.

Just me missing that we already use options elsewhere, including for this rule. Forget about this remark!

I'll see if I can make this work.

[Belco90]

@julienw no worries! Would you like to work on this improvement then?

[julienw]

Yes, I already started with a failing test :-)

[Belco90]

That's amazing! Let us know if you need help or guidance with anything ✨

[github-actions]

🎉 This issue has been resolved in version 4.11.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀