Large test refactorings and improvements

- Moved most test cases towards being more functional tests, that test broader behaviors/scenarios
- Different test scenarios are more clearly separated
- Most tests are now run against every type of execution, including sync and all the async varieties
- Improved utilties for running tests
- Removed a lot of the original test execution code

Author: jhalterman

pom.xml

@@ -88,6 +88,20 @@
           <target>1.8</target>
         </configuration>
       </plugin>
+      <plugin>
+        <groupId>org.apache.maven.plugins</groupId>
+        <artifactId>maven-surefire-plugin</artifactId>
+        <version>2.22.2</version>
+        <configuration>
+          <trimStackTrace>false</trimStackTrace>
+          <properties>
+            <property>
+              <name>listener</name>
+              <value>net.jodah.failsafe.testing.TestCaseLogger</value>
+            </property>
+          </properties>
+        </configuration>
+      </plugin>
       <plugin>
         <artifactId>maven-release-plugin</artifactId>
         <configuration>

src/main/java/net/jodah/failsafe/Failsafe.java

@@ -88,7 +88,7 @@ public static <R, P extends Policy<R>> FailsafeExecutor<R> with(P outerPolicy, P
    * @param <P> policy type
    * @throws NullPointerException if {@code policies} is null
    * @throws IllegalArgumentException if {@code policies} is empty
-   * @deprecated Use {@link #with(Policy, Policy[])} instead
+   * @deprecated This will be removed in 3.0. Use {@link #with(Policy, Policy[])} instead
    */
   @Deprecated
   public static <R, P extends Policy<R>> FailsafeExecutor<R> with(P[] policies) {

src/main/java/net/jodah/failsafe/FailsafeExecutor.java

@@ -247,7 +247,10 @@ public <T extends R> CompletableFuture<T> getStageAsync(
    *
    * @throws NullPointerException if the {@code supplier} is null
    * @throws RejectedExecutionException if the {@code supplier} cannot be scheduled for execution
+   * @deprecated This will be removed in 3.0. Use either {@link #getStageAsync(ContextualSupplier) getStageAsync} or
+   * {@link #getAsyncExecution(AsyncRunnable) getAsyncExecution} instead
    */
+  @Deprecated
   public <T extends R> CompletableFuture<T> getStageAsyncExecution(
     AsyncSupplier<T, ? extends CompletionStage<T>> supplier) {
     return callAsync(future -> getPromiseOfStageExecution(supplier, future), true);

src/main/java/net/jodah/failsafe/Functions.java

@@ -160,8 +160,10 @@ static <R> Function<AsyncExecutionInternal<R>, CompletableFuture<ExecutionResult
    * supplier cannot be applied multiple times concurrently.
    *
    * @param <R> result type
+   * @deprecated This will be removed in 3.0.
    */
   @SuppressWarnings("unchecked")
+  @Deprecated
   static <R> Function<AsyncExecutionInternal<R>, CompletableFuture<ExecutionResult<R>>> getPromiseOfStageExecution(
     AsyncSupplier<R, ? extends CompletionStage<? extends R>> supplier, FailsafeFuture<R> future) {
 
@@ -177,10 +179,10 @@ static <R> Function<AsyncExecutionInternal<R>, CompletableFuture<ExecutionResult
         if (stage instanceof Future)
           future.propagateCancellation((Future<R>) stage);
 
-        stage.whenComplete((innerResult, failure) -> {
+        stage.whenComplete((result, failure) -> {
           try {
             if (failure != null)
-              execution.record(innerResult, failure instanceof CompletionException ? failure.getCause() : failure);
+              execution.record(result, failure instanceof CompletionException ? failure.getCause() : failure);
           } finally {
             asyncFutureLock.release();
           }
@@ -244,7 +246,7 @@ static <R> Function<AsyncExecutionInternal<R>, CompletableFuture<ExecutionResult
           Future<?> scheduledFuture = scheduler.schedule(callable, 0, TimeUnit.NANOSECONDS);
 
           // Propagate outer cancellations to the scheduled innerFn and its promise
-          future.setCancelFn((mayInterrupt, cancelResult) -> {
+          future.setCancelFn(-1, (mayInterrupt, cancelResult) -> {
             scheduledFuture.cancel(mayInterrupt);
 
             // Cancel a pending promise if the execution attempt has not started

src/main/java/net/jodah/failsafe/RetryPolicyExecutor.java

@@ -157,8 +157,11 @@ public Object handleAsync(AsyncExecutionInternal<R> execution,
                         previousResultRef);
                       Future<?> scheduledRetry = scheduler.schedule(retryFn, postResult.getDelay(),
                         TimeUnit.NANOSECONDS);
+                      // Cancel prior inner executions, such as pending timeouts
+                      future.cancelDependencies(this, false, null);
+
                       // Propagate outer cancellations to the thread that the innerFn will run with
-                      future.setCancelFn((mayInterrupt, cancelResult) -> {
+                      future.setCancelFn(-1, (mayInterrupt, cancelResult) -> {
                         scheduledRetry.cancel(mayInterrupt);
                       });
 

src/main/java/net/jodah/failsafe/Timeout.java

@@ -49,8 +49,9 @@ public Duration getTimeout() {
    * This method is deprecated and will be removed in a future minor release.
    *
    * @see #withCancel(boolean)
-   * @deprecated Always returns {@code true}
+   * @deprecated This will be removed in 3.0. Always returns {@code true}.
    */
+  @Deprecated
   public boolean canCancel() {
     return true;
   }
@@ -69,9 +70,10 @@ public boolean canInterrupt() {
    * Use {@link #withInterrupt(boolean)} instead if you wish to set interruption.
    *
    * @see #withInterrupt(boolean)
-   * @deprecated Tasks are cancelled by default when a Timeout is exceeded. Use {@link #withInterrupt(boolean)}
+   * @deprecated This will be removed in 3.0. Tasks are cancelled by default when a Timeout is exceeded. Use {@link #withInterrupt(boolean)}
    * interrupt cancelled tasks when a Timeout is exceeded.
    */
+  @Deprecated
   public Timeout<R> withCancel(boolean mayInterruptIfRunning) {
     interruptable = mayInterruptIfRunning;
     return this;

src/main/java/net/jodah/failsafe/TimeoutExecutor.java

@@ -16,7 +16,6 @@
 package net.jodah.failsafe;
 
 import net.jodah.failsafe.spi.*;
-import net.jodah.failsafe.spi.Scheduler;
 
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.Future;
@@ -131,10 +130,10 @@ public Function<AsyncExecutionInternal<R>, CompletableFuture<ExecutionResult<R>>
             }, policy.getTimeout().toNanos(), TimeUnit.NANOSECONDS);
             timeoutFutureRef.set(timeoutFuture);
 
-            // Propagate outer cancellations to the Timeout future
+            // Propagate outer cancellations to the Timeout future and its promise
             future.setCancelFn(this, (mayInterrupt, cancelResult) -> {
-              resultRef.compareAndSet(null, cancelResult);
               timeoutFuture.cancel(mayInterrupt);
+              resultRef.compareAndSet(null, cancelResult);
             });
           } catch (Throwable t) {
             // Hard scheduling failure
@@ -155,13 +154,14 @@ public Function<AsyncExecutionInternal<R>, CompletableFuture<ExecutionResult<R>>
         if (!resultRef.compareAndSet(null, result))
           result = resultRef.get();
 
-        // Cancel timeout task
-        Future<R> timeoutFuture = timeoutFutureRef.get();
-        if (timeoutFuture != null && !timeoutFuture.isDone())
-          timeoutFuture.cancel(false);
+        if (result != null) {
+          // Cancel timeout task
+          Future<R> timeoutFuture = timeoutFutureRef.get();
+          if (timeoutFuture != null && !timeoutFuture.isDone())
+            timeoutFuture.cancel(false);
 
-        if (result != null)
           postExecuteAsync(execution, result, scheduler, future);
+        }
 
         promise.complete(result);
       });

src/main/java/net/jodah/failsafe/spi/ExecutionResult.java

@@ -189,7 +189,7 @@ public ExecutionResult<R> with(long delayNanos, boolean complete, boolean succes
   }
 
   /**
-   * Returns whether all execution results leading to this result have been successful.
+   * Returns whether the execution was successful for all policies.
    */
   public boolean getSuccessAll() {
     return successAll != null && successAll;
@@ -215,8 +215,4 @@ public boolean equals(Object o) {
   public int hashCode() {
     return Objects.hash(result, failure);
   }
-
-  String toSummary() {
-    return "[result=" + result + ", failure=" + failure + "]";
-  }
 }

src/main/java/net/jodah/failsafe/spi/FailsafeFuture.java

@@ -15,10 +15,11 @@
  */
 package net.jodah.failsafe.spi;
 
-import java.util.HashMap;
+import java.util.Collections;
 import java.util.Iterator;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.TreeMap;
 import java.util.concurrent.CancellationException;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.Future;
@@ -35,9 +36,9 @@ public class FailsafeFuture<R> extends CompletableFuture<R> {
 
   // Mutable state guarded by "this"
 
-  // The most recent executoin attempt
+  // The most recent execution attempt
   private ExecutionInternal<R> newestExecution;
-  // Functions to apply when this future is cancelled
+  // Functions to apply when this future is cancelled for each policy index, in descending order
   private Map<Integer, BiConsumer<Boolean, ExecutionResult<R>>> cancelFunctions;
   // Whether a cancel with interrupt has already occurred
   private boolean cancelledWithInterrupt;
@@ -111,6 +112,10 @@ public synchronized void cancelDependencies(PolicyExecutor<R, ?> cancellingPolic
       int cancellingPolicyIndex =
         cancellingPolicyExecutor == null ? Integer.MAX_VALUE : cancellingPolicyExecutor.getPolicyIndex();
       Iterator<Entry<Integer, BiConsumer<Boolean, ExecutionResult<R>>>> it = cancelFunctions.entrySet().iterator();
+
+      /* This iteration occurs in descending order to ensure that the {@code cancelResult} can be supplied to outer
+      cancel functions before the inner supplier is cancelled, which would cause PolicyExecutors to complete with
+      CancellationException rather than the expected {@code cancelResult}. */
       while (it.hasNext()) {
         Map.Entry<Integer, BiConsumer<Boolean, ExecutionResult<R>>> entry = it.next();
         if (cancellingPolicyIndex > entry.getKey()) {
@@ -134,12 +139,13 @@ public synchronized void setExecution(ExecutionInternal<R> execution) {
 
   /**
    * Sets a {@code cancelFn} to be called when a PolicyExecutor {@link #cancelDependencies(PolicyExecutor, boolean,
-   * ExecutionResult) cancels dependencies} or when this future is {@link #cancel(boolean) cancelled}.
+   * ExecutionResult) cancels dependencies} with a policyIndex > the given {@code policyIndex}, or when this future is
+   * {@link #cancel(boolean) cancelled}.
    */
-  public synchronized void setCancelFn(BiConsumer<Boolean, ExecutionResult<R>> cancelFn) {
+  public synchronized void setCancelFn(int policyIndex, BiConsumer<Boolean, ExecutionResult<R>> cancelFn) {
     if (cancelFunctions == null)
-      cancelFunctions = new HashMap<>();
-    cancelFunctions.put(-1, cancelFn);
+      cancelFunctions = new TreeMap<>(Collections.reverseOrder());
+    cancelFunctions.put(policyIndex, cancelFn);
   }
 
   /**
@@ -150,7 +156,7 @@ public synchronized void setCancelFn(BiConsumer<Boolean, ExecutionResult<R>> can
   public synchronized void setCancelFn(PolicyExecutor<R, ?> policyExecutor,
     BiConsumer<Boolean, ExecutionResult<R>> cancelFn) {
     if (cancelFunctions == null)
-      cancelFunctions = new HashMap<>();
+      cancelFunctions = new TreeMap<>(Collections.reverseOrder());
     cancelFunctions.put(policyExecutor.getPolicyIndex(), cancelFn);
   }
 
@@ -162,6 +168,6 @@ public synchronized void propagateCancellation(Future<R> future) {
     if (isCancelled())
       future.cancel(cancelledWithInterrupt);
     else
-      setCancelFn((mayInterrupt, cancelResult) -> future.cancel(mayInterrupt));
+      setCancelFn(-2, (mayInterrupt, cancelResult) -> future.cancel(mayInterrupt));
   }
 }

src/main/java/net/jodah/failsafe/spi/PolicyExecutor.java

@@ -126,8 +126,8 @@ protected synchronized CompletableFuture<ExecutionResult<R>> postExecuteAsync(As
     ExecutionResult<R> result, Scheduler scheduler, FailsafeFuture<R> future) {
     CompletableFuture<ExecutionResult<R>> postFuture = null;
 
-    // Guard against post executing twice for the same execution. This will happen if one async execution result
-    // is recorded by a timeout and another asynchronously.
+    /* Guard against post executing twice for the same execution. This will happen if one async execution result is
+     * recorded by a timeout and another via AsyncExecution.record. */
     if (!execution.isAsyncExecution() || !execution.isPostExecuted(policyIndex)) {
       execution.recordAttempt();
       if (isFailure(result)) {