test: fix flaky doctool and test#29979
Uh oh!
There was an error while loading. Please reload this page.
Conversation
nodejs-github-bot added doc 
nodejs-github-bot commented Oct 15, 2019
Trott commented Oct 15, 2019
Relevant Windows CI is green. Taking this out of Draft mode. Collaborators please 👍 here to fast-track to unbreak CI. |
nodejs-github-bot commented Oct 15, 2019
Stress test won't work for this because doctool isn't built in that job, but in addition to the green Windows run in the regular CI, here's four more Windows CI runs with this code to confirm it fixes the doctool test issue there: ✅ = Windows job was fully green https://ci.nodejs.org/job/node-test-binary-windows-2/3531/ ✅ |
And to show it failing on master, here are five from master: ❌= doctool test failed on win2008r2 https://ci.nodejs.org/job/node-test-binary-windows-2/3532/ ❌ |
richardlau commented Oct 15, 2019
Given the failure is so prominent on master are we sure it's not a real recent regression? The test has only failed in the two most recent node-daily-master runs (out of the twelve we have history for): The change LGTM btw but I just want to make sure we're not papering over something that has recently changed to suddenly make this test fail consistently. |
Trott commented Oct 15, 2019
Pretty sure it's not a regression. I bisected (by running tests on CI) and the supposed problematic change was 5f80df8 which seems rather unlikely to me given that there's no http in this test. |
Trott commented Oct 15, 2019
(One more 👍 to approve fast-tracking over at #29979 (comment) please?) |
Trott commented Oct 15, 2019
The likely scenarios to me seem to be that either that change (or some other nearby change) caused a timing change on win2008r2 such that it triggered this issue a lot more, or that something changed on the Windows host/configuration itself to make this problem occur far more often. (But yes, I'm speculating.) |
richardlau commented Oct 15, 2019
Not in the tests themselves but the doctool itself does a https request (hence #29918). |
Trott added the author ready Trott commented Oct 15, 2019
Trott commented Oct 15, 2019
Old PRs are still passing when run on CI, so that suggests that the second suggestion by me isn't correct. The first one seems likely: A code change caused a timing change on win2008r2 such that it triggered this issue a lot more. |
tools/doc/generate.js Outdated
There was a problem hiding this comment.
These could now be fsPromises.writeFile(...), although I can do that in a follow up if you prefer.
Trottforce-pushed the async-all-the-way-down branch from b545682 to f4f856bCompareTrott commented Oct 15, 2019
Landed in f4f856b |
Doctool tests have been failing a lot in CI on Win2008 R2. It appears async functions and callback-based functions are being used in combination such that the callback-based function cannot guarantee that it will invoke its callback. Convert the callback-based functions to async functions so we have one paradigm and reliable results. PR-URL: #29979 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]>
Doctool tests have been failing a lot in CI on Win2008 R2. It appears async functions and callback-based functions are being used in combination such that the callback-based function cannot guarantee that it will invoke its callback. Convert the callback-based functions to async functions so we have one paradigm and reliable results. PR-URL: #29979 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]>
Doctool tests have been failing a lot in CI on Win2008 R2. It appears async functions and callback-based functions are being used in combination such that the callback-based function cannot guarantee that it will invoke its callback. Convert the callback-based functions to async functions so we have one paradigm and reliable results. Backport-PR-URL: nodejs#32642 PR-URL: nodejs#29979 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]>
Doctool tests have been failing a lot in CI on Win2008 R2. It appears async functions and callback-based functions are being used in combination such that the callback-based function cannot guarantee that it will invoke its callback. Convert the callback-based functions to async functions so we have one paradigm and reliable results. Backport-PR-URL: #32642 PR-URL: #29979 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]>
6 participants
Doctool tests have been failing a lot in CI on Win2008 R2. It appears
async functions and callback-based functions are being used in
combination such that the callback-based function cannot guarantee that
it will invoke its callback. Convert the callback-based functions to
async functions so we have one paradigm and reliable results.
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes