fix: improve unit testing of TimerEventSource #293

Author: lburgazzoli

Diff for: operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/internal/TimerEventSourceTest.java

@@ -3,36 +3,35 @@
 import static io.javaoperatorsdk.operator.processing.KubernetesResourceUtils.getUID;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
-import static org.mockito.Mockito.any;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.never;
-import static org.mockito.Mockito.timeout;
-import static org.mockito.Mockito.times;
-import static org.mockito.Mockito.verify;
 
 import io.fabric8.kubernetes.client.CustomResource;
 import io.javaoperatorsdk.operator.TestUtils;
 import io.javaoperatorsdk.operator.processing.KubernetesResourceUtils;
+import io.javaoperatorsdk.operator.processing.event.Event;
 import io.javaoperatorsdk.operator.processing.event.EventHandler;
 import java.io.IOException;
 import java.util.List;
+import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.concurrent.TimeUnit;
+import org.awaitility.Awaitility;
+import org.awaitility.core.ConditionFactory;
+import org.awaitility.core.ThrowingRunnable;
 import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
-import org.mockito.ArgumentCaptor;
 
-@Disabled("Currently very flaky, will fix in https://github.com/java-operator-sdk/java-operator-sdk/issues/293")
 class TimerEventSourceTest {
 
   public static final int INITIAL_DELAY = 50;
   public static final int PERIOD = 50;
   public static final int TESTING_TIME_SLACK = 40;
 
   private TimerEventSource timerEventSource;
-  private EventHandler eventHandlerMock = mock(EventHandler.class);
+  private CapturingEventHandler eventHandlerMock;
 
   @BeforeEach
   public void setup() {
+    eventHandlerMock = new CapturingEventHandler();
+
     timerEventSource = new TimerEventSource();
     timerEventSource.setEventHandler(eventHandlerMock);
     timerEventSource.start();
@@ -41,73 +40,69 @@ public void setup() {
   @Test
   public void producesEventsPeriodically() {
     CustomResource customResource = TestUtils.testCustomResource();
-
     timerEventSource.schedule(customResource, INITIAL_DELAY, PERIOD);
 
-    ArgumentCaptor<TimerEvent> argumentCaptor = ArgumentCaptor.forClass(TimerEvent.class);
-    verify(eventHandlerMock, timeout(INITIAL_DELAY + PERIOD + TESTING_TIME_SLACK).times(2))
-        .handleEvent(argumentCaptor.capture());
-    List<TimerEvent> events = argumentCaptor.getAllValues();
-    assertThat(events)
-        .allMatch(e -> e.getRelatedCustomResourceUid().equals(getUID(customResource)));
-    assertThat(events).allMatch(e -> e.getEventSource().equals(timerEventSource));
+    untilAsserted(() -> {
+      assertThat(eventHandlerMock.events)
+          .hasSizeGreaterThan(2);
+      assertThat(eventHandlerMock.events)
+          .allMatch(e -> e.getRelatedCustomResourceUid().equals(getUID(customResource)));
+      assertThat(eventHandlerMock.events)
+          .allMatch(e -> e.getEventSource().equals(timerEventSource));
+    });
   }
 
   @Test
-  public void deRegistersPeriodicalEventSources() throws InterruptedException {
+  public void deRegistersPeriodicalEventSources() {
     CustomResource customResource = TestUtils.testCustomResource();
 
     timerEventSource.schedule(customResource, INITIAL_DELAY, PERIOD);
-    Thread.sleep(INITIAL_DELAY + PERIOD + TESTING_TIME_SLACK);
+    untilAsserted(() -> assertThat(eventHandlerMock.events).hasSizeGreaterThan(1));
+
     timerEventSource.eventSourceDeRegisteredForResource(getUID(customResource));
-    Thread.sleep(PERIOD + TESTING_TIME_SLACK);
 
-    verify(eventHandlerMock, times(2)).handleEvent(any());
+    int size = eventHandlerMock.events.size();
+    untilAsserted(() -> assertThat(eventHandlerMock.events).hasSize(size));
   }
 
   @Test
-  public void schedulesOnce() throws InterruptedException {
+  public void schedulesOnce() {
     CustomResource customResource = TestUtils.testCustomResource();
 
     timerEventSource.scheduleOnce(customResource, PERIOD);
 
-    Thread.sleep(2 * PERIOD + TESTING_TIME_SLACK);
-    verify(eventHandlerMock, times(1)).handleEvent(any());
+    untilAsserted(() -> assertThat(eventHandlerMock.events).hasSize(1));
+    untilAsserted(PERIOD * 2, 0, () -> assertThat(eventHandlerMock.events).hasSize(1));
   }
 
   @Test
-  public void canCancelOnce() throws InterruptedException {
+  public void canCancelOnce() {
     CustomResource customResource = TestUtils.testCustomResource();
 
     timerEventSource.scheduleOnce(customResource, PERIOD);
     timerEventSource.cancelOnceSchedule(KubernetesResourceUtils.getUID(customResource));
 
-    Thread.sleep(PERIOD + TESTING_TIME_SLACK);
-    verify(eventHandlerMock, never()).handleEvent(any());
+    untilAsserted(() -> assertThat(eventHandlerMock.events).isEmpty());
   }
 
   @Test
-  public void canRescheduleOnceEvent() throws InterruptedException {
+  public void canRescheduleOnceEvent() {
     CustomResource customResource = TestUtils.testCustomResource();
 
     timerEventSource.scheduleOnce(customResource, PERIOD);
     timerEventSource.scheduleOnce(customResource, 2 * PERIOD);
 
-    Thread.sleep(PERIOD + TESTING_TIME_SLACK);
-    verify(eventHandlerMock, never()).handleEvent(any());
-    Thread.sleep(PERIOD + TESTING_TIME_SLACK);
-    verify(eventHandlerMock, times(1)).handleEvent(any());
+    untilAsserted(PERIOD * 2, PERIOD, () -> assertThat(eventHandlerMock.events).hasSize(1));
   }
 
   @Test
-  public void deRegistersOnceEventSources() throws InterruptedException {
+  public void deRegistersOnceEventSources() {
     CustomResource customResource = TestUtils.testCustomResource();
 
     timerEventSource.scheduleOnce(customResource, PERIOD);
     timerEventSource.eventSourceDeRegisteredForResource(getUID(customResource));
-    Thread.sleep(PERIOD + TESTING_TIME_SLACK);
 
-    verify(eventHandlerMock, never()).handleEvent(any());
+    untilAsserted(() -> assertThat(eventHandlerMock.events).isEmpty());
   }
 
   @Test
@@ -120,12 +115,42 @@ public void eventNotRegisteredIfStopped() throws IOException {
   }
 
   @Test
-  public void eventNotFiredIfStopped() throws InterruptedException, IOException {
+  public void eventNotFiredIfStopped() throws IOException {
     timerEventSource.scheduleOnce(TestUtils.testCustomResource(), PERIOD);
     timerEventSource.close();
 
-    Thread.sleep(PERIOD + TESTING_TIME_SLACK);
+    untilAsserted(() -> assertThat(eventHandlerMock.events).isEmpty());
+  }
+
+  private void untilAsserted(ThrowingRunnable assertion) {
+    untilAsserted(INITIAL_DELAY, PERIOD, assertion);
+  }
+
+  private void untilAsserted(long initialDelay, long interval, ThrowingRunnable assertion) {
+    long delay = INITIAL_DELAY;
+    long period = PERIOD;
+
+    ConditionFactory cf = Awaitility.await();
+
+    if (initialDelay > 0) {
+      delay = initialDelay;
+      cf = cf.pollDelay(initialDelay, TimeUnit.MILLISECONDS);
+    }
+    if (interval > 0) {
+      period = interval;
+      cf = cf.pollInterval(interval, TimeUnit.MILLISECONDS);
+    }
+
+    cf = cf.atMost(delay + (period * 3), TimeUnit.MILLISECONDS);
+    cf.untilAsserted(assertion);
+  }
+
+  private static class CapturingEventHandler implements EventHandler {
+    private final List<Event> events = new CopyOnWriteArrayList<>();
 
-    verify(eventHandlerMock, never()).handleEvent(any());
+    @Override
+    public void handleEvent(Event event) {
+      events.add(event);
+    }
   }
 }