Github user ahgittin commented on a diff in the pull request: https://github.com/apache/brooklyn-server/pull/816#discussion_r141632615 --- Diff: core/src/test/java/org/apache/brooklyn/util/core/task/BasicTasksFutureTest.java --- @@ -174,29 +175,53 @@ public T call() { } @Test - public void testCancelAfterStartTriggersListenableFuture() throws Exception { - doTestCancelTriggersListenableFuture(Duration.millis(50)); + public void testCancelAfterStartTriggersListenableFutureDynamic() throws Exception { + doTestCancelTriggersListenableFuture(Duration.millis(50), true); } @Test - public void testCancelImmediateTriggersListenableFuture() throws Exception { + public void testCancelImmediateTriggersListenableFutureDynamic() throws Exception { // if cancel fires after submit but before it passes to the executor, - // that needs handling separately; this doesn't guarantee this code path, - // but it happens sometimes (and it should be handled) - doTestCancelTriggersListenableFuture(Duration.ZERO); + // that needs handling separately as it falls into an edge where the future is set and cancelled + // so the executor won't run it, and our wrapper logic doesn't apply; + // this test doesn't guarantee this code path, but makes it likely enough it happens once in a while. + doTestCancelTriggersListenableFuture(Duration.ZERO, true); } - public void doTestCancelTriggersListenableFuture(Duration delay) throws Exception { - Task<String> t = waitForSemaphore(Duration.TEN_SECONDS, true, "x"); + @Test + public void testCancelBeforeTriggersListenableFutureDynamic() throws Exception { + doTestCancelTriggersListenableFuture(Duration.millis(-50), true); + } + @Test + public void testCancelAfterStartTriggersListenableFutureSimple() throws Exception { + doTestCancelTriggersListenableFuture(Duration.millis(50), true); + } + @Test + public void testCancelImmediateTriggersListenableFutureSimple() throws Exception { + doTestCancelTriggersListenableFuture(Duration.ZERO, false); + } + @Test + public void testCancelBeforeTriggersListenableFutureSimple() throws Exception { + doTestCancelTriggersListenableFuture(Duration.millis(-50), false); + } + public void doTestCancelTriggersListenableFuture(Duration delay, boolean dynamic) throws Exception { --- End diff -- good idea
---