Add no-synchronous-tests rule #26
Conversation
|
Thanks for the PR. Can you explain why a eslint rule would be better instead of using the |
|
The |
|
Sorry, I thought I replied earlier and just realized it didn't go through. Thanks for bringing up |
|
Thanks for the explanation, sounds reasonable to me 😉. |
| } | ||
|
|
||
| if (!returnStatement) { | ||
| context.report(functionExpression, 'Expected test to handle a callback or return a promise.'); |
There was a problem hiding this comment.
I think the error message could be improved. The rule doesn’t check if a callback is handled, so this is not quite correct. What do you think about "Unexpected synchronous test."?
|
I've added those test cases and updated the messaging. Thanks for the feedback! |
|
LGTM. Thanks for your work 🍻. |
I've had a need for a rule based on my experience writing a bunch of async tests within mocha. This rule checks that either a callback was specified or that a promise was returned in the test function.
Mocha dynamically determines whether a test is synchronous by whether the function includes a callback or returns a promise. Because of that, it's easy to end up in a situation where you intend to return a promise (and do not have a
donecallback) but accidentally omit thereturnstatement. This leads to a false-positive where the test would automatically succeed regardless of the actual async result.This rule works well side-by-side with
handle-done-callback, as that rule will enforce whether or not a callback was actually called.Let me know if you see any issues with this PR and I'll try to resolve them. Thanks in advance!