doc: improve common.mustCall() explanation #10390
Conversation
|
Maybe not the right time to bring this up, but the message printed when it fails doesn't include the text "common" or "mustCall" in it, so its basically impossible to know that's what is failing. I figured it out after googling for what I thought was a weird v8 error message, and then when that didn't work, using How does this relate to docs? Well, if the message can't include some useful hint as to what is wrong, then docing the current error string might help people who have read the docs. |
|
|
||
| Number of times `fn` should be called. | ||
| Returns a function that calls `fn`. If the returned function has not been called | ||
| exactly `expected` number of times when the test is complete, then the test will |
There was a problem hiding this comment.
We may want to specify that:
- It checks during
process.on('exit') - Wrapping
exithandlers causes problems.
|
When using this function the second parameter seems, even after looking at the source code multiple times, not clear enough for me. If you don't provide the number 1 it throws. I think the reason for this second parameter and for what it's used for should also be documented. |
That's exactly what this PR does. (Are you sure you are looking at the documentation change in this PR and not what is currently in master?)
That's incorrect. The second parameter is called If you do not provide a number, it defaults to 1. That is also currently documented. This PR includes documentation that says:
That's the behavior you are seeing. I welcome a suggestion for better wording. Perhaps this?:
|
|
I'm sorry, I checked the code out again after your PR and probably mixed some things up. The wording is solid and makes it clearer now. Thanks for the help! |
PR-URL: nodejs#10390 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Julian Duque <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Italo A. Casas <[email protected]> Reviewed-By: James M Snell <[email protected]>
|
Landed in 9cfecce |
PR-URL: #10390 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Julian Duque <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Italo A. Casas <[email protected]> Reviewed-By: James M Snell <[email protected]>
PR-URL: #10390 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Julian Duque <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Italo A. Casas <[email protected]> Reviewed-By: James M Snell <[email protected]>
PR-URL: #10390 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Julian Duque <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Italo A. Casas <[email protected]> Reviewed-By: James M Snell <[email protected]>
PR-URL: #10390 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Julian Duque <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Italo A. Casas <[email protected]> Reviewed-By: James M Snell <[email protected]>
PR-URL: #10390 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Julian Duque <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Italo A. Casas <[email protected]> Reviewed-By: James M Snell <[email protected]>
Checklist
Affected core subsystem(s)
test
Description of change