RFR: 8375352: java/net/httpclient/ConnectTimeoutWithProxy*.java tests fail on EC2 [v2]
Volkan Yazici
vyazici at openjdk.org
Tue Jan 27 14:42:22 UTC 2026
On Tue, 27 Jan 2026 09:52:22 GMT, Daniel Fuchs <dfuchs at openjdk.org> wrote:
>> Volkan Yazici has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Apply review feedback
>
> test/jdk/java/net/httpclient/ConnectTimeoutTest.java line 67:
>
>> 65: * @run junit/othervm -Dtest.proxy ${test.main.class}
>> 66: * @run junit/othervm -Dtest.async ${test.main.class}
>> 67: * @run junit/othervm -Dtest.async -Dtest.proxy ${test.main.class}
>
> Instead of several `@run lines` - can you create several `@test id=...` comments?
Done in fc639467db1.
> test/jdk/java/net/httpclient/ConnectTimeoutTest.java line 181:
>
>> 179: Duration requestTimeout)
>> 180: throws Exception {
>> 181: ProxySelector proxySelector = System.getProperty("test.proxy") != null ? PROXY_SELECTOR : NO_PROXY;
>
> Can we use Boolean.getBoolean here? Or at least have some code that wouldn't evaluate `-Dtest.proxy=false` as `true`
Done in fc639467db1.
> test/jdk/java/net/httpclient/ConnectTimeoutTest.java line 183:
>
>> 181: ProxySelector proxySelector = System.getProperty("test.proxy") != null ? PROXY_SELECTOR : NO_PROXY;
>> 182: boolean async = System.getProperty("test.async") != null;
>> 183: if (async) {
>
> Same here.
Done in fc639467db1.
> test/jdk/java/net/httpclient/ConnectTimeoutTest.java line 202:
>
>> 200:
>> 201: for (int i = 0; i < 2; i++) {
>> 202: System.err.printf("iteration %d%n", i);
>
> I'd keep that in `out` rather than `err`. I kind of like having a short summary of what happened in stdout (rather than have it burried within the debug log and possibly lost in the output overflow).
Done in fc639467db1.
> test/jdk/java/net/httpclient/ConnectTimeoutTest.java line 291:
>
>> 289: private static void assertExceptionTypeAndCause(Throwable t) {
>> 290: if (!(t instanceof HttpConnectTimeoutException)) {
>> 291: t.printStackTrace(System.err);
>
> Let's keep that in `out` too. For same reason.
Done in fc639467db1.
> test/jdk/java/net/httpclient/ConnectTimeoutTest.java line 311:
>
>> 309: System.err.println("Headers: " + response.headers());
>> 310: System.err.println("Body: " + response.body());
>> 311: }
>
> ditto
Done in fc639467db1.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/29431#discussion_r2732325786
PR Review Comment: https://git.openjdk.org/jdk/pull/29431#discussion_r2732336903
PR Review Comment: https://git.openjdk.org/jdk/pull/29431#discussion_r2732337328
PR Review Comment: https://git.openjdk.org/jdk/pull/29431#discussion_r2732338140
PR Review Comment: https://git.openjdk.org/jdk/pull/29431#discussion_r2732338817
PR Review Comment: https://git.openjdk.org/jdk/pull/29431#discussion_r2732339308
More information about the net-dev
mailing list