You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by kh...@apache.org on 2017/04/20 22:27:59 UTC
[05/14] geode git commit: GEODE-2653: Fix testRemoveMember and remove
FlakyTest. This closes #437
GEODE-2653: Fix testRemoveMember and remove FlakyTest. This closes #437
Test removed self instead of the other member, and used the `any`
matcher instead of `isA`.
Cleanup GMSJoinLeaveJUnitTest.
* Change Mockito's `any` to `isA`.
* Replace some `Thread.sleep()` calls with Awaitility calls.
* Remove our `MethodExecuted` class -- this can be done with Mockito's
`verify()`.
Remove a redundant assert.
Project: http://git-wip-us.apache.org/repos/asf/geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/geode/commit/b163a8eb
Tree: http://git-wip-us.apache.org/repos/asf/geode/tree/b163a8eb
Diff: http://git-wip-us.apache.org/repos/asf/geode/diff/b163a8eb
Branch: refs/heads/feature/GEODE-2681
Commit: b163a8eb75f0834c09dbc770b443851f739c0ee8
Parents: acd5722
Author: Galen OSullivan <go...@pivotal.io>
Authored: Fri Mar 31 11:44:54 2017 -0700
Committer: Ken Howe <kh...@pivotal.io>
Committed: Thu Apr 20 15:13:17 2017 -0700
----------------------------------------------------------------------
.../gms/membership/GMSJoinLeaveJUnitTest.java | 122 +++++++------------
1 file changed, 41 insertions(+), 81 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/geode/blob/b163a8eb/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeaveJUnitTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeaveJUnitTest.java b/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeaveJUnitTest.java
index 05ab6f7..49b09ca 100644
--- a/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeaveJUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeaveJUnitTest.java
@@ -14,15 +14,16 @@
*/
package org.apache.geode.distributed.internal.membership.gms.membership;
+import static java.util.concurrent.TimeUnit.MILLISECONDS;
+import static java.util.concurrent.TimeUnit.SECONDS;
+import static org.hamcrest.core.IsEqual.equalTo;
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.isA;
-import static org.mockito.Mockito.atLeast;
-import static org.mockito.Mockito.doReturn;
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 static org.mockito.Mockito.when;
@@ -55,7 +56,6 @@ import org.apache.geode.distributed.internal.membership.gms.messages.RemoveMembe
import org.apache.geode.distributed.internal.membership.gms.messages.ViewAckMessage;
import org.apache.geode.internal.Version;
import org.apache.geode.security.AuthenticationFailedException;
-import org.apache.geode.test.junit.categories.FlakyTest;
import org.apache.geode.test.junit.categories.IntegrationTest;
import org.apache.geode.test.junit.categories.MembershipTest;
import org.junit.After;
@@ -79,7 +79,6 @@ import java.util.List;
import java.util.Properties;
import java.util.Set;
import java.util.Timer;
-import java.util.concurrent.TimeUnit;
@Category({IntegrationTest.class, MembershipTest.class})
public class GMSJoinLeaveJUnitTest {
@@ -221,7 +220,7 @@ public class GMSJoinLeaveJUnitTest {
gmsJoinLeave.processMessage(new JoinRequestMessage(mockOldMember, mockOldMember, null, -1, 0));
assertTrue("JoinRequest should not have been added to view request",
gmsJoinLeave.getViewRequests().size() == 0);
- verify(messenger).send(any(JoinResponseMessage.class));
+ verify(messenger).send(isA(JoinResponseMessage.class));
}
@Test
@@ -232,7 +231,7 @@ public class GMSJoinLeaveJUnitTest {
NetView v = new NetView(mockMembers[0], 2, members);
InstallViewMessage message = getInstallViewMessage(v, null, false);
gmsJoinLeave.processMessage(message);
- verify(manager).forceDisconnect(any(String.class));
+ verify(manager).forceDisconnect(isA(String.class));
}
@@ -248,7 +247,7 @@ public class GMSJoinLeaveJUnitTest {
.processMessage(new JoinRequestMessage(mockMembers[0], mockMembers[0], credentials, -1, 0));
assertTrue("JoinRequest should not have been added to view request",
gmsJoinLeave.getViewRequests().size() == 0);
- verify(messenger).send(any(JoinResponseMessage.class));
+ verify(messenger).send(isA(JoinResponseMessage.class));
}
@Test
@@ -263,7 +262,7 @@ public class GMSJoinLeaveJUnitTest {
.processMessage(new JoinRequestMessage(mockMembers[0], mockMembers[0], null, -1, 0));
assertTrue("JoinRequest should not have been added to view request",
gmsJoinLeave.getViewRequests().size() == 0);
- verify(messenger).send(any(JoinResponseMessage.class));
+ verify(messenger).send(isA(JoinResponseMessage.class));
}
// This test does not test the actual join process but rather that the join response gets logged�
@@ -310,7 +309,7 @@ public class GMSJoinLeaveJUnitTest {
NetView netView = new NetView(coordinator, viewId, members);
InstallViewMessage installViewMessage = getInstallViewMessage(netView, credentials, true);
gmsJoinLeave.processMessage(installViewMessage);
- verify(messenger).send(any(ViewAckMessage.class));
+ verify(messenger).send(isA(ViewAckMessage.class));
// install the view
installViewMessage = getInstallViewMessage(netView, credentials, false);
@@ -327,16 +326,12 @@ public class GMSJoinLeaveJUnitTest {
return memberList;
}
- @Category(FlakyTest.class) // GEODE-2653: flaky due to Thread.sleep
@Test
public void testRemoveMember() throws Exception {
initMocks();
prepareAndInstallView(mockMembers[0], createMemberList(mockMembers[0], gmsJoinLeaveMemberId));
- MethodExecuted removeMessageSent = new MethodExecuted();
- when(messenger.send(any(RemoveMemberMessage.class))).thenAnswer(removeMessageSent);
- gmsJoinLeave.remove(mockMembers[0], "removing for test");
- Thread.sleep(ServiceConfig.MEMBER_REQUEST_COLLECTION_INTERVAL * 2);
- assertTrue(removeMessageSent.methodExecuted);
+ gmsJoinLeave.remove(gmsJoinLeaveMemberId, "removing for test");
+ verify(messenger, timeout(2000).atLeastOnce()).send(isA(RemoveMemberMessage.class));
}
@Test
@@ -344,11 +339,10 @@ public class GMSJoinLeaveJUnitTest {
initMocks();
prepareAndInstallView(mockMembers[0],
createMemberList(mockMembers[0], mockMembers[1], gmsJoinLeaveMemberId));
- MethodExecuted removeMessageSent = new MethodExecuted();
- when(messenger.send(any(RemoveMemberMessage.class))).thenAnswer(removeMessageSent);
assertFalse(gmsJoinLeave.isMemberLeaving(mockMembers[0]));
assertFalse(gmsJoinLeave.isMemberLeaving(mockMembers[1]));
gmsJoinLeave.remove(mockMembers[0], "removing for test");
+ verify(messenger, timeout(2000).atLeastOnce()).send(isA(RemoveMemberMessage.class));
assertTrue(gmsJoinLeave.isMemberLeaving(mockMembers[0]));
LeaveRequestMessage msg =
new LeaveRequestMessage(gmsJoinLeave.getMemberID(), mockMembers[1], "leaving for test");
@@ -364,20 +358,17 @@ public class GMSJoinLeaveJUnitTest {
initMocks();
final int viewInstallationTime = 15000;
- when(healthMonitor.checkIfAvailable(any(InternalDistributedMember.class), any(String.class),
- any(Boolean.class))).thenReturn(true);
+ when(healthMonitor.checkIfAvailable(isA(InternalDistributedMember.class), isA(String.class),
+ isA(Boolean.class))).thenReturn(true);
gmsJoinLeave.delayViewCreationForTest(5000); // ensures multiple requests are queued for a view
// change
GMSJoinLeaveTestHelper.becomeCoordinatorForTest(gmsJoinLeave);
- NetView oldView = null;
- long giveup = System.currentTimeMillis() + viewInstallationTime;
- while (System.currentTimeMillis() < giveup && oldView == null) {
- Thread.sleep(500);
- oldView = gmsJoinLeave.getView();
- }
- assertTrue(oldView != null); // it should have become coordinator and installed a view
+ Awaitility.await().atMost(viewInstallationTime, MILLISECONDS)
+ .until(() -> gmsJoinLeave.getView() != null);
+
+ NetView oldView = gmsJoinLeave.getView();
NetView newView = new NetView(oldView, oldView.getViewId() + 1);
newView.add(mockMembers[1]);
@@ -387,12 +378,8 @@ public class GMSJoinLeaveJUnitTest {
gmsJoinLeave.memberShutdown(mockMembers[1], "shutting down for test");
gmsJoinLeave.remove(mockMembers[1], "removing for test");
- giveup = System.currentTimeMillis() + viewInstallationTime;
- while (System.currentTimeMillis() < giveup
- && gmsJoinLeave.getView().getViewId() == newView.getViewId()) {
- Thread.sleep(500);
- }
- assertTrue(gmsJoinLeave.getView().getViewId() > newView.getViewId());
+ Awaitility.await().atMost(viewInstallationTime, MILLISECONDS)
+ .until(() -> gmsJoinLeave.getView().getViewId() > newView.getViewId());
assertFalse(gmsJoinLeave.getView().getCrashedMembers().contains(mockMembers[1]));
}
@@ -432,19 +419,7 @@ public class GMSJoinLeaveJUnitTest {
gmsJoinLeave.processMessage(installViewMessage);
Assert.assertNotEquals(netView, gmsJoinLeave.getView());
- verify(mockManager).forceDisconnect(any(String.class));
- }
-
- @SuppressWarnings("rawtypes")
- private class MethodExecuted implements Answer {
- private boolean methodExecuted = false;
-
- @Override
- public Object answer(InvocationOnMock invocation) {
- // do we only expect a join response on a failure?
- methodExecuted = true;
- return null;
- }
+ verify(mockManager).forceDisconnect(isA(String.class));
}
@Test
@@ -517,8 +492,8 @@ public class GMSJoinLeaveJUnitTest {
@Test
public void testDuplicateJoinRequestDoesNotCauseNewView() throws Exception {
initMocks();
- when(healthMonitor.checkIfAvailable(any(InternalDistributedMember.class), any(String.class),
- any(Boolean.class))).thenReturn(true);
+ when(healthMonitor.checkIfAvailable(isA(InternalDistributedMember.class), isA(String.class),
+ isA(Boolean.class))).thenReturn(true);
gmsJoinLeave.unitTesting.add("noRandomViewChange");
prepareAndInstallView(gmsJoinLeaveMemberId,
createMemberList(gmsJoinLeaveMemberId, mockMembers[0]));
@@ -546,8 +521,8 @@ public class GMSJoinLeaveJUnitTest {
}
assertTrue("expected member to only be in the view once: " + mockMembers[2] + "; view: " + view,
occurrences == 1);
- verify(healthMonitor, times(5)).checkIfAvailable(any(InternalDistributedMember.class),
- any(String.class), any(Boolean.class));
+ verify(healthMonitor, times(5)).checkIfAvailable(isA(InternalDistributedMember.class),
+ isA(String.class), isA(Boolean.class));
}
@@ -610,11 +585,7 @@ public class GMSJoinLeaveJUnitTest {
public void testBecomeCoordinatorOnStartup() throws Exception {
initMocks();
GMSJoinLeaveTestHelper.becomeCoordinatorForTest(gmsJoinLeave);
- long giveup = System.currentTimeMillis() + 20000;
- while (System.currentTimeMillis() < giveup && !gmsJoinLeave.isCoordinator()) {
- Thread.sleep(1000);
- }
- assertTrue(gmsJoinLeave.isCoordinator());
+ Awaitility.await().atMost(20, SECONDS).until(() -> gmsJoinLeave.isCoordinator());
}
@Test
@@ -727,7 +698,7 @@ public class GMSJoinLeaveJUnitTest {
GMSJoinLeaveTestHelper.becomeCoordinatorForTest(gmsJoinLeave);
NetworkPartitionMessage message = new NetworkPartitionMessage();
gmsJoinLeave.processMessage(message);
- verify(manager).forceDisconnect(any(String.class));
+ verify(manager).forceDisconnect(isA(String.class));
}
@@ -765,7 +736,7 @@ public class GMSJoinLeaveJUnitTest {
installViewMessage = getInstallViewMessage(partitionView, credentials, false);
gmsJoinLeave.processMessage(installViewMessage);
- verify(manager, never()).forceDisconnect(any(String.class));
+ verify(manager, never()).forceDisconnect(isA(String.class));
verify(manager).quorumLost(crashes, newView);
}
@@ -789,8 +760,8 @@ public class GMSJoinLeaveJUnitTest {
@Test
public void testNoViewAckCausesRemovalMessage() throws Exception {
initMocks(true);
- when(healthMonitor.checkIfAvailable(any(InternalDistributedMember.class), any(String.class),
- any(Boolean.class))).thenReturn(false);
+ when(healthMonitor.checkIfAvailable(isA(InternalDistributedMember.class), isA(String.class),
+ isA(Boolean.class))).thenReturn(false);
prepareAndInstallView(mockMembers[0], createMemberList(mockMembers[0], gmsJoinLeaveMemberId));
NetView oldView = gmsJoinLeave.getView();
NetView newView = new NetView(oldView, oldView.getViewId() + 1);
@@ -804,15 +775,13 @@ public class GMSJoinLeaveJUnitTest {
InstallViewMessage installViewMessage = getInstallViewMessage(newView, credentials, false);
gmsJoinLeave.processMessage(installViewMessage);
- long giveup = System.currentTimeMillis() + (2000 * 3); // this test's member-timeout * 3
- while (System.currentTimeMillis() < giveup
- && gmsJoinLeave.getView().getViewId() == oldView.getViewId()) {
- Thread.sleep(1000);
- }
+ // this test's member-timeout * 3
+ Awaitility.await().atMost(6, SECONDS)
+ .until(() -> gmsJoinLeave.getView().getViewId() != oldView.getViewId());
assertTrue(gmsJoinLeave.isCoordinator());
// wait for suspect processing
- Thread.sleep(10000);
- verify(healthMonitor, atLeast(1)).checkIfAvailable(isA(DistributedMember.class),
+
+ verify(healthMonitor, timeout(10000).atLeast(1)).checkIfAvailable(isA(DistributedMember.class),
isA(String.class), isA(Boolean.class));
// verify(messenger, atLeast(1)).send(isA(RemoveMemberMessage.class));
}
@@ -920,7 +889,7 @@ public class GMSJoinLeaveJUnitTest {
NetView netView = new NetView(coordinator, viewId, members);
InstallViewMessage installViewMessage = getInstallViewMessage(netView, credentials, false);
gmsJoinLeave.processMessage(installViewMessage);
- // verify(messenger).send(any(ViewAckMessage.class));
+ // verify(messenger).send(isA(ViewAckMessage.class));
}
@Test
@@ -946,11 +915,9 @@ public class GMSJoinLeaveJUnitTest {
new JoinRequestMessage(mockMembers[0], mockMembers[0], credentials, -1, 0));
int viewRequests = gmsJoinLeave.getViewRequests().size();
- assertTrue("There should be 1 viewRequest but found " + viewRequests, viewRequests == 1);
- Thread.sleep(2 * ServiceConfig.MEMBER_REQUEST_COLLECTION_INTERVAL);
-
- viewRequests = gmsJoinLeave.getViewRequests().size();
- assertEquals("Found view requests: " + gmsJoinLeave.getViewRequests(), 0, viewRequests);
+ assertEquals("There should be 1 viewRequest", 1, viewRequests);
+ Awaitility.await().atMost(2 * ServiceConfig.MEMBER_REQUEST_COLLECTION_INTERVAL, MILLISECONDS)
+ .until(() -> gmsJoinLeave.getViewRequests().size(), equalTo(0));
} finally {
System.getProperties().remove(GMSJoinLeave.BYPASS_DISCOVERY_PROPERTY);
}
@@ -1105,7 +1072,7 @@ public class GMSJoinLeaveJUnitTest {
gmsJoinLeave.processMessage(msg);
}
- Awaitility.await("waiting for view creator to stop").atMost(5000, TimeUnit.MILLISECONDS)
+ Awaitility.await("waiting for view creator to stop").atMost(5000, MILLISECONDS)
.until(() -> !gmsJoinLeave.getViewCreator().isAlive());
assertEquals(1, gmsJoinLeave.getView().getViewId());
@@ -1148,14 +1115,7 @@ public class GMSJoinLeaveJUnitTest {
vack.setSender(gmsJoinLeaveMemberId);
gmsJoinLeave.processMessage(vack);
- int tries = 0;
- while (!vc.waiting) {
- if (tries > 30) {
- Assert.fail("view creator never finished");
- }
- tries++;
- Thread.sleep(1000);
- }
+ Awaitility.await("view creator finishes").atMost(30, SECONDS).until(() -> vc.waiting);
NetView newView = gmsJoinLeave.getView();
System.out.println("new view is " + newView);
assertTrue(newView.contains(mockMembers[1]));