[GitHub] [geode] kirklund commented on a change in pull request #6827: GEODE-9522: When a server is force disconnected, it should set shutdo…

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

kirklund commented on a change in pull request #6827:
URL: https://github.com/apache/geode/pull/6827#discussion_r720415820



##########
File path: geode-core/src/distributedTest/java/org/apache/geode/cache30/ReconnectWithCacheXMLDUnitTest.java
##########
@@ -118,7 +117,7 @@ public void beforeMembershipFailure(String reason, Throwable cause) {
       }
     });
     MembershipManagerHelper.crashDistributedSystem(cache.getDistributedSystem());
-    assertTrue(membershipFailed.get());
+    await().untilAsserted(() -> membershipFailed.get());

Review comment:
       This line won't do anything as written. It needs to either be:
   ```
   await().until(() -> membershipFailed.get());
   ```
   Or:
   ```
   await().untilAsserted(() -> assertThat(membershipFailed.get()).isTrue());
   ```

##########
File path: geode-membership/src/integrationTest/java/org/apache/geode/distributed/internal/membership/gms/GMSMembershipJUnitTest.java
##########
@@ -197,7 +202,54 @@ public void testSendMessage() throws Exception {
     }
   }
 
+  @Test
+  public void testForceDisconnectUncleanShutdownDS() throws Exception {
+    final String reason = "For testing";
+    manager = spy(manager);
+    manager.getGMSManager().start();
+    manager.getGMSManager().started();
+    MemberIdentifier myGMSMemberId = myMemberId;
+    List<MemberIdentifier> gmsMembers =
+        members.stream().map(x -> ((MemberIdentifier) x)).collect(Collectors.toList());
+    manager.getGMSManager().installView(new GMSMembershipView<>(myGMSMemberId, 1, gmsMembers));
+
+    manager.inhibitForcedDisconnectLogging(true);
+    GMSMembership.ManagerImpl managerImpl = (GMSMembership.ManagerImpl) manager.getGMSManager();
+    managerImpl = spy(managerImpl);
+    when(manager.getGMSManager()).thenReturn(managerImpl);
+
+    manager.forceDisconnect(reason);
+    InOrder inOrder = inOrder(managerImpl, directChannelCallback, listener);
+    inOrder.verify(managerImpl, times(1)).uncleanShutdownDS(eq(reason),
+        isA(MemberDisconnectedException.class));
+    inOrder.verify(directChannelCallback, timeout(3000).times(1)).forcedDisconnect();
+    inOrder.verify(listener, timeout(3000).times(1)).forcedDisconnectHappened(eq(reason));

Review comment:
       Is there a reason you need a timeout and if so, does it need to be less than `GeodeAwaitility.getTimeout`?




-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org