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