mynameborat 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_r399517388
##########
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:
I see you are not the author of tests. But, do we need to wait for 2 seconds
here for this test? Isn't verifying the scheduler for no task submissions below
sufficient?
----------------------------------------------------------------
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
