From 1c5d8236392f0b0d45708bed6b4d746e7f7bcbb6 Mon Sep 17 00:00:00 2001 From: Ryan Schmitt Date: Wed, 22 Apr 2026 17:21:19 -0700 Subject: [PATCH] Simplify connection timeout tests Convert to pass/fail style by dropping timing duration assertions. If a `ConnectTimeoutException` is thrown at all, the configured timeout was applied correctly. This eliminates spurious failures. Also reduce the timeout from 500ms to 10ms and the select interval from 50ms to 10ms to make the tests run faster. --- .../async/TestAsyncConnectionTimeouts.java | 32 ++++--------------- .../testing/sync/TestConnectionTimeouts.java | 26 +++------------ 2 files changed, 11 insertions(+), 47 deletions(-) diff --git a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/async/TestAsyncConnectionTimeouts.java b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/async/TestAsyncConnectionTimeouts.java index a931da7a62..fb1b6fc376 100644 --- a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/async/TestAsyncConnectionTimeouts.java +++ b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/async/TestAsyncConnectionTimeouts.java @@ -40,11 +40,8 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; -import java.time.Duration; -import java.time.temporal.ChronoUnit; import java.util.concurrent.ExecutionException; -import static java.lang.String.format; import static java.util.concurrent.TimeUnit.MILLISECONDS; import static org.apache.hc.core5.util.TimeValue.ofMilliseconds; import static org.junit.jupiter.api.Assertions.assertInstanceOf; @@ -52,7 +49,6 @@ import static org.junit.jupiter.api.Assertions.assertTrue; class TestAsyncConnectionTimeouts { - private static final Duration EXPECTED_TIMEOUT = Duration.ofMillis(500); @Timeout(5) @ParameterizedTest @@ -61,13 +57,13 @@ class TestAsyncConnectionTimeouts { void testRequestConfig(final String scheme) throws Exception { try ( final CloseableHttpAsyncClient client = HttpAsyncClientBuilder.create() - .setIOReactorConfig(IOReactorConfig.custom().setSelectInterval(ofMilliseconds(50)).build()) + .setIOReactorConfig(IOReactorConfig.custom().setSelectInterval(ofMilliseconds(10)).build()) .setDefaultRequestConfig(RequestConfig.custom() - .setConnectTimeout(EXPECTED_TIMEOUT.toMillis(), MILLISECONDS) + .setConnectTimeout(10, MILLISECONDS) .build()) .build() ) { - warmUpClient(client); + client.start(); assertTimeout(getRequest(scheme), client); } } @@ -79,42 +75,28 @@ void testConnectionConfig(final String scheme) throws Exception { final PoolingAsyncClientConnectionManager connMgr = PoolingAsyncClientConnectionManagerBuilder.create() .setDefaultConnectionConfig( ConnectionConfig.custom() - .setConnectTimeout(EXPECTED_TIMEOUT.toMillis(), MILLISECONDS) + .setConnectTimeout(10, MILLISECONDS) .build()) .build(); try ( final CloseableHttpAsyncClient client = HttpAsyncClientBuilder.create() - .setIOReactorConfig(IOReactorConfig.custom().setSelectInterval(ofMilliseconds(50)).build()) + .setIOReactorConfig(IOReactorConfig.custom().setSelectInterval(ofMilliseconds(10)).build()) .setConnectionManager(connMgr) .build() ) { - warmUpClient(client); + client.start(); assertTimeout(getRequest(scheme), client); } } - private static void warmUpClient(final CloseableHttpAsyncClient client) { - client.start(); - } - private static SimpleHttpRequest getRequest(final String scheme) { return SimpleHttpRequest.create("GET", scheme + "://198.51.100.1/ping"); } private static void assertTimeout(final SimpleHttpRequest request, final CloseableHttpAsyncClient client) { - final long startTime = System.nanoTime(); final Throwable ex = assertThrows(ExecutionException.class, () -> client.execute(request, null).get()) .getCause(); - final Duration actualTime = Duration.of(System.nanoTime() - startTime, ChronoUnit.NANOS); - assertTrue(actualTime.toMillis() > EXPECTED_TIMEOUT.toMillis() / 2, - format("Connection attempt timed out too soon (only %,d out of %,d ms)", - actualTime.toMillis(), - EXPECTED_TIMEOUT.toMillis())); - assertTrue(actualTime.toMillis() < EXPECTED_TIMEOUT.toMillis() * 2, - format("Connection attempt timed out too late (%,d out of %,d ms)", - actualTime.toMillis(), - EXPECTED_TIMEOUT.toMillis())); assertInstanceOf(ConnectTimeoutException.class, ex); - assertTrue(ex.getMessage().contains(EXPECTED_TIMEOUT.toMillis() + " MILLISECONDS"), ex.getMessage()); + assertTrue(ex.getMessage().contains("10 MILLISECONDS"), ex.getMessage()); } } diff --git a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestConnectionTimeouts.java b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestConnectionTimeouts.java index b327d4de17..db169afa8e 100644 --- a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestConnectionTimeouts.java +++ b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestConnectionTimeouts.java @@ -28,7 +28,6 @@ package org.apache.hc.client5.testing.sync; import org.apache.hc.client5.http.ConnectTimeoutException; -import org.apache.hc.client5.http.classic.HttpClient; import org.apache.hc.client5.http.classic.methods.HttpGet; import org.apache.hc.client5.http.classic.methods.HttpUriRequestBase; import org.apache.hc.client5.http.config.ConnectionConfig; @@ -38,21 +37,15 @@ import org.apache.hc.client5.http.impl.classic.HttpClientBuilder; import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManager; import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManagerBuilder; -import org.apache.hc.core5.http.ClassicHttpRequest; import org.junit.jupiter.api.Timeout; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; -import java.time.Duration; -import java.time.temporal.ChronoUnit; - -import static java.lang.String.format; import static java.util.concurrent.TimeUnit.MILLISECONDS; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; class TestConnectionTimeouts { - private static final Duration EXPECTED_TIMEOUT = Duration.ofMillis(500); @Timeout(5) @ParameterizedTest @@ -61,7 +54,7 @@ class TestConnectionTimeouts { void testRequestConfig(final String scheme) throws Exception { try (final CloseableHttpClient client = HttpClientBuilder.create() .setDefaultRequestConfig(RequestConfig.custom() - .setConnectTimeout(EXPECTED_TIMEOUT.toMillis(), MILLISECONDS) + .setConnectTimeout(10, MILLISECONDS) .build()) .build()) { assertTimeout(getRequest(scheme), client); @@ -75,7 +68,7 @@ void testConnectionConfig(final String scheme) throws Exception { final PoolingHttpClientConnectionManager connMgr = PoolingHttpClientConnectionManagerBuilder.create() .setDefaultConnectionConfig( ConnectionConfig.custom() - .setConnectTimeout(EXPECTED_TIMEOUT.toMillis(), MILLISECONDS) + .setConnectTimeout(10, MILLISECONDS) .build()) .build(); try (final CloseableHttpClient client = HttpClientBuilder.create().setConnectionManager(connMgr).build()) { @@ -87,21 +80,10 @@ private static HttpUriRequestBase getRequest(final String scheme) { return new HttpGet(scheme + "://198.51.100.1/ping"); } - private static void assertTimeout(final ClassicHttpRequest request, final HttpClient client) { - final long startTime = System.nanoTime(); + private static void assertTimeout(final HttpUriRequestBase request, final CloseableHttpClient client) { final ConnectTimeoutException ex = assertThrows(ConnectTimeoutException.class, () -> client.execute(request, new BasicHttpClientResponseHandler())); - final Duration actualTime = Duration.of(System.nanoTime() - startTime, ChronoUnit.NANOS); - assertTrue(actualTime.toMillis() > EXPECTED_TIMEOUT.toMillis() / 2, - format("Connection attempt timed out too soon (only %,d out of %,d ms)", - actualTime.toMillis(), - EXPECTED_TIMEOUT.toMillis())); - assertTrue(actualTime.toMillis() < EXPECTED_TIMEOUT.toMillis() * 2, - format("Connection attempt timed out too late (%,d out of %,d ms)", - actualTime.toMillis(), - EXPECTED_TIMEOUT.toMillis())); // Capitalization (`connect` vs `Connect`) varies by Java version - final String message = ex.getMessage(); - assertTrue(message.toLowerCase().contains("connect timed out"), message); + assertTrue(ex.getMessage().toLowerCase().contains("connect timed out"), ex.getMessage()); } }