RFR: 8330814: Cleanups for KeepAliveCache tests [v6]
Jaikiran Pai
jpai at openjdk.org
Tue May 7 07:11:54 UTC 2024
On Wed, 1 May 2024 20:45:27 GMT, Christoph Langer <clanger at openjdk.org> wrote:
>> While working in that area I found some potential for cleanup of a few tests.
>>
>> Most notably:
>>
>> B5045306.java:
>> - does not need to run in othervm
>> - the executor service that it uses should be shut down eventually to free resources
>>
>> B8291637.java:
>> - use just one instead of two test VM invocations
>> KeepAliveTimerThread.java:
>> call to grp.destroy() at the end is pointless (API is void & deprecated for removal)
>>
>> Generally:
>> The deprecated URL constructor is used. It can be switched to the handy URIBuilder
>> Some more try with resources here and there
>
> Christoph Langer has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains ten additional commits since the last revision:
>
> - Further review suggestions
> - Merge branch 'master' into keepalivetests
> - Merge branch 'master' into keepalivetests
> - Fix two URLs
> - Merge branch 'master' into keepalivetests
> - Jaikiran's comments
> - Update test/jdk/sun/net/www/http/KeepAliveCache/B5045306.java
>
> Co-authored-by: Andrey Turbanov <turbanoff at gmail.com>
> - Copyright year
> - JDK-8330814
Hello Christoph, the latest changes look fine to me. I've added a trivial comment about reading the inputstream in one of the test, but you can leave it in its current form if you prefer to.
Please run `tier2` tests once before integrating.
test/jdk/sun/net/www/http/KeepAliveCache/B8291637.java line 144:
> 142: int c;
> 143: byte[] buf = new byte[256];
> 144: while ((c = i.read(buf)) != -1) {
Given that the server response is just a hello world message, I think you can just replace this entire code within the try block with something like:
int count;
try (InputStream i = urlc.getInputStream()) {
count = i.readAllBytes().length;
}
If you prefer to leave it in the current form, that's fine too.
-------------
Marked as reviewed by jpai (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/18884#pullrequestreview-2042246470
PR Review Comment: https://git.openjdk.org/jdk/pull/18884#discussion_r1591917930
More information about the net-dev
mailing list