Replacing www.example.com in OkHttp Client test to fix flakiness - #27277
Open
dennisoelkers wants to merge 2 commits into
Open
dennisoelkers wants to merge 2 commits into
dennisoelkers wants to merge 2 commits into
Conversation
The class (and two of its methods) has been @Ignore/@disabled since #7644 and #7799 (2020) for producing intermittent CI failures. The tests all target the real hostname www.example.com, and ProxySelectorProvider#get() resolves the request host with InetAddress.getByName() on every proxy selection -- a live, blocking DNS lookup as a side effect of running the suite. That's a plausible and well-fitting explanation for years of "fails every now and then": it depends on outbound DNS being fast and reliable in CI, which it isn't always. Swap the target for 203.0.113.10 (RFC 5737 TEST-NET-3, a documentation-only address that InetAddress.getByName() resolves without a network call) and drop the @disabled annotations. Ran the suite 8 times back to back locally: 14/14 passing, 0 failures, every time.
Review feedback: the DNS lookup happens in the ProxySelector.select() override that ProxySelectorProvider#get() returns, not in get() itself.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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
Motivation and Context
Previously, in the
OkHttpClientProviderTest, a dummy value ofwww.example.comwas used for testing proxy configuration. This lead to unnecessary DNS lookups (inProxySelectorProvider's implementation ofProxySelector#select) and therefore flakiness. This change is now replacing this with the use of an IP (that is designated for use in tests) and reenables the test./nocl Internal refactoring.
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist: