-
Notifications
You must be signed in to change notification settings - Fork 30.2k
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
Split up internet DNS tests to avoid timeout problem #2802
Conversation
For whatever reason, the CI win2012 machine was timing out on the internet test-dns file. Split out ipv4 and ipv6 specific tests to separate files so tests do not time out. (Each file is given a 60 second timeout on CI. Tests within a file are run in sequence.)
bump /cc @nodejs/collaborators |
Why does splitting the tests into separate files fix the timeout issue? |
@silverwind It fixes it because the timeout that's blowing up isn't for any individual test in the file. It's the 60 second timeout for each test file imposed by the Python test runner for the file as a whole. |
Ah, right. LGTM if this is just copy/paste split up. |
running = false, | ||
queue = []; | ||
|
||
function TEST(f) { |
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.
Future pr: export these things to common if they are re-usable.
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.
As far as TEST()
goes, I was torn between "move this into the common.js
" vs. "extract each test into its own file and get rid of TEST()
entirely". So I left it alone for now.
For whatever reason, the CI win2012 machine was timing out on the internet test-dns file. Split out ipv4 and ipv6 specific tests to separate files so tests do not time out. (Each file is given a 60 second timeout on CI. Tests within a file are run in sequence.) PR-URL: #2802 Fixes: #2468 Reviewed-By: Roman Reiss <[email protected]>
Landed in a787f72 |
For whatever reason, the CI win2012 machine was timing out on the internet test-dns file. Split out ipv4 and ipv6 specific tests to separate files so tests do not time out. (Each file is given a 60 second timeout on CI. Tests within a file are run in sequence.) PR-URL: #2802 Fixes: #2468 Reviewed-By: Roman Reiss <[email protected]>
For whatever reason, the CI win2012 machine was timing out on the internet test-dns file. Split out ipv4 and ipv6 specific tests to separate files so tests do not time out. (Each file is given a 60 second timeout on CI. Tests within a file are run in sequence.) PR-URL: #2802 Fixes: #2468 Reviewed-By: Roman Reiss <[email protected]>
On the CI setup, the Win2012 server times out sometimes on
test/internet/test-dns.js
. This PR follows on the changes in #2785 (which will land soon) and splits that file into three files which avoids the timeout.CI doesn't (yet) run tests in
test/internet
but I altered thevcbuild.bat
in a branch so that it would. Results for this change are at https://ci.nodejs.org/job/node-test-commit-windows/604/.