re-enable UDP ListenAndServer tests #12901
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
djaglowski
approved these changes
Aug 2, 2022
bogdandrutu
pushed a commit
that referenced
this pull request
Aug 2, 2022
This reverts commit 8b6aef5.
bogdandrutu
added a commit
that referenced
this pull request
Aug 2, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description:
Some UDP listener tests were disabled because of flaky behavior associated with local UDP address availability. Underling cause the the flaky behavior was the internal testutil function used to select an available local address was not UDP aware and was handing back free TCP local addresses.
This PR updates re-enables the skipped tests and updates them to use the network selectable test util function to find local UDP address.
Link to tracking Issue:
Issue: #10916
Previous PR: #12888
Testing:
I've done locally testing with the changes, but because this was originally a flaky test in production (essentially a race) it's unclear that local testing is sufficient.
Worst case these tests are still flaky in production are are disabled again.
Or they could flaky in a different way now. The port race might have hidden other race conditions.
Note
There could be other UDP components that need to be updated that have not yet been flagged as flaky tests due to the listening address race. On review of the use of
testutil.GetAvailableLocalAddressinside the codebase I'm concerned thatjaegerreceiver/jaeger_agent_test.gohas a racy use oftestutil.GetAvailableLocalAddressfor UDP addresses, but since these tests have not been disabled yet I didn't want to disturb them with this change.Documentation:
No documentation Added.