Skip to content

Commit 1c5d823

Browse files
rschmittrschmitt
committed
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.
1 parent 4b26224 commit 1c5d823

2 files changed

Lines changed: 11 additions & 47 deletions

File tree

‎httpclient5-testing/src/test/java/org/apache/hc/client5/testing/async/TestAsyncConnectionTimeouts.java‎

Lines changed: 7 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -40,19 +40,15 @@
4040
import org.junit.jupiter.params.ParameterizedTest;
4141
import org.junit.jupiter.params.provider.ValueSource;
4242

43-
import java.time.Duration;
44-
import java.time.temporal.ChronoUnit;
4543
import java.util.concurrent.ExecutionException;
4644

47-
import static java.lang.String.format;
4845
import static java.util.concurrent.TimeUnit.MILLISECONDS;
4946
import static org.apache.hc.core5.util.TimeValue.ofMilliseconds;
5047
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
5148
import static org.junit.jupiter.api.Assertions.assertThrows;
5249
import static org.junit.jupiter.api.Assertions.assertTrue;
5350

5451
class TestAsyncConnectionTimeouts {
55-
private static final Duration EXPECTED_TIMEOUT = Duration.ofMillis(500);
5652

5753
@Timeout(5)
5854
@ParameterizedTest
@@ -61,13 +57,13 @@ class TestAsyncConnectionTimeouts {
6157
void testRequestConfig(final String scheme) throws Exception {
6258
try (
6359
final CloseableHttpAsyncClient client = HttpAsyncClientBuilder.create()
64-
.setIOReactorConfig(IOReactorConfig.custom().setSelectInterval(ofMilliseconds(50)).build())
60+
.setIOReactorConfig(IOReactorConfig.custom().setSelectInterval(ofMilliseconds(10)).build())
6561
.setDefaultRequestConfig(RequestConfig.custom()
66-
.setConnectTimeout(EXPECTED_TIMEOUT.toMillis(), MILLISECONDS)
62+
.setConnectTimeout(10, MILLISECONDS)
6763
.build())
6864
.build()
6965
) {
70-
warmUpClient(client);
66+
client.start();
7167
assertTimeout(getRequest(scheme), client);
7268
}
7369
}
@@ -79,42 +75,28 @@ void testConnectionConfig(final String scheme) throws Exception {
7975
final PoolingAsyncClientConnectionManager connMgr = PoolingAsyncClientConnectionManagerBuilder.create()
8076
.setDefaultConnectionConfig(
8177
ConnectionConfig.custom()
82-
.setConnectTimeout(EXPECTED_TIMEOUT.toMillis(), MILLISECONDS)
78+
.setConnectTimeout(10, MILLISECONDS)
8379
.build())
8480
.build();
8581
try (
8682
final CloseableHttpAsyncClient client = HttpAsyncClientBuilder.create()
87-
.setIOReactorConfig(IOReactorConfig.custom().setSelectInterval(ofMilliseconds(50)).build())
83+
.setIOReactorConfig(IOReactorConfig.custom().setSelectInterval(ofMilliseconds(10)).build())
8884
.setConnectionManager(connMgr)
8985
.build()
9086
) {
91-
warmUpClient(client);
87+
client.start();
9288
assertTimeout(getRequest(scheme), client);
9389
}
9490
}
9591

96-
private static void warmUpClient(final CloseableHttpAsyncClient client) {
97-
client.start();
98-
}
99-
10092
private static SimpleHttpRequest getRequest(final String scheme) {
10193
return SimpleHttpRequest.create("GET", scheme + "://198.51.100.1/ping");
10294
}
10395

10496
private static void assertTimeout(final SimpleHttpRequest request, final CloseableHttpAsyncClient client) {
105-
final long startTime = System.nanoTime();
10697
final Throwable ex = assertThrows(ExecutionException.class, () -> client.execute(request, null).get())
10798
.getCause();
108-
final Duration actualTime = Duration.of(System.nanoTime() - startTime, ChronoUnit.NANOS);
109-
assertTrue(actualTime.toMillis() > EXPECTED_TIMEOUT.toMillis() / 2,
110-
format("Connection attempt timed out too soon (only %,d out of %,d ms)",
111-
actualTime.toMillis(),
112-
EXPECTED_TIMEOUT.toMillis()));
113-
assertTrue(actualTime.toMillis() < EXPECTED_TIMEOUT.toMillis() * 2,
114-
format("Connection attempt timed out too late (%,d out of %,d ms)",
115-
actualTime.toMillis(),
116-
EXPECTED_TIMEOUT.toMillis()));
11799
assertInstanceOf(ConnectTimeoutException.class, ex);
118-
assertTrue(ex.getMessage().contains(EXPECTED_TIMEOUT.toMillis() + " MILLISECONDS"), ex.getMessage());
100+
assertTrue(ex.getMessage().contains("10 MILLISECONDS"), ex.getMessage());
119101
}
120102
}

‎httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestConnectionTimeouts.java‎

Lines changed: 4 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828
package org.apache.hc.client5.testing.sync;
2929

3030
import org.apache.hc.client5.http.ConnectTimeoutException;
31-
import org.apache.hc.client5.http.classic.HttpClient;
3231
import org.apache.hc.client5.http.classic.methods.HttpGet;
3332
import org.apache.hc.client5.http.classic.methods.HttpUriRequestBase;
3433
import org.apache.hc.client5.http.config.ConnectionConfig;
@@ -38,21 +37,15 @@
3837
import org.apache.hc.client5.http.impl.classic.HttpClientBuilder;
3938
import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManager;
4039
import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManagerBuilder;
41-
import org.apache.hc.core5.http.ClassicHttpRequest;
4240
import org.junit.jupiter.api.Timeout;
4341
import org.junit.jupiter.params.ParameterizedTest;
4442
import org.junit.jupiter.params.provider.ValueSource;
4543

46-
import java.time.Duration;
47-
import java.time.temporal.ChronoUnit;
48-
49-
import static java.lang.String.format;
5044
import static java.util.concurrent.TimeUnit.MILLISECONDS;
5145
import static org.junit.jupiter.api.Assertions.assertThrows;
5246
import static org.junit.jupiter.api.Assertions.assertTrue;
5347

5448
class TestConnectionTimeouts {
55-
private static final Duration EXPECTED_TIMEOUT = Duration.ofMillis(500);
5649

5750
@Timeout(5)
5851
@ParameterizedTest
@@ -61,7 +54,7 @@ class TestConnectionTimeouts {
6154
void testRequestConfig(final String scheme) throws Exception {
6255
try (final CloseableHttpClient client = HttpClientBuilder.create()
6356
.setDefaultRequestConfig(RequestConfig.custom()
64-
.setConnectTimeout(EXPECTED_TIMEOUT.toMillis(), MILLISECONDS)
57+
.setConnectTimeout(10, MILLISECONDS)
6558
.build())
6659
.build()) {
6760
assertTimeout(getRequest(scheme), client);
@@ -75,7 +68,7 @@ void testConnectionConfig(final String scheme) throws Exception {
7568
final PoolingHttpClientConnectionManager connMgr = PoolingHttpClientConnectionManagerBuilder.create()
7669
.setDefaultConnectionConfig(
7770
ConnectionConfig.custom()
78-
.setConnectTimeout(EXPECTED_TIMEOUT.toMillis(), MILLISECONDS)
71+
.setConnectTimeout(10, MILLISECONDS)
7972
.build())
8073
.build();
8174
try (final CloseableHttpClient client = HttpClientBuilder.create().setConnectionManager(connMgr).build()) {
@@ -87,21 +80,10 @@ private static HttpUriRequestBase getRequest(final String scheme) {
8780
return new HttpGet(scheme + "://198.51.100.1/ping");
8881
}
8982

90-
private static void assertTimeout(final ClassicHttpRequest request, final HttpClient client) {
91-
final long startTime = System.nanoTime();
83+
private static void assertTimeout(final HttpUriRequestBase request, final CloseableHttpClient client) {
9284
final ConnectTimeoutException ex = assertThrows(ConnectTimeoutException.class,
9385
() -> client.execute(request, new BasicHttpClientResponseHandler()));
94-
final Duration actualTime = Duration.of(System.nanoTime() - startTime, ChronoUnit.NANOS);
95-
assertTrue(actualTime.toMillis() > EXPECTED_TIMEOUT.toMillis() / 2,
96-
format("Connection attempt timed out too soon (only %,d out of %,d ms)",
97-
actualTime.toMillis(),
98-
EXPECTED_TIMEOUT.toMillis()));
99-
assertTrue(actualTime.toMillis() < EXPECTED_TIMEOUT.toMillis() * 2,
100-
format("Connection attempt timed out too late (%,d out of %,d ms)",
101-
actualTime.toMillis(),
102-
EXPECTED_TIMEOUT.toMillis()));
10386
// Capitalization (`connect` vs `Connect`) varies by Java version
104-
final String message = ex.getMessage();
105-
assertTrue(message.toLowerCase().contains("connect timed out"), message);
87+
assertTrue(ex.getMessage().toLowerCase().contains("connect timed out"), ex.getMessage());
10688
}
10789
}

0 commit comments

Comments
 (0)