Skip to content

Split up internet DNS tests to avoid timeout problem - #2802

Closed
Trott wants to merge 2 commits into
nodejs:masterfrom
Trott:test-dns-win
Closed

Split up internet DNS tests to avoid timeout problem#2802
Trott wants to merge 2 commits into
nodejs:masterfrom
Trott:test-dns-win

Conversation

@Trott

Copy link
Copy Markdown
Member

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 the vcbuild.bat in a branch so that it would. Results for this change are at https://ci.nodejs.org/job/node-test-commit-windows/604/.

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.)
@Trott

Copy link
Copy Markdown
MemberAuthor

Landing a1949e8 closed #2785 which hopefully makes the diff here slightly easier to deal with.

@mscdexmscdex added dns Issues and PRs related to the dns subsystem. test Issues and PRs related to the tests. labels Sep 10, 2015
@Trott

Copy link
Copy Markdown
MemberAuthor

bump /cc @nodejs/collaborators

@TrottTrott mentioned this pull request Sep 12, 2015
@silverwind

Copy link
Copy Markdown
Contributor

Why does splitting the tests into separate files fix the timeout issue?

@Trott

Copy link
Copy Markdown
MemberAuthor

@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.

@silverwind

Copy link
Copy Markdown
Contributor

Ah, right. LGTM if this is just copy/paste split up.

Copy link
Copy Markdown
Contributor

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.

Copy link
Copy Markdown
MemberAuthor

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.

Trott added a commit that referenced this pull request Sep 12, 2015
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: #2802Fixes: #2468
Reviewed-By: Roman Reiss <me@silverwind.io>
@Trott

Copy link
Copy Markdown
MemberAuthor

Landed in a787f72

@TrottTrott closed this Sep 12, 2015
@Fishrock123Fishrock123 mentioned this pull request Sep 13, 2015
7 tasks
Trott added a commit that referenced this pull request Sep 15, 2015
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: #2802Fixes: #2468
Reviewed-By: Roman Reiss <me@silverwind.io>
Trott added a commit that referenced this pull request Sep 15, 2015
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: #2802Fixes: #2468
Reviewed-By: Roman Reiss <me@silverwind.io>
@rvaggrvagg mentioned this pull request Sep 15, 2015
@rvaggrvagg mentioned this pull request Sep 22, 2015
@Trott
Trott deleted the test-dns-win branch January 9, 2022 22:03
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dnsIssues and PRs related to the dns subsystem.testIssues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@Trott@silverwind@Fishrock123@mscdex