Add sync and async policy composition tests

- Test various combinations of policies
- Test nested retry policies
- Test retry policies where one policy is exceeded

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

@@ -270,8 +270,7 @@ public RetryPolicy<R> onFailedAttempt(CheckedConsumer<? extends ExecutionAttempt
 
   /**
    * Registers the {@code listener} to be called when an execution fails and the {@link RetryPolicy#withMaxRetries(int)
-   * max retry attempts} or {@link RetryPolicy#withMaxDuration(Duration) max duration} are
-   * exceeded.
+   * max retry attempts} or {@link RetryPolicy#withMaxDuration(Duration) max duration} are exceeded.
    */
   public RetryPolicy<R> onRetriesExceeded(CheckedConsumer<? extends ExecutionCompletedEvent<R>> listener) {
     retriesExceededListener = EventListener.of(Assert.notNull(listener, "listener"));
@@ -391,7 +390,8 @@ public Duration getMaxDuration() {
   }
 
   /**
-   * Returns the max retries. Defaults to {@code 100}, which retries forever.
+   * Returns the max number of retries to perform when an execution attempt fails. Defaults to {@code -1}, which retries
+   * forever.
    *
    * @see #withMaxRetries(int)
    */
@@ -539,8 +539,8 @@ public RetryPolicy<R> withDelayWhen(DelayFunction<R, ? extends Throwable> delayF
    * random} or {@link #withBackoff(long, long, ChronoUnit) exponential backoff} delays.
    *
    * @throws IllegalArgumentException if {@code jitterFactor} is < 0 or > 1
-   * @throws IllegalStateException if no delay has been configured or {@link #withJitter(Duration)} has
-   *     already been called
+   * @throws IllegalStateException if no delay has been configured or {@link #withJitter(Duration)} has already been
+   *     called
    */
   public RetryPolicy<R> withJitter(double jitterFactor) {
     Assert.isTrue(jitterFactor >= 0.0 && jitterFactor <= 1.0, "jitterFactor must be >= 0 and <= 1");
@@ -577,8 +577,7 @@ public RetryPolicy<R> withJitter(Duration jitter) {
    * Sets the max duration to perform retries for, else the execution will be failed.
    *
    * @throws NullPointerException if {@code maxDuration} is null
-   * @throws IllegalStateException if {@code maxDuration} is <= the {@link RetryPolicy#withDelay(Duration)
-   *     delay}
+   * @throws IllegalStateException if {@code maxDuration} is <= the {@link RetryPolicy#withDelay(Duration) delay}
    */
   public RetryPolicy<R> withMaxDuration(Duration maxDuration) {
     Assert.notNull(maxDuration, "maxDuration");
@@ -588,7 +587,7 @@ public RetryPolicy<R> withMaxDuration(Duration maxDuration) {
   }
 
   /**
-   * Sets the max number of retries to perform. {@code -1} indicates to retry forever.
+   * Sets the max number of retries to perform when an execution attempt fails. {@code -1} indicates to retry forever.
    *
    * @throws IllegalArgumentException if {@code maxRetries} &lt -1
    */

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

@@ -37,6 +37,7 @@
  */
 public class RetryPolicyExecutor extends PolicyExecutor<RetryPolicy> {
   // Mutable state
+  private volatile int failedAttempts;
   private volatile boolean retriesExceeded;
   /** The fixed, backoff, random or computed delay time in nanoseconds. */
   private volatile long delayNanos = -1;
@@ -62,7 +63,11 @@ public RetryPolicyExecutor(RetryPolicy retryPolicy, EventListener abortListener,
   protected Supplier<ExecutionResult> supplySync(Supplier<ExecutionResult> supplier) {
     return () -> {
       while (true) {
-        ExecutionResult result = postExecute(supplier.get());
+        ExecutionResult result = supplier.get();
+        if (retriesExceeded)
+          return result;
+
+        result = postExecute(result);
         if (result.completed)
           return result;
 
@@ -96,8 +101,9 @@ public Object call() {
           return supplier.get().handle((result, error) -> {
             // Propagate result
             if (result != null) {
-              result = postExecute(result);
-              if (result.completed)
+              if (!retriesExceeded)
+                result = postExecute(result);
+              if (retriesExceeded || result.completed)
                 promise.complete(result);
               else if (!future.isDone() && !future.isCancelled()) {
                 try {
@@ -128,6 +134,8 @@ else if (!future.isDone() && !future.isCancelled()) {
   @Override
   @SuppressWarnings("unchecked")
   protected ExecutionResult onFailure(ExecutionResult result) {
+    failedAttempts++;
+
     // Determine the computed delay
     long computedDelayNanos = -1;
     DelayFunction<Object, Throwable> delayFunction = (DelayFunction<Object, Throwable>) policy.getDelayFn();
@@ -171,7 +179,7 @@ else if (policy.getJitterFactor() > 0.0)
     }
 
     // Calculate result
-    boolean maxRetriesExceeded = policy.getMaxRetries() != -1 && execution.getAttemptCount() > policy.getMaxRetries();
+    boolean maxRetriesExceeded = policy.getMaxRetries() != -1 && failedAttempts > policy.getMaxRetries();
     boolean maxDurationExceeded = policy.getMaxDuration() != null && elapsedNanos > policy.getMaxDuration().toNanos();
     retriesExceeded = maxRetriesExceeded || maxDurationExceeded;
     boolean isAbortable = policy.isAbortable(result.result, result.failure);

src/test/java/net/jodah/failsafe/AbstractFailsafeTest.java

@@ -16,6 +16,8 @@
 package net.jodah.failsafe;
 
 import net.jodah.concurrentunit.Waiter;
+import net.jodah.failsafe.Testing.ConnectException;
+import net.jodah.failsafe.Testing.Service;
 import net.jodah.failsafe.function.CheckedBiFunction;
 import net.jodah.failsafe.function.CheckedRunnable;
 import net.jodah.failsafe.function.CheckedSupplier;
@@ -40,15 +42,6 @@ public abstract class AbstractFailsafeTest {
   Service service = mock(Service.class);
   AtomicInteger counter;
 
-  public static class ConnectException extends RuntimeException {
-  }
-
-  public interface Service {
-    boolean connect();
-
-    boolean disconnect();
-  }
-
   public interface FastService extends Service {
   }
 

src/test/java/net/jodah/failsafe/AsyncFailsafeTest.java

@@ -16,6 +16,8 @@
 package net.jodah.failsafe;
 
 import net.jodah.concurrentunit.Waiter;
+import net.jodah.failsafe.Testing.ConnectException;
+import net.jodah.failsafe.Testing.Service;
 import net.jodah.failsafe.function.*;
 import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.Test;
@@ -60,7 +62,7 @@ private void assertRunAsync(Object runnable) throws Throwable {
 
     // When / Then
     Future<?> future = runAsync(Failsafe.with(retryAlways).with(executor).onComplete(e -> {
-      waiter.assertEquals(e.getAttemptCount(), expectedExecutions.get());
+      waiter.assertEquals(expectedExecutions.get(), e.getAttemptCount());
       waiter.assertNull(e.getResult());
       waiter.assertNull(e.getFailure());
       waiter.resume();
@@ -76,7 +78,7 @@ private void assertRunAsync(Object runnable) throws Throwable {
 
     // When
     Future<?> future2 = runAsync(Failsafe.with(retryTwice).with(executor).onComplete(e -> {
-      waiter.assertEquals(e.getAttemptCount(), expectedExecutions.get());
+      waiter.assertEquals(expectedExecutions.get(), e.getAttemptCount());
       waiter.assertNull(e.getResult());
       waiter.assertTrue(e.getFailure() instanceof ConnectException);
       waiter.resume();
@@ -122,7 +124,7 @@ private void assertGetAsync(Object supplier) throws Throwable {
 
     // When / Then
     Future<Boolean> future = getAsync(Failsafe.with(retryPolicy).with(executor).onComplete(e -> {
-      waiter.assertEquals(e.getAttemptCount(), expectedExecutions.get());
+      waiter.assertEquals(expectedExecutions.get(), e.getAttemptCount());
       waiter.assertTrue(e.getResult());
       waiter.assertNull(e.getFailure());
       waiter.resume();
@@ -140,7 +142,7 @@ private void assertGetAsync(Object supplier) throws Throwable {
 
     // When / Then
     Future<Boolean> future2 = getAsync(Failsafe.with(retryTwice).with(executor).onComplete(e -> {
-      waiter.assertEquals(e.getAttemptCount(), expectedExecutions.get());
+      waiter.assertEquals(expectedExecutions.get(), e.getAttemptCount());
       waiter.assertNull(e.getResult());
       waiter.assertTrue(e.getFailure() instanceof ConnectException);
       waiter.resume();

src/test/java/net/jodah/failsafe/ListenersTest.java

@@ -16,6 +16,7 @@
 package net.jodah.failsafe;
 
 import net.jodah.concurrentunit.Waiter;
+import net.jodah.failsafe.Testing.Service;
 import net.jodah.failsafe.function.CheckedSupplier;
 import org.testng.Assert;
 import org.testng.annotations.BeforeMethod;
@@ -80,10 +81,6 @@ void reset() {
     }
   }
 
-  public interface Service {
-    boolean connect();
-  }
-
   @BeforeMethod
   void beforeMethod() {
     reset(service);
@@ -147,7 +144,7 @@ private <T> FailsafeExecutor<T> registerListeners(RetryPolicy<T> retryPolicy, Ci
   /**
    * Asserts that listeners are called the expected number of times for a successful completion.
    */
-  private void assertListenersForSuccess(boolean sync) throws Throwable {
+  private void assertForSuccess(boolean sync) throws Throwable {
     // Given - Fail 4 times then succeed
     when(service.connect()).thenThrow(failures(2, new IllegalStateException())).thenReturn(false, false, true);
     RetryPolicy<Boolean> retryPolicy = new RetryPolicy<Boolean>().handleResult(false);
@@ -184,18 +181,18 @@ private void assertListenersForSuccess(boolean sync) throws Throwable {
     failure.assertEquals(0);
   }
 
-  public void testListenersForSuccessSync() throws Throwable {
-    assertListenersForSuccess(true);
+  public void testForSuccessSync() throws Throwable {
+    assertForSuccess(true);
   }
 
-  public void testListenersForSuccessAsync() throws Throwable {
-    assertListenersForSuccess(false);
+  public void testForSuccessAsync() throws Throwable {
+    assertForSuccess(false);
   }
 
   /**
    * Asserts that listeners are called the expected number of times for an unhandled failure.
    */
-  private void assertListenersForUnhandledFailure(boolean sync) throws Throwable {
+  private void assertForUnhandledFailure(boolean sync) throws Throwable {
     // Given - Fail 2 times then don't match policy
     when(service.connect()).thenThrow(failures(2, new IllegalStateException()))
         .thenThrow(IllegalArgumentException.class);
@@ -230,18 +227,18 @@ private void assertListenersForUnhandledFailure(boolean sync) throws Throwable {
     success.assertEquals(0);
   }
 
-  public void testListenersForUnhandledFailureSync() throws Throwable {
-    assertListenersForUnhandledFailure(true);
+  public void testForUnhandledFailureSync() throws Throwable {
+    assertForUnhandledFailure(true);
   }
 
-  public void testListenersForUnhandledFailureAsync() throws Throwable {
-    assertListenersForUnhandledFailure(false);
+  public void testForUnhandledFailureAsync() throws Throwable {
+    assertForUnhandledFailure(false);
   }
 
   /**
    * Asserts that listeners aree called the expected number of times when retries are exceeded.
    */
-  private void assertListenersForRetriesExceeded(boolean sync) throws Throwable {
+  private void assertForRetriesExceeded(boolean sync) throws Throwable {
     // Given - Fail 4 times and exceed retries
     when(service.connect()).thenThrow(failures(10, new IllegalStateException()));
     RetryPolicy<Object> retryPolicy = new RetryPolicy<>().abortOn(IllegalArgumentException.class).withMaxRetries(3);
@@ -275,18 +272,18 @@ private void assertListenersForRetriesExceeded(boolean sync) throws Throwable {
     failure.assertEquals(1);
   }
 
-  public void testListenersForRetriesExceededSync() throws Throwable {
-    assertListenersForRetriesExceeded(true);
+  public void testForRetriesExceededSync() throws Throwable {
+    assertForRetriesExceeded(true);
   }
 
-  public void testListenersForRetriesExceededAsync() throws Throwable {
-    assertListenersForRetriesExceeded(false);
+  public void testForRetriesExceededAsync() throws Throwable {
+    assertForRetriesExceeded(false);
   }
 
   /**
    * Asserts that listeners are called the expected number of times for an aborted execution.
    */
-  private void assertListenersForAbort(boolean sync) throws Throwable {
+  private void assertForAbort(boolean sync) throws Throwable {
     // Given - Fail twice then abort
     when(service.connect()).thenThrow(failures(3, new IllegalStateException()))
         .thenThrow(new IllegalArgumentException());
@@ -321,44 +318,15 @@ private void assertListenersForAbort(boolean sync) throws Throwable {
     failure.assertEquals(1);
   }
 
-  public void testListenersForAbortSync() throws Throwable {
-    assertListenersForAbort(true);
-  }
-
-  public void testListenersForAbortAsync() throws Throwable {
-    assertListenersForAbort(false);
-  }
-
-  private void assertFailsafeExecutorForSuccess(boolean sync) throws Throwable {
-    // Given - Fail 4 times then succeed
-    when(service.connect()).thenThrow(failures(2, new IllegalStateException())).thenReturn(false, false, true);
-    RetryPolicy<Boolean> retryPolicy = new RetryPolicy<Boolean>().handleResult(false);
-    CircuitBreaker<Boolean> circuitBreaker = new CircuitBreaker<Boolean>().handleResult(false).withDelay(Duration.ZERO);
-    Fallback<Boolean> fallback = Fallback.of(() -> true);
-    FailsafeExecutor<Boolean> failsafe = registerListeners(retryPolicy, circuitBreaker, fallback);
-
-    // When
-    if (sync)
-      Testing.ignoreExceptions(() -> failsafe.get(supplier));
-    else
-      Testing.ignoreExceptions(() -> failsafe.getAsync(supplier));
-
-    // Then
-    waiter.await(1000);
-    complete.assertEquals(1);
-    success.assertEquals(1);
-    failure.assertEquals(0);
-  }
-
-  public void testFailsafeExecutorForSuccessSync() throws Throwable {
-    assertFailsafeExecutorForSuccess(true);
+  public void testForAbortSync() throws Throwable {
+    assertForAbort(true);
   }
 
-  public void testFailsafeExecutorForSuccessAsync() throws Throwable {
-    assertFailsafeExecutorForSuccess(false);
+  public void testForAbortAsync() throws Throwable {
+    assertForAbort(false);
   }
 
-  private void assertFailsafeExecutorForFailingRetryPolicy(boolean sync) throws Throwable {
+  private void assertForFailingRetryPolicy(boolean sync) throws Throwable {
     when(service.connect()).thenThrow(failures(10, new IllegalStateException()));
 
     // Given failing RetryPolicy
@@ -391,15 +359,15 @@ private void assertFailsafeExecutorForFailingRetryPolicy(boolean sync) throws Th
     failure.assertEquals(1);
   }
 
-  public void testFailsafeExecutorForFailureSync() throws Throwable {
-    assertFailsafeExecutorForFailingRetryPolicy(true);
+  public void testFailingRetryPolicySync() throws Throwable {
+    assertForFailingRetryPolicy(true);
   }
 
-  public void testFailsafeExecutorForFailureAsync() throws Throwable {
-    assertFailsafeExecutorForFailingRetryPolicy(false);
+  public void testFailingRetryPolicyAsync() throws Throwable {
+    assertForFailingRetryPolicy(false);
   }
 
-  private void assertFailsafeExecutorForFailingCircuitBreaker(boolean sync) throws Throwable {
+  private void assertForFailingCircuitBreaker(boolean sync) throws Throwable {
     when(service.connect()).thenThrow(failures(10, new IllegalStateException()));
 
     // Given successful RetryPolicy
@@ -432,15 +400,15 @@ private void assertFailsafeExecutorForFailingCircuitBreaker(boolean sync) throws
     failure.assertEquals(1);
   }
 
-  public void testFailsafeExecutorForFailingCircuitBreakerSync() throws Throwable {
-    assertFailsafeExecutorForFailingCircuitBreaker(true);
+  public void testFailingCircuitBreakerSync() throws Throwable {
+    assertForFailingCircuitBreaker(true);
   }
 
-  public void testFailsafeExecutorForFailingCircuitBreakerAsync() throws Throwable {
-    assertFailsafeExecutorForFailingCircuitBreaker(false);
+  public void testFailingCircuitBreakerAsync() throws Throwable {
+    assertForFailingCircuitBreaker(false);
   }
 
-  private void assertFailsafeExecutorForFailingFallback(boolean sync) throws Throwable {
+  private void assertForFailingFallback(boolean sync) throws Throwable {
     when(service.connect()).thenThrow(failures(10, new IllegalStateException()));
 
     // Given successful RetryPolicy and CircuitBreaker
@@ -473,11 +441,11 @@ private void assertFailsafeExecutorForFailingFallback(boolean sync) throws Throw
     failure.assertEquals(1);
   }
 
-  public void testFailsafeExecutorForFailingFallbackSync() throws Throwable {
-    assertFailsafeExecutorForFailingFallback(true);
+  public void testFailingFallbackSync() throws Throwable {
+    assertForFailingFallback(true);
   }
 
-  public void testFailsafeExecutorForFailingFallbackAsync() throws Throwable {
-    assertFailsafeExecutorForFailingFallback(false);
+  public void testFailingFallbackAsync() throws Throwable {
+    assertForFailingFallback(false);
   }
 }