On Tue, 23 Apr 2024 08:39:56 GMT, Christoph Langer <clan...@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 incrementally with one
> additional commit since the last revision:
>
> Update test/jdk/sun/net/www/http/KeepAliveCache/B5045306.java
>
> Co-authored-by: Andrey Turbanov <turban...@gmail.com>
test/jdk/sun/net/www/http/KeepAliveCache/B5045306.java line 73:
> 71: public static void startHttpServer() {
> 72: try {
> 73: server = HttpServer.create(new
> InetSocketAddress(InetAddress.getLocalHost(), 0), 10, "/", new
> SimpleHttpTransactionHandler());
While we are changing this test, can you change this to
`InetAddress.getLoopbackAddress()` and then when constructing the requet URI,
use `URIBuilder.loopback()`?
test/jdk/sun/net/www/http/KeepAliveCache/B5045306.java line 91:
> 89: uncaught.add(ex);
> 90: });
> 91: System.out.println("http server listens on: " +
> server.getAddress().getPort());
Maybe move this log message to the line after where we call server.start() and
perhaps print the `server.getAddress()` instead of just the port.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18884#discussion_r1577368917
PR Review Comment: https://git.openjdk.org/jdk/pull/18884#discussion_r1577366679
