On Fri, 15 Oct 2021 09:38:50 GMT, Daniel Fuchs <dfu...@openjdk.org> wrote:
>> Evan Whelan has updated the pull request incrementally with two additional >> commits since the last revision: >> >> - Extracted both connectTimeouts to variables and added copyright header to >> accessor >> - Removed reflection in favour of injected accessor for KeepAliveCache > > test/jdk/sun/net/www/http/KeepAliveCache/RequestMethodEquality.java line 95: > >> 93: // Injecting a mock KeepAliveCache that the HttpClient can >> use >> 94: KeepAliveCache kac = new KeepAliveCache(); >> 95: kac.put(url, null, freshClient); > > Instead of injecting a new KeepAliveCache you could get the cache already > present in HttpClient. One way to do this is to use reflection - another is > to inject an accessor class in the sun.net.www.http package. You may not even > need to have to create a "fresh" HttpClient - there should be one there > corresponding to the connection you already created? Thanks Daniel! I've used an injected accessor instead like you've suggested. In regards to creating a "fresh" client. From looking at the implementation of HttpURLConnection, the internal HttpClient (namely, `http`) is not actually initialised during this test. There are various methods in which it gets initialised (`setNewClient`, `proxiedConnect`, `plainConnect0`). With the way I've written this test, none of these methods are called and the internal `http` object remains null throughout. This is why I am explicitly creating a fresh and cached client. By setting the differing `connectTimeout` values and comparing them, I feel it nicely demonstrates whether or not the same client is being used, while trying to minimise the amount of actual network communication that gets carried out. I verified the test behaviour before and after the source fix and before, the `cachedClient` was always being initialised with the broken client we manually inserted into the cache ------------- PR: https://git.openjdk.java.net/jdk/pull/5964
