Skip to content

Commit ef9b592

Browse files
dadoonetrnorth
authored andcommitted
Fix HttpWaitStrategy.forStatusCodeMatching used with HttpWaitStrategy.forStatusCode (#881)
In #630 we introduced predicates but with a default one which is always present whatever is passed in the `forStatusCodeMatching()` method. This commit adds a test that demonstrates the issue: * We have a service returning `200 OK` * The predicate expects anything which is a code >= to `300` * The test should throw a Timeout as this condition is never reached but without the current fix, the test never throws the Timeout as 200 matches the default builtin predicate. This commit fixes the problem by checking at startup time what is/are the predicates that needs to be applied. Note that in most cases, an HTTP service is expected to throw a `200 OK` status so that fix might not fix actually any real problem and might be a theory only. But I'd prefer to have code that actually implements what is supposed to work. Closes #880.
1 parent e13f781 commit ef9b592

2 files changed

Lines changed: 33 additions & 7 deletions

File tree

core/src/main/java/org/testcontainers/containers/wait/strategy/HttpWaitStrategy.java

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -39,11 +39,7 @@ public class HttpWaitStrategy extends AbstractWaitStrategy {
3939
private String username;
4040
private String password;
4141
private Predicate<String> responsePredicate;
42-
private Predicate<Integer> statusCodePredicate = responseCode -> {
43-
// If we did not provide any status code, we assume by default HttpURLConnection.HTTP_OK
44-
if (statusCodes.isEmpty() && HttpURLConnection.HTTP_OK == responseCode) return true;
45-
return statusCodes.contains(responseCode);
46-
};
42+
private Predicate<Integer> statusCodePredicate = null;
4743
private Optional<Integer> livenessPort = Optional.empty();
4844

4945
/**
@@ -63,7 +59,7 @@ public HttpWaitStrategy forStatusCode(int statusCode) {
6359
* @return this
6460
*/
6561
public HttpWaitStrategy forStatusCodeMatching(Predicate<Integer> statusCodePredicate) {
66-
this.statusCodePredicate = this.statusCodePredicate.or(statusCodePredicate);
62+
this.statusCodePredicate = statusCodePredicate;
6763
return this;
6864
}
6965

@@ -159,7 +155,22 @@ protected void waitUntilReady() {
159155

160156
log.trace("Get response code {}", connection.getResponseCode());
161157

162-
if (!statusCodePredicate.test(connection.getResponseCode())) {
158+
// Choose the statusCodePredicate strategy depending on what we defined.
159+
Predicate<Integer> predicate;
160+
if (statusCodes.isEmpty() && statusCodePredicate == null) {
161+
// We have no status code and no predicate so we expect a 200 OK response code
162+
predicate = responseCode -> HttpURLConnection.HTTP_OK == responseCode;
163+
} else if (!statusCodes.isEmpty() && statusCodePredicate == null) {
164+
// We use the default status predicate checker when we only have status codes
165+
predicate = responseCode -> statusCodes.contains(responseCode);
166+
} else if (statusCodes.isEmpty()) {
167+
// We only have a predicate
168+
predicate = statusCodePredicate;
169+
} else {
170+
// We have both predicate and status code
171+
predicate = statusCodePredicate.or(responseCode -> statusCodes.contains(responseCode));
172+
}
173+
if (!predicate.test(connection.getResponseCode())) {
163174
throw new RuntimeException(String.format("HTTP response code was: %s",
164175
connection.getResponseCode()));
165176
}

core/src/test/java/org/testcontainers/junit/wait/strategy/HttpWaitStrategyTest.java

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,21 @@ public void testWaitUntilReadyWithTimeoutAndWithManyStatusCodesAndLambda() {
8484
));
8585
}
8686

87+
/**
88+
* Expects that the WaitStrategy throws a {@link RetryCountExceededException} after not receiving any of the
89+
* error code defined with {@link HttpWaitStrategy#forStatusCode(int)}
90+
* and {@link HttpWaitStrategy#forStatusCodeMatching(Predicate)}. Note that a 200 status code should not
91+
* be considered as a successful return as not explicitly set.
92+
* Test case for: https://github.com/testcontainers/testcontainers-java/issues/880
93+
*/
94+
@Test
95+
public void testWaitUntilReadyWithTimeoutAndWithLambdaShouldNotMatchOk() {
96+
waitUntilReadyAndTimeout(startContainerWithCommand(createShellCommand("200 OK", GOOD_RESPONSE_BODY),
97+
createHttpWaitStrategy(ready)
98+
.forStatusCodeMatching(it -> it >= 300)
99+
));
100+
}
101+
87102
/**
88103
* Expects that the WaitStrategy throws a {@link RetryCountExceededException} after not receiving an HTTP 200
89104
* response from the container within the timeout period.

0 commit comments

Comments
 (0)