Github user bschuchardt commented on a diff in the pull request:
https://github.com/apache/geode/pull/402#discussion_r102343450
--- Diff:
geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionManagerDUnitTest.java
---
@@ -159,77 +159,74 @@ public void testConnectAfterBeingShunned() {
**/
@Test
public void testSurpriseMemberHandling() {
- VM vm0 = Host.getHost(0).getVM(0);
-
- InternalDistributedSystem sys = getSystem();
- MembershipManager mgr =
MembershipManagerHelper.getMembershipManager(sys);
+ System.setProperty(DistributionConfig.GEMFIRE_PREFIX +
"surprise-member-timeout", "3000");
try {
- InternalDistributedMember mbr =
- new InternalDistributedMember(NetworkUtils.getIPLiteral(),
12345);
+ InternalDistributedSystem sys = getSystem();
+ MembershipManager mgr =
MembershipManagerHelper.getMembershipManager(sys);
+ assertTrue(((GMSMembershipManager) mgr).isCleanupTimerStarted());
- // first make sure we can't add this as a surprise member (bug
#44566)
-
- // if the view number isn't being recorded correctly the test will
pass but the
- // functionality is broken
- Assert.assertTrue("expected view ID to be greater than zero",
mgr.getView().getViewId() > 0);
-
- int oldViewId = mbr.getVmViewId();
- mbr.setVmViewId((int) mgr.getView().getViewId() - 1);
- org.apache.geode.test.dunit.LogWriterUtils.getLogWriter()
- .info("current membership view is " + mgr.getView());
- org.apache.geode.test.dunit.LogWriterUtils.getLogWriter()
- .info("created ID " + mbr + " with view ID " +
mbr.getVmViewId());
- sys.getLogWriter()
- .info("<ExpectedException action=add>attempt to add old
member</ExpectedException>");
- sys.getLogWriter()
- .info("<ExpectedException action=add>Removing shunned GemFire
node</ExpectedException>");
try {
- boolean accepted = mgr.addSurpriseMember(mbr);
- Assert.assertTrue("member with old ID was not rejected (bug
#44566)", !accepted);
- } finally {
+ InternalDistributedMember mbr =
+ new InternalDistributedMember(NetworkUtils.getIPLiteral(),
12345);
+
+ // first make sure we can't add this as a surprise member (bug
#44566)
+
+ // if the view number isn't being recorded correctly the test will
pass but the
+ // functionality is broken
+ Assert.assertTrue("expected view ID to be greater than zero",
+ mgr.getView().getViewId() > 0);
+
+ int oldViewId = mbr.getVmViewId();
+ mbr.setVmViewId((int) mgr.getView().getViewId() - 1);
+ org.apache.geode.test.dunit.LogWriterUtils.getLogWriter()
+ .info("current membership view is " + mgr.getView());
+ org.apache.geode.test.dunit.LogWriterUtils.getLogWriter()
+ .info("created ID " + mbr + " with view ID " +
mbr.getVmViewId());
sys.getLogWriter()
- .info("<ExpectedException action=remove>attempt to add old
member</ExpectedException>");
+ .info("<ExpectedException action=add>attempt to add old
member</ExpectedException>");
--- End diff --
Thanks Kirk! I'm implementing all of your suggested changes. There are
several other places in this class that are using try/finally to remove system
properties that I'll fix as well.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
