Misc test improvements

- Change some tests to use ExecutorService instead of ScheduledExecutorService for scheduling
- Eagerly shutdown threadpools after test cases

Author: jhalterman

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

@@ -27,12 +27,13 @@
 
 import java.lang.reflect.Method;
 import java.time.Duration;
-import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.ExecutorService;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
 
 import static net.jodah.failsafe.Asserts.assertThrows;
-import static net.jodah.failsafe.Testing.*;
+import static net.jodah.failsafe.Testing.failures;
+import static net.jodah.failsafe.Testing.unwrapExceptions;
 import static org.mockito.Mockito.*;
 import static org.testng.Assert.*;
 
@@ -48,7 +49,7 @@ public abstract class AbstractFailsafeTest {
   public interface FastService extends Service {
   }
 
-  abstract ScheduledExecutorService getExecutor();
+  abstract ExecutorService getExecutor();
 
   @BeforeMethod
   void beforeMethod(Method method) {
@@ -89,7 +90,7 @@ <T> T get(FailsafeExecutor<T> failsafe, ContextualSupplier<T> supplier) {
    */
   <T> T failsafeGetWithFallback(Policy<T> policy, CheckedFunction<ExecutionAttemptedEvent<? extends T>, T> fallback,
     CheckedSupplier<T> supplier) {
-    ScheduledExecutorService executor = getExecutor();
+    ExecutorService executor = getExecutor();
     return unwrapExceptions(() -> executor == null ?
       Failsafe.with(Fallback.of(fallback), policy).get(supplier) :
       Failsafe.with(Fallback.ofAsync(fallback), policy).with(executor).getAsync(supplier).get());
@@ -99,7 +100,7 @@ <T> T failsafeGetWithFallback(Policy<T> policy, CheckedFunction<ExecutionAttempt
    * Does a failsafe run with an optional executor.
    */
   void failsafeRun(Policy<?> policy, CheckedRunnable runnable) {
-    ScheduledExecutorService executor = getExecutor();
+    ExecutorService executor = getExecutor();
     if (executor == null)
       Failsafe.with(policy).run(runnable);
     else

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

@@ -18,6 +18,8 @@
 import net.jodah.failsafe.Testing.ConnectException;
 import net.jodah.failsafe.Testing.Service;
 import net.jodah.failsafe.function.*;
+import net.jodah.failsafe.util.concurrent.Scheduler;
+import org.testng.annotations.AfterClass;
 import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.Test;
 
@@ -38,7 +40,7 @@
 
 @Test
 public class AsyncFailsafeTest extends AbstractFailsafeTest {
-  private ScheduledExecutorService executor = Executors.newScheduledThreadPool(5);
+  private ExecutorService executor = Executors.newFixedThreadPool(5);
 
   // Results from a getAsync against a future that wraps an asynchronous Failsafe call
   private @SuppressWarnings("unchecked") Class<? extends Throwable>[] futureAsyncThrowables = new Class[] {
@@ -50,8 +52,13 @@ protected void beforeMethod() {
     counter = new AtomicInteger();
   }
 
+  @AfterClass
+  protected void afterClass() {
+    executor.shutdownNow();
+  }
+
   @Override
-  ScheduledExecutorService getExecutor() {
+  ExecutorService getExecutor() {
     return executor;
   }
 
@@ -476,13 +483,14 @@ public void shouldHandleCompletedExceptionallyGetStageAsync() {
    * Asserts that asynchronous completion via an execution is supported.
    */
   public void shouldCompleteAsync() throws Throwable {
-    Failsafe.with(retryAlways).runAsyncExecution(exec -> executor.schedule(() -> {
+    Failsafe.with(retryAlways).runAsyncExecution(exec -> Scheduler.DEFAULT.schedule(() -> {
       try {
         exec.complete();
         waiter.resume();
       } catch (Exception e) {
         waiter.fail(e);
       }
+      return null;
     }, 100, TimeUnit.MILLISECONDS));
 
     waiter.await(5000);
@@ -573,12 +581,13 @@ public void shouldInterruptExecutionOnCancelWithForkJoinPool() throws Throwable
   }
 
   public void shouldInterruptExecutionOnCancelWithScheduledExecutorService() throws Throwable {
-    assertInterruptedExceptionOnCancel(Failsafe.with(retryAlways).with(executor));
+    ScheduledExecutorService executorService = Executors.newScheduledThreadPool(1);
+    assertInterruptedExceptionOnCancel(Failsafe.with(retryAlways).with(executorService));
+    executorService.shutdownNow();
   }
 
   public void shouldInterruptExecutionOnCancelWithExecutorService() throws Throwable {
-    ExecutorService executorService = Executors.newSingleThreadScheduledExecutor();
-    assertInterruptedExceptionOnCancel(Failsafe.with(retryAlways).with(executorService));
+    assertInterruptedExceptionOnCancel(Failsafe.with(retryAlways).with(executor));
   }
 
   @SuppressWarnings("unused")

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

@@ -16,15 +16,24 @@
 package net.jodah.failsafe;
 
 import net.jodah.concurrentunit.Waiter;
+import org.testng.annotations.AfterClass;
 import org.testng.annotations.Test;
 
-import java.util.concurrent.*;
+import java.util.concurrent.CancellationException;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
 
 import static org.testng.Assert.*;
 
 @Test
 public class FailsafeFutureTest {
-  ScheduledExecutorService executor = Executors.newScheduledThreadPool(2);
+  ExecutorService executor = Executors.newFixedThreadPool(2);
+
+  @AfterClass
+  protected void afterClass() {
+    executor.shutdownNow();
+  }
 
   /**
    * Asserts that retries are stopped and completion handlers are called on cancel.
@@ -77,13 +86,13 @@ public void shouldNotCancelCompletedFuture() throws Throwable {
    * Asserts that a cancelled future ignores subsequent completion attempts.
    */
   public void shouldNotCompleteCancelledFuture() {
-    CompletableFuture<String> future = Failsafe.with(new RetryPolicy<String>()).with(executor).getAsync(() -> {
+    CompletableFuture<String> future = Failsafe.with(new RetryPolicy<>()).with(executor).getAsync(() -> {
       Thread.sleep(1000);
       throw new IllegalStateException();
     });
 
     future.cancel(true);
-    future.complete("unexpected2");
+    future.complete("unexpected");
     Asserts.assertThrows(future::get, CancellationException.class);
   }
 }

src/test/java/net/jodah/failsafe/functional/ShutdownExecutorTest.java

@@ -30,7 +30,7 @@ protected void beforeMethod() {
    */
   public void shouldHandleInitialSchedulingFailure() {
     // Given
-    ScheduledExecutorService executor = Executors.newScheduledThreadPool(0);
+    ExecutorService executor = Executors.newFixedThreadPool(1);
     executor.shutdownNow();
 
     // When
@@ -46,7 +46,7 @@ public void shouldHandleInitialSchedulingFailure() {
    */
   public void shouldHandleShutdown() throws Throwable {
     // Given
-    ScheduledExecutorService executor = Executors.newSingleThreadScheduledExecutor();
+    ExecutorService executor = Executors.newSingleThreadExecutor();
     AtomicInteger counter = new AtomicInteger();
 
     // When
@@ -70,7 +70,7 @@ public void shouldHandleShutdown() throws Throwable {
    */
   public void shouldHandleShutdownNow() throws Throwable {
     // Given
-    ScheduledExecutorService executor = Executors.newSingleThreadScheduledExecutor();
+    ExecutorService executor = Executors.newSingleThreadExecutor();
     AtomicInteger counter = new AtomicInteger();
 
     // When
@@ -91,7 +91,7 @@ public void shouldHandleShutdownNow() throws Throwable {
    */
   public void testShutdownDoesNotPreventTimeoutSync() throws Throwable {
     // Given
-    ScheduledExecutorService executor = Executors.newSingleThreadScheduledExecutor();
+    ExecutorService executor = Executors.newSingleThreadExecutor();
     Timeout<Object> timeout = Timeout.of(Duration.ofMillis(200)).withInterrupt(true);
     AtomicInteger counter = new AtomicInteger();
 
@@ -113,7 +113,7 @@ public void testShutdownDoesNotPreventTimeoutSync() throws Throwable {
    */
   public void testShutdownDoesNotPreventTimeoutAsync() throws Throwable {
     // Given
-    ScheduledExecutorService executor = Executors.newSingleThreadScheduledExecutor();
+    ExecutorService executor = Executors.newSingleThreadExecutor();
     Timeout<Object> timeout = Timeout.of(Duration.ofMillis(200)).withInterrupt(true);
     AtomicInteger counter = new AtomicInteger();
 

src/test/java/net/jodah/failsafe/issues/Issue131Test.java

@@ -25,11 +25,11 @@
 import java.io.IOException;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
 import java.util.function.Predicate;
 
 @Test
 public class Issue131Test {
-
   /**
    * This predicate is invoked in failure scenarios with an arg of null,
    * producing a {@link NullPointerException} yielding surprising results.
@@ -60,7 +60,8 @@ public void syncShouldThrowTheUnderlyingIOException() {
    */
   public void asyncShouldCompleteTheFuture() throws Throwable {
     CircuitBreaker<String> circuitBreaker = new CircuitBreaker<String>().handleResultIf(handleIfEqualsIgnoreCaseFoo);
-    FailsafeExecutor<String> failsafe = Failsafe.with(circuitBreaker).with(Executors.newSingleThreadScheduledExecutor());
+    ScheduledExecutorService executor = Executors.newSingleThreadScheduledExecutor();
+    FailsafeExecutor<String> failsafe = Failsafe.with(circuitBreaker).with(executor);
 
     Waiter waiter = new Waiter();
 
@@ -73,5 +74,6 @@ public void asyncShouldCompleteTheFuture() throws Throwable {
       .whenComplete((s, t) -> waiter.resume()); // Never invoked!
 
     waiter.await(1000);
+    executor.shutdownNow();
   }
 }

src/test/java/net/jodah/failsafe/issues/Issue190Test.java

@@ -5,8 +5,8 @@
 import net.jodah.failsafe.RetryPolicy;
 import net.jodah.failsafe.Testing;
 import org.testng.Assert;
-import org.testng.annotations.AfterTest;
-import org.testng.annotations.BeforeTest;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
 import org.testng.annotations.Test;
 
 import java.util.concurrent.Executors;
@@ -17,13 +17,13 @@
 public class Issue190Test {
   ScheduledExecutorService executor;
 
-  @BeforeTest
-  protected void beforeTest() {
+  @BeforeClass
+  protected void beforeClass() {
     executor = Executors.newSingleThreadScheduledExecutor();
   }
 
-  @AfterTest
-  protected void afterTest() {
+  @AfterClass
+  protected void afterClass() {
     executor.shutdownNow();
   }
 

src/test/java/net/jodah/failsafe/issues/Issue192Test.java

@@ -5,8 +5,8 @@
 import net.jodah.failsafe.RetryPolicy;
 import net.jodah.failsafe.Testing;
 import org.testng.Assert;
-import org.testng.annotations.AfterTest;
-import org.testng.annotations.BeforeTest;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
 import org.testng.annotations.Test;
 
 import java.util.concurrent.ExecutionException;
@@ -27,13 +27,13 @@ static class ExceptionB extends Exception {
   static class ExceptionC extends Exception {
   }
 
-  @BeforeTest
-  protected void beforeTest() {
+  @BeforeClass
+  protected void beforeClass() {
     executor = Executors.newSingleThreadScheduledExecutor();
   }
 
-  @AfterTest
-  protected void afterTest() {
+  @AfterClass
+  protected void afterClass() {
     executor.shutdownNow();
   }
 

src/test/java/net/jodah/failsafe/issues/Issue52Test.java

@@ -18,6 +18,7 @@
 import net.jodah.failsafe.Asserts;
 import net.jodah.failsafe.Failsafe;
 import net.jodah.failsafe.RetryPolicy;
+import org.testng.annotations.AfterClass;
 import org.testng.annotations.Test;
 
 import java.time.Duration;
@@ -29,9 +30,15 @@
 
 @Test
 public class Issue52Test {
+  ScheduledExecutorService scheduler = Executors.newScheduledThreadPool(2);
+
+  @AfterClass
+  protected void afterClass() {
+    scheduler.shutdownNow();
+  }
+
   @Test(expectedExceptions = CancellationException.class)
   public void shouldCancelExecutionViaFuture() throws Throwable {
-    ScheduledExecutorService scheduler = Executors.newScheduledThreadPool(2);
     Future<Object> proxyFuture = Failsafe.with(new RetryPolicy<>().withDelay(Duration.ofMillis(10)))
         .with(scheduler)
         .getAsync(exec -> {
@@ -43,7 +50,6 @@ public void shouldCancelExecutionViaFuture() throws Throwable {
   }
 
   public void shouldCancelExecutionViaCompletableFuture() throws Throwable {
-    ScheduledExecutorService scheduler = Executors.newScheduledThreadPool(2);
     AtomicInteger counter = new AtomicInteger();
     CompletableFuture<String> proxyFuture = Failsafe.with(new RetryPolicy<>().withDelay(Duration.ofMillis(10)))
         .with(scheduler)

src/test/java/net/jodah/failsafe/issues/Issue55Test.java

@@ -45,5 +45,6 @@ public void shouldOnlyFallbackOnFailure() throws Throwable {
 
     Thread.sleep(100);
     assertEquals(counter.get(), 1);
+    executor.shutdownNow();
   }
 }

src/test/java/net/jodah/failsafe/issues/Issue5Test.java

@@ -43,5 +43,6 @@ public void test() throws Throwable {
     }).getAsync(() -> null);
 
     waiter.await(1000);
+    executor.shutdownNow();
   }
 }