On Wed, 24 Apr 2024 06:47:55 GMT, Jaikiran Pai <j...@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 <turban...@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
