[GitHub] [geode] DonalEvans commented on a change in pull request #6503: GEODE-9295: prevent expiry message hangs

[GitBox <gi...@apache.org>]

DonalEvans commented on a change in pull request #6503:
URL: https://github.com/apache/geode/pull/6503#discussion_r637069833



##########
File path: geode-core/src/test/java/org/apache/geode/internal/cache/LatestLastAccessTimeMessageTest.java
##########
@@ -14,30 +14,142 @@
  */
 package org.apache.geode.internal.cache;
 
+import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
 
 import java.util.Collections;
 import java.util.Set;
 
 import org.junit.Test;
 
 import org.apache.geode.distributed.internal.ClusterDistributionManager;
-import org.apache.geode.distributed.internal.DistributionManager;
 import org.apache.geode.distributed.internal.membership.InternalDistributedMember;
+import org.apache.geode.internal.InternalStatisticsDisabledException;
 import org.apache.geode.internal.inet.LocalHostUtil;
 
 public class LatestLastAccessTimeMessageTest {
 
   @Test
-  public void processMessageShouldLookForNullCache() throws Exception {
-    final DistributionManager distributionManager = mock(DistributionManager.class);
+  public void processWithNullCacheRepliesZero() throws Exception {
     final LatestLastAccessTimeReplyProcessor replyProcessor =
         mock(LatestLastAccessTimeReplyProcessor.class);
     final InternalDistributedRegion region = mock(InternalDistributedRegion.class);
     Set<InternalDistributedMember> recipients = Collections.singleton(new InternalDistributedMember(
         LocalHostUtil.getLocalHost(), 1234));
+    final LatestLastAccessTimeMessage<String> realLastAccessTimeMessage =
+        new LatestLastAccessTimeMessage<>(replyProcessor, recipients, region, "foo");
+    final LatestLastAccessTimeMessage<String> lastAccessTimeMessage =
+        spy(realLastAccessTimeMessage);
+    lastAccessTimeMessage.setSender(new InternalDistributedMember(
+        LocalHostUtil.getLocalHost(), 12345));
+    final ClusterDistributionManager dm = mock(ClusterDistributionManager.class);
+    when(dm.getCache()).thenReturn(null);
+
+    lastAccessTimeMessage.process(dm);
+
+    verify(lastAccessTimeMessage).sendReply(dm, 0);
+  }
+
+  @Test
+  public void processWithNullRegionRepliesZero() throws Exception {
+    final LatestLastAccessTimeReplyProcessor replyProcessor =
+        mock(LatestLastAccessTimeReplyProcessor.class);
+    final InternalDistributedRegion region = mock(InternalDistributedRegion.class);
+    Set<InternalDistributedMember> recipients = Collections.singleton(new InternalDistributedMember(
+        LocalHostUtil.getLocalHost(), 1234));
+    final LatestLastAccessTimeMessage<String> realLastAccessTimeMessage =
+        new LatestLastAccessTimeMessage<>(replyProcessor, recipients, region, "foo");
+    final LatestLastAccessTimeMessage<String> lastAccessTimeMessage =
+        spy(realLastAccessTimeMessage);
+    lastAccessTimeMessage.setSender(new InternalDistributedMember(
+        LocalHostUtil.getLocalHost(), 12345));
+    final ClusterDistributionManager dm = mock(ClusterDistributionManager.class);
+    final InternalCache cache = mock(InternalCache.class);

Review comment:
       A lot of the repeated set-up in these test cases can be eliminated if you create a few fields and a `@Before` method like below. Also, you can use mock `InternalDistributedMember` instead of using real instances of the class to help keep this as much of a unit test as possible:
   ```
     private InternalDistributedRegion region;
     private ClusterDistributionManager dm;
     private LatestLastAccessTimeMessage<String> lastAccessTimeMessage;
     private InternalCache cache;
   
     @Before
     public void setUp() throws UnknownHostException {
       final LatestLastAccessTimeReplyProcessor replyProcessor =
           mock(LatestLastAccessTimeReplyProcessor.class);
       region = mock(InternalDistributedRegion.class);
       Set<InternalDistributedMember> recipients =
           Collections.singleton(mock(InternalDistributedMember.class));
       lastAccessTimeMessage =
           spy(new LatestLastAccessTimeMessage<>(replyProcessor, recipients, region, "foo"));
       lastAccessTimeMessage.setSender(mock(InternalDistributedMember.class));
       dm = mock(ClusterDistributionManager.class);
       cache = mock(InternalCache.class);
     }
   ```




-- 
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:
users@infra.apache.org