cameronlee314 commented on a change in pull request #1334: SAMZA-2496:
TestContainerHeartbeatMonitor does not properly stop the
ContainerHeartbeatMonitor
URL: https://github.com/apache/samza/pull/1334#discussion_r399520833
##########
File path:
samza-core/src/test/java/org/apache/samza/container/TestContainerHeartbeatMonitor.java
##########
@@ -20,44 +20,77 @@
package org.apache.samza.container;
import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.TimeUnit;
-import junit.framework.Assert;
import org.junit.Test;
-import static org.mockito.Mockito.*;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.anyLong;
+import static org.mockito.Matchers.eq;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
-public class TestContainerHeartbeatMonitor {
+public class TestContainerHeartbeatMonitor {
@Test
- public void testCallbackWhenHeartbeatDead()
- throws InterruptedException {
+ public void testCallbackWhenHeartbeatDead() throws InterruptedException {
ContainerHeartbeatClient mockClient = mock(ContainerHeartbeatClient.class);
CountDownLatch countDownLatch = new CountDownLatch(1);
- Runnable onExpired = () -> {
- countDownLatch.countDown();
- };
- ContainerHeartbeatMonitor monitor = new
ContainerHeartbeatMonitor(onExpired, mockClient);
+ Runnable onExpired = countDownLatch::countDown;
ContainerHeartbeatResponse response = new
ContainerHeartbeatResponse(false);
when(mockClient.requestHeartbeat()).thenReturn(response);
+ ScheduledExecutorService scheduler = buildScheduledExecutorService();
+ ContainerHeartbeatMonitor monitor = new
ContainerHeartbeatMonitor(onExpired, mockClient, scheduler);
monitor.start();
boolean success = countDownLatch.await(2, TimeUnit.SECONDS);
- Assert.assertTrue(success);
+ assertTrue(success);
+ // check that the shutdown task got submitted, but don't actually execute
it since it will shut down the process
+ verify(scheduler).schedule(any(Runnable.class), eq((long)
ContainerHeartbeatMonitor.SHUTDOWN_TIMOUT_MS),
+ eq(TimeUnit.MILLISECONDS));
+
+ monitor.stop();
+ verify(scheduler).shutdown();
}
@Test
- public void testDoesNotCallbackWhenHeartbeatAlive()
- throws InterruptedException {
+ public void testDoesNotCallbackWhenHeartbeatAlive() throws
InterruptedException {
ContainerHeartbeatClient client = mock(ContainerHeartbeatClient.class);
CountDownLatch countDownLatch = new CountDownLatch(1);
- Runnable onExpired = () -> {
- countDownLatch.countDown();
- };
- ContainerHeartbeatMonitor monitor = new
ContainerHeartbeatMonitor(onExpired, client);
+ Runnable onExpired = countDownLatch::countDown;
ContainerHeartbeatResponse response = new ContainerHeartbeatResponse(true);
when(client.requestHeartbeat()).thenReturn(response);
+ ScheduledExecutorService scheduler = buildScheduledExecutorService();
+ ContainerHeartbeatMonitor monitor = new
ContainerHeartbeatMonitor(onExpired, client, scheduler);
monitor.start();
boolean success = countDownLatch.await(2, TimeUnit.SECONDS);
Review comment:
The task executes asynchronously, so this 2 seconds gives some time to make
sure that it doesn't actually execute. If we check for no task submission
immediately, and there was a bug which caused the task to be submitted, it's
possible that the test would still pass because the verification happened
before the async execution.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
