Skip to content

Conversation

@Trott
Copy link
Member

The test is unreliable on some Windows platforms in its current form.
Make it more robust by using setInterval() to repeat the flooding
until an error is triggered.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@TrottTrott added the wip Issues and PRs that are still a work in progress. label Mar 14, 2018
@nodejs-github-botnodejs-github-bot added the test Issues and PRs related to the tests. label Mar 14, 2018
@Trott
Copy link
MemberAuthor

Marking as in progress until I can determine that this actually fixes the issue....

@Trott
Copy link
MemberAuthor

@TrottTrott mentioned this pull request Mar 14, 2018
@TrottTrottforce-pushed the unflake branch 2 times, most recently from d35d667 to 38db513CompareMarch 14, 2018 21:26
@Trott
Copy link
MemberAuthor

The change "worked" in that the test kept trying but it got ECONNRESET before the flood error. Trying again but swallowing ECONNRESET errors. Really, though, thing to do is probably make the flooding threshold configurable (if it is not) and then reduce it to something really low for this test...

@Trott
Copy link
MemberAuthor

CI: https://ci.nodejs.org/job/node-test-commit/16923/

Well, in that case, it just times out....

@Trott
Copy link
MemberAuthor

One option is to skip the test if ECONNRESET is received, but that may be even worse than the current situation, which unfortunately is that if the test fails, we shrug it off because the test is inherently unreliable.

@Trott
Copy link
MemberAuthor

OMG, doing it in 10K chunks appears to have fixed the problem on Windows! Removing in progress.

CI: https://ci.nodejs.org/job/node-test-pull-request/13689/ (Failure on LinuxONE appears build related and unrelated to this change. Re-run: https://ci.nodejs.org/job/node-test-commit-linuxone/13659/ )

@TrottTrott removed the wip Issues and PRs that are still a work in progress. label Mar 15, 2018
The test is unreliable on some Windows platforms in its current form. Make it more robust by using `setInterval()` to repeat the flooding until an error is triggered. Fixes: nodejs#18251
@Trott
Copy link
MemberAuthor

@TrottTrott added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Mar 15, 2018
Copy link
Contributor

@apapirovskiapapirovski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nicely done!

@Trott
Copy link
MemberAuthor

Landed in 879f521

@TrottTrott closed this Mar 16, 2018
Trott added a commit to Trott/io.js that referenced this pull request Mar 16, 2018
The test is unreliable on some Windows platforms in its current form. Make it more robust by using `setInterval()` to repeat the flooding until an error is triggered. Fixes: nodejs#18251 PR-URL: nodejs#19349 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]>
targos pushed a commit that referenced this pull request Mar 17, 2018
The test is unreliable on some Windows platforms in its current form. Make it more robust by using `setInterval()` to repeat the flooding until an error is triggered. Fixes: #18251 PR-URL: #19349 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]>
@targostargos mentioned this pull request Mar 18, 2018
MylesBorins pushed a commit that referenced this pull request Mar 20, 2018
The test is unreliable on some Windows platforms in its current form. Make it more robust by using `setInterval()` to repeat the flooding until an error is triggered. Fixes: #18251 PR-URL: #19349 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]>
MylesBorins pushed a commit that referenced this pull request Mar 20, 2018
The test is unreliable on some Windows platforms in its current form. Make it more robust by using `setInterval()` to repeat the flooding until an error is triggered. Fixes: #18251 PR-URL: #19349 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]>
@tniessentniessen removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Mar 24, 2018
codebytere pushed a commit that referenced this pull request Oct 18, 2018
The test is unreliable on some Windows platforms in its current form. Make it more robust by using `setInterval()` to repeat the flooding until an error is triggered. Fixes: #18251 PR-URL: #19349 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

testIssues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@Trott@apapirovski@jasnell@codebytere@tniessen@nodejs-github-bot