-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
docs: Add warning about async callback for describe #5046
Conversation
I couldn't find any reference to `describe` not supporting an async function. It seems like a natural idea given `it` and `before` do. I spent considerable time trying to figure out why an async test was failing before I discovered the reason deep in an issue discussion (#2975 (comment)), so hope that this helps others design their test suites!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Just a couple small touchups for typos.
Note that #4921 should also help with this. |
Co-authored-by: Josh Goldberg ✨ <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we simplify it a bit?
👋 @mcdurdin is this still something you have time for? Just checking in! |
Co-authored-by: Josh Goldberg ✨ <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll go ahead and take the liberty of merging this now, as it's a docs change. Thanks for the productive conversation @mcdurdin! 🙌
Requirements
Description of the Change
This is a documentation only change.
I couldn't find any reference to
describe
not supporting an async callback. It seems like a natural conclusion to draw, given mocha specializes in async tests, and givenit
and hook do.I spent considerable time trying to figure out why an async test was failing in one of my modules before I discovered the reason deep in an issue discussion (#2975 (comment))!
Alternate Designs
Why should this be in core?
This is a documentation change.
Benefits
I hope that this helps others design their test suites (I saw a number of comments about spending 'lots of time' trying to figure this out).
Possible Drawbacks
Applicable issues