test: ensure nextTick is not scheduled in exit#9555
Uh oh!
There was an error while loading. Please reload this page.
Conversation
Fishrock123 added process Fishrock123force-pushed the nexttick-coverage branch from 3685de1 to 3ad531bCompare
Fishrock123 commented Nov 14, 2016
CI again cuz I think it was having some issues? https://ci.nodejs.org/job/node-test-pull-request/4840/ |
Previously our tests did not check this codepath as seen at coverage.nodejs.org PR-URL: nodejs#9555 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>
Fishrock123force-pushed the nexttick-coverage branch from 3ad531b to f65a48fCompare
Trott commented Nov 15, 2016
I notice this and the other PR merged by @Fishrock123 around the same time does not have the |
Fishrock123 commented Nov 15, 2016
Uh, I'm not the only one that does this for our own commits which we can make github see as "merged", also this is in no way new. it is still important if you merged more than one commit though |
sam-github commented Nov 15, 2016
|
Fishrock123 commented Nov 15, 2016
You do need to link to the commits somehow, whether stated or not (and should be stated). |
gibfahn commented Nov 15, 2016
@sam-github It's actually in https://github.com/nodejs/node/blob/master/doc/onboarding.md#landing-prs-details (look for "landed in"). I think @BethGriggs is going to submit a PR to clarify that bit of the docs. On another note, it's a little odd that COLLABORATOR_GUIDE and onboarding.md seem to cover much of the same ground, epecially as you have onboarding-extras.md as well. |
sam-github commented Nov 16, 2016
Ouph, yes, maybe we can unify the 3 docs... :-(, I always follow the COLLABORATOR_GUIDE, now I wonder if I've been doing it wrong. |
Fishrock123 commented Nov 16, 2016
A large problem is that the |
Previously our tests did not check this codepath as seen at coverage.nodejs.org PR-URL: #9555 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>
Previously our tests did not check this codepath as seen at coverage.nodejs.org PR-URL: nodejs#9555 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>
Previously our tests did not check this codepath as seen at coverage.nodejs.org PR-URL: #9555 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>
Previously our tests did not check this codepath as seen at coverage.nodejs.org PR-URL: #9555 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>
Previously our tests did not check this codepath as seen at coverage.nodejs.org PR-URL: #9555 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>
9 participants
Checklist
make -j8 test(UNIX), orvcbuild test nosign(Windows) passesAffected core subsystem(s)
test, process
Description of change
Previously our tests did not check this codepath as seen at
coverage.nodejs.org
See https://coverage.nodejs.org/coverage-fb05e31466ac0bad/root/internal/process/next_tick.js.html (scroll down to the bottom).
CI: https://ci.nodejs.org/job/node-test-pull-request/4824/
(This patch was made live during https://www.twitch.tv/nodesource/v/100431274 if you'd like to see me working on this in retrospect. :P)