Repository: aurora Updated Branches: refs/heads/master 805a53f72 -> 83025f471
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/ Project: http://git-wip-us.apache.org/repos/asf/aurora/repo Commit: http://git-wip-us.apache.org/repos/asf/aurora/commit/83025f47 Tree: http://git-wip-us.apache.org/repos/asf/aurora/tree/83025f47 Diff: http://git-wip-us.apache.org/repos/asf/aurora/diff/83025f47 Branch: refs/heads/master Commit: 83025f47106109b11d46a43e8f707aaf63af8e40 Parents: 805a53f Author: Jordan Ly <[email protected]> Authored: Mon May 21 14:16:49 2018 -0700 Committer: Jordan Ly <[email protected]> Committed: Mon May 21 14:16:49 2018 -0700 ---------------------------------------------------------------------- .../aurora/scheduler/events/WebhookTest.java | 35 ++++++++++---------- 1 file changed, 17 insertions(+), 18 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/aurora/blob/83025f47/src/test/java/org/apache/aurora/scheduler/events/WebhookTest.java ---------------------------------------------------------------------- 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.List; 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 @@ public class WebhookTest { */ 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 class WebhookTest { 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 @@ public class WebhookTest { @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 @@ public class WebhookTest { * 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();
