Added tests against periodic scheduling. I hope the delays are stable

yet fast enough...

Author: jmhofer

rxjava-core/src/main/java/rx/concurrency/SwingScheduler.java

@@ -20,20 +20,22 @@
 import java.awt.EventQueue;
 import java.awt.event.ActionEvent;
 import java.awt.event.ActionListener;
-import java.lang.reflect.InvocationTargetException;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicReference;
 
 import javax.swing.Timer;
 
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 import org.mockito.InOrder;
 
 import rx.Scheduler;
 import rx.Subscription;
 import rx.subscriptions.CompositeSubscription;
 import rx.subscriptions.Subscriptions;
 import rx.util.functions.Action0;
+import rx.util.functions.Func0;
 import rx.util.functions.Func2;
 
 /**
@@ -110,7 +112,6 @@ public void call() {
 
     @Override
     public <T> Subscription schedulePeriodically(T state, final Func2<Scheduler, T, Subscription> action, long initialDelay, long period, TimeUnit unit) {
-        // FIXME test this!
         final AtomicReference<Timer> timer = new AtomicReference<Timer>();
 
         final long delay = unit.toMillis(period); 
@@ -119,20 +120,17 @@ public <T> Subscription schedulePeriodically(T state, final Func2<Scheduler, T,
         final CompositeSubscription subscriptions = new CompositeSubscription();
         final Func2<Scheduler, T, Subscription> initialAction = new Func2<Scheduler, T, Subscription>() {
               @Override
-              public Subscription call(final Scheduler scheduler, final T state) {
-                  // call the action once initially
-                  subscriptions.add(action.call(scheduler, state));
-                  
+              public Subscription call(final Scheduler scheduler, final T state0) {
                   // start timer for periodic execution, collect subscriptions
                   timer.set(new Timer((int) delay, new ActionListener() {
                       @Override
                       public void actionPerformed(ActionEvent e) {
-                          subscriptions.add(action.call(scheduler,  state));
+                          subscriptions.add(action.call(scheduler,  state0));
                       }
                   }));
                   timer.get().start();
 
-                  return action.call(scheduler, state);
+                  return action.call(scheduler, state0);
               }
         };
         subscriptions.add(schedule(state, initialAction, initialDelay, unit));
@@ -141,22 +139,68 @@ public void actionPerformed(ActionEvent e) {
             @Override
             public void call() {
                 // in addition to all the individual unsubscriptions, stop the timer on unsubscribing
-                timer.get().stop();
+                Timer maybeTimer = timer.get();
+                if (maybeTimer != null) {
+                    maybeTimer.stop();
+                }
             }
         }));
 
         return subscriptions;
     }
 
     private static void assertThatTheDelayIsValidForTheSwingTimer(long delay) {
-        if (delay > Integer.MAX_VALUE) {
-            throw new IllegalArgumentException(String.format("The swing timer only accepts delays up to %d milliseconds.", Integer.MAX_VALUE));
+        if (delay < 0 || delay > Integer.MAX_VALUE) {
+            throw new IllegalArgumentException(String.format("The swing timer only accepts non-negative delays up to %d milliseconds.", Integer.MAX_VALUE));
         }
     }
 
     public static class UnitTest {
+        @Rule
+        public ExpectedException exception = ExpectedException.none();
+        
         @Test
-        public void testNestedActions() throws InterruptedException, InvocationTargetException {
+        public void testInvalidDelayValues() {
+            final SwingScheduler scheduler = new SwingScheduler();
+            final Action0 action = mock(Action0.class);
+
+            exception.expect(IllegalArgumentException.class);
+            scheduler.schedulePeriodically(action, -1L, 100L, TimeUnit.SECONDS);
+
+            exception.expect(IllegalArgumentException.class);
+            scheduler.schedulePeriodically(action, 100L, -1L, TimeUnit.SECONDS);
+
+            exception.expect(IllegalArgumentException.class);
+            scheduler.schedulePeriodically(action, 1L + Integer.MAX_VALUE, 100L, TimeUnit.MILLISECONDS);
+            
+            exception.expect(IllegalArgumentException.class);
+            scheduler.schedulePeriodically(action, 100L, 1L + Integer.MAX_VALUE / 1000, TimeUnit.SECONDS);
+        }
+        
+        @Test
+        public void testPeriodicScheduling() throws Exception {
+            final SwingScheduler scheduler = new SwingScheduler();
+
+            final Action0 innerAction = mock(Action0.class);
+            final Action0 unsubscribe = mock(Action0.class);
+            final Func0<Subscription> action = new Func0<Subscription>() {
+                @Override
+                public Subscription call() {
+                    innerAction.call();
+                    return Subscriptions.create(unsubscribe);
+                }
+            };
+            
+            Subscription sub = scheduler.schedulePeriodically(action, 20, 100, TimeUnit.MILLISECONDS);
+            Thread.sleep(400);
+            sub.unsubscribe();
+            waitForEmptyEventQueue();
+            verify(innerAction, times(4)).call();
+            verify(unsubscribe, times(4)).call();
+        }
+        
+        @Test
+        public void testNestedActions() throws Exception {
             final SwingScheduler scheduler = new SwingScheduler();
 
             final Action0 firstStepStart = mock(Action0.class);
@@ -195,12 +239,7 @@ public void call() {
             InOrder inOrder = inOrder(firstStepStart, firstStepEnd, secondStepStart, secondStepEnd, thirdStepStart, thirdStepEnd);
 
             scheduler.schedule(thirdAction);
-            EventQueue.invokeAndWait(new Runnable() {
-                @Override
-                public void run() {
-                    // nothing to do, we're just waiting here for the event queue to be emptied
-                }
-            });
+            waitForEmptyEventQueue();
 
             inOrder.verify(thirdStepStart, times(1)).call();
             inOrder.verify(thirdStepEnd, times(1)).call();
@@ -210,5 +249,13 @@ public void run() {
             inOrder.verify(firstStepEnd, times(1)).call();
         }
 
+        private static void waitForEmptyEventQueue() throws Exception {
+            EventQueue.invokeAndWait(new Runnable() {
+                @Override
+                public void run() {
+                    // nothing to do, we're just waiting here for the event queue to be emptied
+                }
+            });
+        }
     }
 }