RFR: 8375352: java/net/httpclient/ConnectTimeoutWithProxy*.java tests fail on EC2

[Daniel Fuchs]

On Mon, 26 Jan 2026 20:14:32 GMT, Volkan Yazici <vyazici at openjdk.org> wrote:

> Fix tests using `AbstractConnectTimeout` by replacing the usage of an external host (i.e., `example.com`), which unexpectedly resolves to a real IP address on certain test hosts, with a custom `ServerSocket` bootstrapped by the test.
> 
> The initial PR contains 3 commits:
> 
> 1. b06f2606c88 Replace external domain with a custom `ServerSocket` (i.e., _"the fix"_)
> 2. b9262c7ca01 Remove the need for multiple test files
> 3. d95a335db93 Migrate tests to JUnit
> 
> All commits (i.e., 1, 1+2, and 1+2+3) pass tests.

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?

test/jdk/java/net/httpclient/ConnectTimeoutTest.java line 108:

> 106:                                 "Failed creating client socket %s/%s",
> 107:                                 (socketIndex + 1), socketCount);
> 108:                         throw new RuntimeException(message, ioe);

why not `UncheckedIOException`? alternatively a simple for loop ...

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`

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.

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

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.

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

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

PR Review Comment: https://git.openjdk.org/jdk/pull/29431#discussion_r2731166234
PR Review Comment: https://git.openjdk.org/jdk/pull/29431#discussion_r2731177602
PR Review Comment: https://git.openjdk.org/jdk/pull/29431#discussion_r2731209131
PR Review Comment: https://git.openjdk.org/jdk/pull/29431#discussion_r2731211802
PR Review Comment: https://git.openjdk.org/jdk/pull/29431#discussion_r2731225093
PR Review Comment: https://git.openjdk.org/jdk/pull/29431#discussion_r2731229687
PR Review Comment: https://git.openjdk.org/jdk/pull/29431#discussion_r2731231475