Fix flaky Webhook test by ensuring proper error condition
Attempt #3 at fixing the flaky Webhook test once and for all.
Previously, I was testing the error condition by hitting a bad url with a port
of -1. I believe this was erroneous (I am assuming the -1 overflowed into
a valid port). Additionally, there was a timing associated with the test which
could make it flaky as well.
I ensured that the test hit a bad host url and removed the timing for a more
deterministic test.
Reviewed at https://reviews.apache.org/r/67219/
diff --git a/src/test/java/org/apache/aurora/scheduler/events/WebhookTest.java b/src/test/java/org/apache/aurora/scheduler/events/WebhookTest.java
index 3e10c57..3316531 100644
--- a/src/test/java/org/apache/aurora/scheduler/events/WebhookTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/events/WebhookTest.java
@@ -19,7 +19,6 @@
import java.util.Map;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutionException;
-import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;
import javax.servlet.ServletException;
@@ -104,28 +103,24 @@
*/
static class WebhookOnThrowableHandler<T> implements AsyncHandler<T> {
private final AsyncHandler<T> handler;
- private final CountDownLatch latch = new CountDownLatch(1);
+ private final CountDownLatch latch;
- private boolean onThrowableFinished = false;
-
- WebhookOnThrowableHandler(AsyncHandler<T> handler) {
+ WebhookOnThrowableHandler(AsyncHandler<T> handler, CountDownLatch latch) {
this.handler = handler;
+ this.latch = latch;
}
- boolean hasOnThrowableFinished(long timeout, TimeUnit unit) {
+ void waitForOnThrowableToFinish() {
try {
- latch.await(timeout, unit);
+ latch.await();
} catch (InterruptedException e) {
// No-op
}
-
- return onThrowableFinished;
}
@Override
public void onThrowable(Throwable t) {
handler.onThrowable(t);
- onThrowableFinished = true;
latch.countDown();
}
@@ -165,14 +160,16 @@
public <T> ListenableFuture<T> executeRequest(org.asynchttpclient.Request request,
AsyncHandler<T> handler) {
- WebhookOnThrowableHandler<T> wrapped = new WebhookOnThrowableHandler<>(handler);
+ WebhookOnThrowableHandler<T> wrapped = new WebhookOnThrowableHandler<>(
+ handler,
+ new CountDownLatch(1));
ListenableFuture<T> future = super.executeRequest(request, wrapped);
try {
future.get();
future.done();
} catch (InterruptedException | ExecutionException e) {
- // The future threw an exception, wait up to 60 seconds for onThrowable to complete.
- wrapped.hasOnThrowableFinished(60, TimeUnit.SECONDS);
+ // The future threw an exception, wait for onThrowable to complete.
+ wrapped.waitForOnThrowableToFinish();
}
return future;
}
@@ -241,8 +238,10 @@
@Test
public void testTaskChangedWithOldStateError() throws Exception {
- // Don't start Jetty server, send the request to an invalid port to force a ConnectError
- WebhookInfo webhookInfo = buildWebhookInfoWithJettyPort(WEBHOOK_INFO_BUILDER, -1);
+ // Don't start Jetty server, send the request to an invalid URL to force a UnknownHostException
+ WebhookInfo webhookInfo = buildWebhookInfo(
+ WEBHOOK_INFO_BUILDER,
+ "http://bad.host.com");
Webhook webhook = new Webhook(httpClient, webhookInfo, statsProvider);
webhook.taskChangedState(CHANGE_OLD_STATE);
@@ -417,11 +416,11 @@
* should have been started before this method is called.
*/
private WebhookInfo buildWebhookInfoWithJettyPort(WebhookInfoBuilder builder) {
- return buildWebhookInfoWithJettyPort(builder, jettyServer.getURI().getPort());
+ String fullUrl = String.format("http://localhost:%d", jettyServer.getURI().getPort());
+ return buildWebhookInfo(builder, fullUrl);
}
- private WebhookInfo buildWebhookInfoWithJettyPort(WebhookInfoBuilder builder, int port) {
- String fullUrl = String.format("http://localhost:%d", port);
+ private WebhookInfo buildWebhookInfo(WebhookInfoBuilder builder, String fullUrl) {
return builder
.setTargetURL(fullUrl)
.build();
