From: Tom Pantelis <tpanteli@brocade.com>
Date: Wed, 15 Feb 2017 16:02:57 +0000 (-0500)
Subject: Fix intermittent failure in testCloseCandidateRegistrationInQuickSuccession
X-Git-Tag: release/carbon~261
X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=controller.git;a=commitdiff_plain;h=15741ff84dcd9bab5ab382a3ceaa48ceddffb9a3

Fix intermittent failure in testCloseCandidateRegistrationInQuickSuccession

java.lang.IllegalStateException: Optional.get() cannot be called on an absent value
  at com.google.common.base.Absent.get(Absent.java:47)
  at org.opendaylight.controller.cluster.datastore.entityownership.DistributedEntityOwnershipIntegrationTest.testCloseCandidateRegistrationInQuickSuccession(DistributedEntityOwnershipIntegrationTest.java:512)

Code:

if (!leaderEntityOwnershipService.getOwnershipState(ENTITY1).isPresent()
  || leaderEntityOwnershipService.getOwnershipState(ENTITY1).get() ==
                                               EntityOwnershipState.NO_OWNER

The code inlines calls to getOwnershipState so it's possible the first call
returns a present Optional and the second call returns absent which leads to
the failure. It's safer to capture the Optional in a lcoal var.

Change-Id: I9baa120efc9924dc820435dd63217b4598731a13
Signed-off-by: Tom Pantelis <tpanteli@brocade.com>
---

diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/entityownership/DistributedEntityOwnershipIntegrationTest.java b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/entityownership/DistributedEntityOwnershipIntegrationTest.java
index b4ad323c49..e5c17f54c6 100644
--- a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/entityownership/DistributedEntityOwnershipIntegrationTest.java
+++ b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/entityownership/DistributedEntityOwnershipIntegrationTest.java
@@ -44,6 +44,7 @@ import org.mockito.ArgumentCaptor;
 import org.mockito.Mock;
 import org.mockito.Mockito;
 import org.mockito.MockitoAnnotations;
+import org.mockito.exceptions.base.MockitoException;
 import org.opendaylight.controller.cluster.datastore.AbstractDataStore;
 import org.opendaylight.controller.cluster.datastore.DatastoreContext;
 import org.opendaylight.controller.cluster.datastore.IntegrationTestKit;
@@ -486,10 +487,13 @@ public class DistributedEntityOwnershipIntegrationTest {
 
         final DOMEntityOwnershipCandidateRegistration candidate2 =
                 follower1EntityOwnershipService.registerCandidate(ENTITY1);
+        verify(follower1MockListener, timeout(5000)).ownershipChanged(ownershipChange(ENTITY1, false, false, true));
+
         final DOMEntityOwnershipCandidateRegistration candidate3 =
                 follower2EntityOwnershipService.registerCandidate(ENTITY1);
+        verify(follower2MockListener, timeout(5000)).ownershipChanged(ownershipChange(ENTITY1, false, false, true));
 
-        Mockito.reset(leaderMockListener);
+        Mockito.reset(leaderMockListener, follower1MockListener, follower2MockListener);
 
         ArgumentCaptor<DOMEntityOwnershipChange> leaderChangeCaptor =
                 ArgumentCaptor.forClass(DOMEntityOwnershipChange.class);
@@ -508,20 +512,20 @@ public class DistributedEntityOwnershipIntegrationTest {
         boolean passed = false;
         for (int i = 0; i < 100; i++) {
             Uninterruptibles.sleepUninterruptibly(50, TimeUnit.MILLISECONDS);
-            if (!leaderEntityOwnershipService.getOwnershipState(ENTITY1).isPresent()
-                    || leaderEntityOwnershipService.getOwnershipState(ENTITY1).get() == EntityOwnershipState.NO_OWNER
-                            && follower1EntityOwnershipService.getOwnershipState(ENTITY1).isPresent()
-                            && follower1EntityOwnershipService.getOwnershipState(ENTITY1)
-                                    .get() == EntityOwnershipState.NO_OWNER
-                            && follower2EntityOwnershipService.getOwnershipState(ENTITY1).isPresent()
-                            && follower2EntityOwnershipService.getOwnershipState(ENTITY1)
-                                    .get() == EntityOwnershipState.NO_OWNER
-                            && leaderChangeCaptor.getAllValues().size() > 0
-                            && !leaderChangeCaptor.getValue().getState().hasOwner()
-                            && leaderChangeCaptor.getAllValues().size() > 0
-                            && !follower1ChangeCaptor.getValue().getState().hasOwner()
-                            && leaderChangeCaptor.getAllValues().size() > 0
-                            && !follower2ChangeCaptor.getValue().getState().hasOwner()) {
+            final Optional<EntityOwnershipState> leaderState = leaderEntityOwnershipService.getOwnershipState(ENTITY1);
+            final Optional<EntityOwnershipState> follower1State =
+                    follower1EntityOwnershipService.getOwnershipState(ENTITY1);
+            final Optional<EntityOwnershipState> follower2State =
+                    follower2EntityOwnershipService.getOwnershipState(ENTITY1);
+            final Optional<DOMEntityOwnershipChange> leaderChange = getValueSafely(leaderChangeCaptor);
+            final Optional<DOMEntityOwnershipChange> follower1Change = getValueSafely(follower1ChangeCaptor);
+            final Optional<DOMEntityOwnershipChange> follower2Change = getValueSafely(follower2ChangeCaptor);
+            if (!leaderState.isPresent() || leaderState.get() == EntityOwnershipState.NO_OWNER
+                    && follower1State.isPresent() && follower1State.get() == EntityOwnershipState.NO_OWNER
+                    && follower2State.isPresent() && follower2State.get() == EntityOwnershipState.NO_OWNER
+                    && leaderChange.isPresent() && !leaderChange.get().getState().hasOwner()
+                    && follower1Change.isPresent() && !follower1Change.get().getState().hasOwner()
+                    && follower2Change.isPresent() && !follower2Change.get().getState().hasOwner()) {
                 passed = true;
                 break;
             }
@@ -530,6 +534,15 @@ public class DistributedEntityOwnershipIntegrationTest {
         assertTrue("No ownership change message was sent with hasOwner=false", passed);
     }
 
+    private static Optional<DOMEntityOwnershipChange> getValueSafely(ArgumentCaptor<DOMEntityOwnershipChange> captor) {
+        try {
+            return Optional.fromNullable(captor.getValue());
+        } catch (MockitoException e) {
+            // No value was captured
+            return Optional.absent();
+        }
+    }
+
     /**
      * Tests bootstrapping the entity-ownership shard when there's no shards initially configured for local
      * member. The entity-ownership shard is initially created as inactive (ie remains a follower), requiring