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
[prefer-expect-assertions]: ensure expect.assertions is used when an expect is found to be within a loop #540
Comments
@SimenB mentioned this case in #482 with a comment about them probably being too hard to detect properly, and I'm inclined to agree: I feel that we'd have either far too many edge-cases, or end up only covering half the ways you can loop. We already have |
@G-Rath perfect is the enemy of good. It doesn't have to cover every possible type of loop. Even if it only captures some or most of them. |
True, but I'm thinking about if we can hit "good" let alone "perfect": there are not a lot of ways to loop in JS, so it's reasonable to assume that such a rule would get at least the common ones ( I'm not against having said rule only work on say We already have this with rules like This is vs the base complexity that would be require - in particular, detecting a valid This is why In theory that rule could find a call to either of those functions, and if they're not first report on those calls saying they should be at the top. In practice, this requires iterating over every statement, going through every form of branch node (i.e every case in every switch, every if/else, try/catch - because those are all "equal" in value of checking, so it's not really easy to justify checking one but not the other), for every test. You'd then have to do it all over again to try and figure out if something is "a loop", which is easier to to for some loops than others. While generally that shouldn't grind everything to a halt performance wise, it's still very tied to the size of your test functions, and it's entirely possible for people to have mega sized tests, or even just a lot of them (looking at you Facebook!); this is relevant because IDEs re-run linting rules generally everytime you type a character, so that's a lot of work when you add it all up! So overall it's a question of burden vs usefulness - I do think this rule could be really useful, and that's again why I'm not straight closing this issue out, but I'm wanting to make sure the rule will catch enough cases to be useful. Overall the main problem I see here is not the loop detection, but detecting other expects, because afaik there's not a nice way to structure a rule to say to eslint "run me over everything that matches this selector, and then let me do some processing and decide what to do"; thus this rule will likely always require you to check the entire body of the function. I am going to have a play around with a possible implementation for this rule, but I can't make promises. |
Have implemented an option for native loops in #1013 |
🎉 This issue has been resolved in version 25.4.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
It would be nice to have a lint rule that enforced a length check or use of expect.assertions when an expect call is found inside a loop.
for example:
would report an error
but this would not:
nor would this:
Main goal here is to prevent false positive tests if
things
is an empty list.The text was updated successfully, but these errors were encountered: