Skip to content
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

test: fix flaky test-cluster-net-listen-ipv6only-none and related #29681

Closed
wants to merge 1 commit into from

Conversation

Trott
Copy link
Member

@Trott Trott commented Sep 23, 2019

test-cluster-net-listen-ipv6only-none was using port 0 for an
IPv6-only operation and assuming that the operating system would supply
a port that was also available in IPv4. However, CI results seem to
indicate that a port can be supplied that is in use by IPv4 but
available to IPv6, resulting in the test failing. Use common.PORT to
avoid this issue in this and similar tests.

Fixes: #29679

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

@Trott Trott added the flaky-test Issues and PRs related to the tests with unstable failures on the CI. label Sep 23, 2019
@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Sep 23, 2019
@nodejs-github-bot
Copy link
Collaborator

@Trott

This comment has been minimized.

@Trott
Copy link
Member Author

Trott commented Sep 23, 2019

Stress test against master (9999 runs--see debian8-64 for relevant results):
https://ci.nodejs.org/job/node-stress-single-test/11/

@Trott
Copy link
Member Author

Trott commented Sep 23, 2019

Stress test against this PR (9999 runs):
https://ci.nodejs.org/job/node-stress-single-test/12/

@Trott
Copy link
Member Author

Trott commented Sep 24, 2019

Stress test against master showed 21 failures (all of test-cluster-net-listen-ipv6only-none) out of 9999 runs on debian8-64.

On stress test for this PR, we're 3800 runs in so far and 0 failures. 🎉[UPDATE: 0 failures in 9999 runs. This definitely fixes a flaky test.]

@Trott
Copy link
Member Author

Trott commented Sep 24, 2019

It's not clear to me why the one test is susceptible and the other four are either not susceptible or are susceptible at much lower rates. I'm happy to reduce this PR to just be for the one demonstrably affected test even though the others should be affected as best as I can tell.

Either way: @nodejs/testing This is ready for some reviews.

@Trott
Copy link
Member Author

Trott commented Sep 24, 2019

@nodejs/cluster

@Trott Trott added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Sep 25, 2019
test-cluster-net-listen-ipv6only-none was using port `0` for an
IPv6-only operation and assuming that the operating system would supply
a port that was also available in IPv4. However, CI results seem to
indicate that a port can be supplied that is in use by IPv4 but
available to IPv6, resulting in the test failing. Use `common.PORT` to
avoid this issue.

Fixes: nodejs#29679
@Trott
Copy link
Member Author

Trott commented Sep 25, 2019

Since it was only the one test that was demonstrably failing in parallel, I moved the other ones back. It's just the one test that definitely needs to be in sequential that's being moved now.

@nodejs-github-bot
Copy link
Collaborator

@Trott
Copy link
Member Author

Trott commented Oct 1, 2019

Landed in 1c5a3f0

Trott added a commit to Trott/io.js that referenced this pull request Oct 1, 2019
test-cluster-net-listen-ipv6only-none was using port `0` for an
IPv6-only operation and assuming that the operating system would supply
a port that was also available in IPv4. However, CI results seem to
indicate that a port can be supplied that is in use by IPv4 but
available to IPv6, resulting in the test failing. Use `common.PORT` to
avoid this issue.

Fixes: nodejs#29679

PR-URL: nodejs#29681
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
@Trott Trott closed this Oct 1, 2019
targos pushed a commit that referenced this pull request Oct 1, 2019
test-cluster-net-listen-ipv6only-none was using port `0` for an
IPv6-only operation and assuming that the operating system would supply
a port that was also available in IPv4. However, CI results seem to
indicate that a port can be supplied that is in use by IPv4 but
available to IPv6, resulting in the test failing. Use `common.PORT` to
avoid this issue.

Fixes: #29679

PR-URL: #29681
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
@Trott Trott deleted the fix-ipv6Only branch January 13, 2022 22:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. flaky-test Issues and PRs related to the tests with unstable failures on the CI. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

test-cluster-net-listen-ipv6only-none is unreliable/flaky
3 participants