RFR: 8330814: Cleanups for KeepAliveCache tests [v2]

[Christoph Langer]

On Wed, 24 Apr 2024 06:47:55 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:

>> 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 <turbanoff at gmail.com>
>
> test/jdk/sun/net/www/http/KeepAliveCache/B5045306.java line 55:
> 
>> 53: 
>> 54: /* Part 1:
>> 55:  * The http client makes a connection to a URL who's content contains a lot of
> 
> I think it should be "whose"

Fixed

> 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()`?

Done

> 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.

Done

> test/jdk/sun/net/www/http/KeepAliveCache/B8291637.java line 142:
> 
>> 140:                 .port(port)
>> 141:                 .path("/firstCall")
>> 142:                 .toURL();
> 
> Hello Christoph, what's the significance of changing the path from `/` to `/firstCall` in this test?

Good catch, copy&paste error.

> test/jdk/sun/net/www/http/KeepAliveCache/B8291637.java line 148:
> 
>> 146:             int c;
>> 147:             byte[] buf = new byte[256];
>> 148:             while ((c=i.read(buf)) != -1) {
> 
> Nit: needs a space before and after the `=`

Fixed.

> test/jdk/sun/net/www/http/KeepAliveCache/B8291637.java line 149:
> 
>> 147:             byte[] buf = new byte[256];
>> 148:             while ((c=i.read(buf)) != -1) {
>> 149:                 count+=c;
> 
> Same here, needs a space before/after `+=`

Fixed

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/18884#discussion_r1579046291
PR Review Comment: https://git.openjdk.org/jdk/pull/18884#discussion_r1579047929
PR Review Comment: https://git.openjdk.org/jdk/pull/18884#discussion_r1579049163
PR Review Comment: https://git.openjdk.org/jdk/pull/18884#discussion_r1579042706
PR Review Comment: https://git.openjdk.org/jdk/pull/18884#discussion_r1579043468
PR Review Comment: https://git.openjdk.org/jdk/pull/18884#discussion_r1579043659