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

net: TestTCPSelfConnect failures due to unexpected connections [1.19 backport] #58716

Closed
gopherbot opened this issue Feb 24, 2023 · 2 comments
Closed
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge Testing An issue that has been verified to require only test changes, not just a test failure.
Milestone

Comments

@gopherbot
Copy link

@bcmills requested issue #18290 to be considered for backport to the next 1.19 minor release.

@gopherbot, please backport to Go 1.19 and 1.20. This is a trivial backport to delete a noisy test.

@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label Feb 24, 2023
@gopherbot gopherbot added this to the Go1.19.7 milestone Feb 24, 2023
@gopherbot
Copy link
Author

Change https://go.dev/cl/471156 mentions this issue: [release-branch.go1.19] net: delete TestTCPSelfConnect

@dmitshur dmitshur added Testing An issue that has been verified to require only test changes, not just a test failure. CherryPickApproved Used during the release process for point releases and removed CherryPickCandidate Used during the release process for point releases labels Feb 28, 2023
@gopherbot
Copy link
Author

Closed by merging 8417168 to release-branch.go1.19.

gopherbot pushed a commit that referenced this issue Mar 1, 2023
This test is flaky, apparently due to a typo'd operator in CL 21447
that causes it to compare “same port OR IP” instead of
“same port AND IP”.

If we merely fixed the comparison, the test would hopefully stop being
flaky itself, but we would still be left with another problem:
repeatedly dialing a port that we believe to be unused can interfere
with other tests, which may open the previously-unused port and then
attempt a single Dial and expect it to succeed. Arbitrary other Dial
calls for that port may cause the wrong connection to be accepted,
leading to spurious test failures.

Moreover, the test can be extremely expensive for the amount of data
we hope to get from it, depending on the system's port-reuse
algorithms and dial implementations. It is already scaled back by up
to 1000x on a huge number of platforms due to latency, and may even be
ineffective on those platforms because of the arbitrary 1ms Dial
timeout. And the incremental value from it is quite low, too: it tests
the workaround for what is arguably a bug in the Linux kernel, which
ought to be fixed (and tested) upstream instead of worked around in
every open-source project that dials local ports.

Instead of trying to deflake this test, let's just get rid of it.

Updates #18290.
Fixes #58716.

Change-Id: I8a58b93d67916a33741c9ab29ef99c49c46b32c4
Reviewed-on: https://go-review.googlesource.com/c/go/+/460657
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
(cherry picked from commit e08642c)
Reviewed-on: https://go-review.googlesource.com/c/go/+/471156
Reviewed-by: Damien Neil <dneil@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
@golang golang locked and limited conversation to collaborators Feb 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge Testing An issue that has been verified to require only test changes, not just a test failure.
Projects
None yet
Development

No branches or pull requests

2 participants