Created
February 14, 2017 04:16
-
-
Save notarseniy/c9971e79f42f59c6389c679619beb017 to your computer and use it in GitHub Desktop.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| notarseniy | |
| Thank you for idea for first PR. I've wrote a day ago to #node-dev about one thing that I found: http://logs.libuv.org/node-dev/2017-02-12#14:24:04.509 | |
| Trott | |
| Not all callbacks should have `common.mustCall()`. | |
| Trott | |
| Sometimes you can not say for certain how many times a callback will be called or even whether it will be called. | |
| notarseniy | |
| umm.. sorry, i misunderstood you :) Where is common.mustCall? | |
| Trott | |
| Oh, sorry, you're asking about platformTimeout() and not mustCall(). | |
| Trott | |
| Let me try again... | |
| Trott | |
| platformTimeout() is not always required. | |
| Trott | |
| And in fact to the extent it can be avoided, all the better. | |
| Trott | |
| platformTimeout() is a sign of a problem in the test, really. To the extent we need it, it should be used. But to the extent that we can avoid it, it is better for the tests IMO. | |
| Trott | |
| So, basically, I wouldn't add it to a test unless you are certain there's a problem that it helps with. | |
| notarseniy | |
| wow, okay! probably got it. really good explanation! maybe we should write about this in https://github.com/nodejs/node/blob/master/doc/guides/writing-tests.md? | |
| Trott | |
| (I would love to be able to get rid of platformTimeout() some day, but that would involve fixing all the tests that use it, and a small number of them may not even be fixable.) | |
| Trott | |
| Oh, yeah, that needs more detail for sure. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment