Improve CheckUtil testss

-enforce checkstyle
-enforce findbug

Change-Id: I9d3dd4dc6b6b6192d8015f0d9f11a4c7082e7297
Signed-off-by: Claudio D. Gasparini <claudio.gasparini@pantheon.tech>
(cherry picked from commit 3fc26db07a4eadd2c6ed4d89b6beb42fce9cb568)

diff --git a/testtool-util/pom.xml b/testtool-util/pom.xml
index 1ca5799a7ebacac1520df21176507c04af874616..33cdd58759fabfe9f20122a129e0540c51c19283 100644 (file)
--- a/testtool-util/pom.xml
+++ b/testtool-util/pom.xml
@@ -45,6 +45,11 @@
             <artifactId>netty-common</artifactId>
         </dependency>
         <!-- Testing dependencies -->
+        <dependency>
+            <groupId>org.opendaylight.mdsal.model</groupId>
+            <artifactId>ietf-topology</artifactId>
+            <scope>test</scope>
+        </dependency>
         <dependency>
             <groupId>junit</groupId>
             <artifactId>junit</artifactId>
@@ -58,6 +63,17 @@
             <artifactId>netty-transport</artifactId>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>org.opendaylight.controller</groupId>
+            <artifactId>sal-binding-broker-impl</artifactId>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.opendaylight.controller</groupId>
+            <artifactId>sal-binding-broker-impl</artifactId>
+            <scope>test</scope>
+            <type>test-jar</type>
+        </dependency>
     </dependencies>
 
     <build>
@@ -84,6 +100,20 @@
                     </execution>
                 </executions>
             </plugin>
+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-checkstyle-plugin</artifactId>
+                <configuration>
+                    <propertyExpansion>checkstyle.violationSeverity=error</propertyExpansion>
+                </configuration>
+            </plugin>
+            <plugin>
+                <groupId>org.codehaus.mojo</groupId>
+                <artifactId>findbugs-maven-plugin</artifactId>
+                <configuration>
+                    <failOnError>true</failOnError>
+                </configuration>
+            </plugin>
         </plugins>
     </build>
 

diff --git a/testtool-util/src/main/java/org/opendaylight/protocol/util/CheckUtil.java b/testtool-util/src/main/java/org/opendaylight/protocol/util/CheckUtil.java
index 2bc17aa99dddc3f5cbe3b374da0c8cdae1b5ec6f..da0c0545f2082aae912ede7507c4b840d93f7593 100644 (file)
--- a/testtool-util/src/main/java/org/opendaylight/protocol/util/CheckUtil.java
+++ b/testtool-util/src/main/java/org/opendaylight/protocol/util/CheckUtil.java
@@ -10,6 +10,7 @@ package org.opendaylight.protocol.util;
 import static org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType.CONFIGURATION;
 import static org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType.OPERATIONAL;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Optional;
 import com.google.common.base.Stopwatch;
 import com.google.common.base.Verify;
@@ -43,21 +44,35 @@ public final class CheckUtil {
         Verify.verify(future.isSuccess());
     }
 
-    public static <R, T extends DataObject> R readDataOperational(final DataBroker dataBroker, final InstanceIdentifier<T> iid,
-        final Function<T, R> function) throws ReadFailedException {
-        return readData(dataBroker, OPERATIONAL, iid, function);
+    public static <R, T extends DataObject> R readDataOperational(final DataBroker dataBroker,
+            final InstanceIdentifier<T> iid, final Function<T, R> function) throws ReadFailedException {
+        return readDataOperational(dataBroker, iid, function, TIMEOUT);
     }
 
-    public static <R, T extends DataObject> R readDataConfiguration(final DataBroker dataBroker, final InstanceIdentifier<T> iid,
-        final Function<T, R> function) throws ReadFailedException {
-        return readData(dataBroker, CONFIGURATION, iid, function);
+    @VisibleForTesting
+    static <R, T extends DataObject> R readDataOperational(final DataBroker dataBroker,
+            final InstanceIdentifier<T> iid, final Function<T, R> function, final int timeout)
+            throws ReadFailedException {
+        return readData(dataBroker, OPERATIONAL, iid, function, timeout);
+    }
+
+    public static <R, T extends DataObject> R readDataConfiguration(final DataBroker dataBroker,
+            final InstanceIdentifier<T> iid, final Function<T, R> function) throws ReadFailedException {
+        return readDataConfiguration(dataBroker, iid, function, TIMEOUT);
+    }
+
+    @VisibleForTesting
+    static <R, T extends DataObject> R readDataConfiguration(final DataBroker dataBroker,
+            final InstanceIdentifier<T> iid, final Function<T, R> function, final int timeout) throws ReadFailedException {
+        return readData(dataBroker, CONFIGURATION, iid, function, timeout);
     }
 
     private static <R, T extends DataObject> R readData(final DataBroker dataBroker, final LogicalDatastoreType ldt,
-        final InstanceIdentifier<T> iid, final Function<T, R> function) throws ReadFailedException {
+            final InstanceIdentifier<T> iid, final Function<T, R> function, final int timeout)
+            throws ReadFailedException {
         AssertionError lastError = null;
         final Stopwatch sw = Stopwatch.createStarted();
-        while (sw.elapsed(TimeUnit.SECONDS) <= TIMEOUT) {
+        while (sw.elapsed(TimeUnit.SECONDS) <= timeout) {
             try (final ReadOnlyTransaction tx = dataBroker.newReadOnlyTransaction()) {
                 final Optional<T> data = tx.read(ldt, iid).checkedGet();
                 if (data.isPresent()) {
@@ -73,28 +88,28 @@ public final class CheckUtil {
         throw lastError;
     }
 
-    public static <T extends DataObject> T checkPresentOperational(final DataBroker dataBroker, final InstanceIdentifier<T> iid)
-        throws ReadFailedException {
-        return readData(dataBroker, OPERATIONAL, iid, bgpRib -> bgpRib);
+    public static <T extends DataObject> T checkPresentOperational(final DataBroker dataBroker,
+            final InstanceIdentifier<T> iid) throws ReadFailedException {
+        return readData(dataBroker, OPERATIONAL, iid, bgpRib -> bgpRib, TIMEOUT);
     }
 
     public static <T extends DataObject> T checkPresentConfiguration(final DataBroker dataBroker,
-        final InstanceIdentifier<T> iid) throws ReadFailedException {
-        return readData(dataBroker, CONFIGURATION, iid, bgpRib -> bgpRib);
+            final InstanceIdentifier<T> iid) throws ReadFailedException {
+        return readData(dataBroker, CONFIGURATION, iid, bgpRib -> bgpRib, TIMEOUT);
     }
 
     public static <T extends DataObject> void checkNotPresentOperational(final DataBroker dataBroker,
-        final InstanceIdentifier<T> iid) throws ReadFailedException {
+            final InstanceIdentifier<T> iid) throws ReadFailedException {
         checkNotPresent(dataBroker, OPERATIONAL, iid);
     }
 
     public static <T extends DataObject> void checkNotPresentConfiguration(final DataBroker dataBroker,
-        final InstanceIdentifier<T> iid) throws ReadFailedException {
+            final InstanceIdentifier<T> iid) throws ReadFailedException {
         checkNotPresent(dataBroker, CONFIGURATION, iid);
     }
 
     private static <T extends DataObject> void checkNotPresent(final DataBroker dataBroker,
-        final LogicalDatastoreType ldt, final InstanceIdentifier<T> iid) throws ReadFailedException {
+            final LogicalDatastoreType ldt, final InstanceIdentifier<T> iid) throws ReadFailedException {
         AssertionError lastError = null;
         final Stopwatch sw = Stopwatch.createStarted();
         while (sw.elapsed(TimeUnit.SECONDS) <= 10) {
@@ -113,9 +128,13 @@ public final class CheckUtil {
     }
 
     public static void checkEquals(final CheckEquals function) throws Exception {
+        checkEquals(function, TIMEOUT);
+    }
+
+    public static void checkEquals(final CheckEquals function, final int timeout) throws Exception {
         AssertionError lastError = null;
         final Stopwatch sw = Stopwatch.createStarted();
-        while (sw.elapsed(TimeUnit.SECONDS) <= TIMEOUT) {
+        while (sw.elapsed(TimeUnit.SECONDS) <= timeout) {
             try {
                 function.check();
                 return;
@@ -128,9 +147,15 @@ public final class CheckUtil {
     }
 
     public static void checkReceivedMessages(final ListenerCheck listener, final int numberOfMessages)
-        throws ReadFailedException {
+            throws ReadFailedException {
+        checkReceivedMessages(listener, numberOfMessages, TIMEOUT);
+    }
+
+    @VisibleForTesting
+    static void checkReceivedMessages(final ListenerCheck listener, final int numberOfMessages,
+            final int timeout) throws ReadFailedException {
         final Stopwatch sw = Stopwatch.createStarted();
-        while (sw.elapsed(TimeUnit.SECONDS) <= TIMEOUT) {
+        while (sw.elapsed(TimeUnit.SECONDS) <= timeout) {
             if (listener.getListMessageSize() != numberOfMessages) {
                 Uninterruptibles.sleepUninterruptibly(SLEEP_FOR, TimeUnit.MILLISECONDS);
             } else {
@@ -138,12 +163,13 @@ public final class CheckUtil {
             }
         }
         throw new AssertionError("Expected " + numberOfMessages + " but received "
-            + listener.getListMessageSize());
+                + listener.getListMessageSize());
     }
 
     public interface ListenerCheck {
         int getListMessageSize();
     }
+
     @FunctionalInterface
     public interface CheckEquals {
         void check() throws ExecutionException, InterruptedException;

diff --git a/testtool-util/src/test/java/org/opendaylight/protocol/util/CheckUtilTest.java b/testtool-util/src/test/java/org/opendaylight/protocol/util/CheckUtilTest.java
index 33c74e7a9de86a22176ce784fa17169c143ef0a2..99be9d9804ef219157838ae0aaac5fa5b2506214 100644 (file)
--- a/testtool-util/src/test/java/org/opendaylight/protocol/util/CheckUtilTest.java
+++ b/testtool-util/src/test/java/org/opendaylight/protocol/util/CheckUtilTest.java
@@ -7,69 +7,57 @@
  */
 package org.opendaylight.protocol.util;
 
+import static junit.framework.TestCase.assertNull;
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.Matchers.any;
 import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.when;
 import static org.opendaylight.protocol.util.CheckUtil.checkEquals;
+import static org.opendaylight.protocol.util.CheckUtil.checkNotPresentConfiguration;
 import static org.opendaylight.protocol.util.CheckUtil.checkNotPresentOperational;
+import static org.opendaylight.protocol.util.CheckUtil.checkPresentConfiguration;
+import static org.opendaylight.protocol.util.CheckUtil.checkPresentOperational;
 import static org.opendaylight.protocol.util.CheckUtil.checkReceivedMessages;
+import static org.opendaylight.protocol.util.CheckUtil.readDataConfiguration;
 import static org.opendaylight.protocol.util.CheckUtil.readDataOperational;
 import static org.opendaylight.protocol.util.CheckUtil.waitFutureSuccess;
 
-import com.google.common.base.Optional;
 import com.google.common.base.VerifyException;
-import com.google.common.util.concurrent.CheckedFuture;
 import io.netty.channel.ChannelFuture;
 import io.netty.util.concurrent.GenericFutureListener;
-import java.lang.reflect.Constructor;
-import java.lang.reflect.InvocationTargetException;
+import java.util.concurrent.ExecutionException;
 import org.junit.Before;
 import org.junit.Test;
 import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
-import org.opendaylight.controller.md.sal.binding.api.DataBroker;
-import org.opendaylight.controller.md.sal.binding.api.ReadOnlyTransaction;
+import org.opendaylight.controller.md.sal.binding.api.WriteTransaction;
+import org.opendaylight.controller.md.sal.binding.test.AbstractConcurrentDataBrokerTest;
+import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
 import org.opendaylight.protocol.util.CheckUtil.ListenerCheck;
-import org.opendaylight.yangtools.yang.binding.DataObject;
+import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.NetworkTopology;
+import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.TopologyId;
+import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.Topology;
+import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.TopologyBuilder;
+import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.TopologyKey;
 import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
+import org.opendaylight.yangtools.yang.binding.KeyedInstanceIdentifier;
 
-public class CheckUtilTest {
-    private final InstanceIdentifier<MockInterface> instanceIdentifier = InstanceIdentifier.create(MockInterface.class);
-    @Mock
-    private ChannelFuture future;
-    @Mock
-    private DataBroker dataBroker;
-    @Mock
-    private ReadOnlyTransaction readOnlyTransaction;
-    @Mock
-    private CheckedFuture checkedFuture;
-    @Mock
-    private Optional opt;
-    @Mock
-    private MockInterface mockInterface;
+public class CheckUtilTest extends AbstractConcurrentDataBrokerTest {
+    private static final TopologyId TOPOLOGY_ID = new TopologyId("topotest");
+    private final KeyedInstanceIdentifier<Topology, TopologyKey> topologyIIdKeyed =
+            InstanceIdentifier.create(NetworkTopology.class).child(Topology.class,
+                    new TopologyKey(TOPOLOGY_ID));
+    private static int TIMEOUT = 1;
     @Mock
     private ListenerCheck listenerCheck;
+    @Mock
+    private ChannelFuture future;
 
     @Before
     public void setUp() throws Exception {
         MockitoAnnotations.initMocks(this);
-        doReturn(this.readOnlyTransaction).when(this.dataBroker).newReadOnlyTransaction();
-        doReturn(this.checkedFuture).when(this.readOnlyTransaction).read(any(), any());
-        doReturn(this.opt).when(this.checkedFuture).checkedGet();
-        doReturn(this.mockInterface).when(this.opt).get();
-    }
-
-    @Test(expected = UnsupportedOperationException.class)
-    public void testPrivateConstructor() throws Throwable {
-        final Constructor<CheckUtil> c = CheckUtil.class.getDeclaredConstructor();
-        c.setAccessible(true);
-        try {
-            c.newInstance();
-        } catch (final InvocationTargetException e) {
-            throw e.getCause();
-        }
     }
 
     @Test(expected = VerifyException.class)
@@ -90,46 +78,101 @@ public class CheckUtilTest {
     }
 
     @Test(expected = NullPointerException.class)
-    public void testReadDataNull() throws Exception {
-        doReturn(false).when(this.opt).isPresent();
-        final InstanceIdentifier instanceIdentifier = null;
-        readDataOperational(this.dataBroker, instanceIdentifier, test -> false);
+    public void testReadDataOperationalNull() throws Exception {
+        readDataOperational(getDataBroker(), topologyIIdKeyed, test -> false, TIMEOUT);
+    }
+
+    @Test(expected = NullPointerException.class)
+    public void testReadDataConfigurationNull() throws Exception {
+        readDataConfiguration(getDataBroker(), topologyIIdKeyed, test -> false, TIMEOUT);
     }
 
     @Test(expected = AssertionError.class)
-    public void testReadDataNotEquall() throws Exception {
-        doReturn(true).when(this.opt).isPresent();
-        doReturn(false).when(this.mockInterface).getResult();
-        readDataOperational(this.dataBroker, this.instanceIdentifier, test -> {
-            assertTrue(test.getResult());
-            return test;
-        });
+    public void testReadDataOperationalFail() throws Exception {
+        storeTopo(LogicalDatastoreType.OPERATIONAL);
+        readDataOperational(getDataBroker(), this.topologyIIdKeyed, result -> {
+            assertNotNull(result.getNode());
+            return result;
+        }, TIMEOUT);
+    }
+
+    @Test(expected = AssertionError.class)
+    public void testReadDataConfigurationFail() throws Exception {
+        storeTopo(LogicalDatastoreType.CONFIGURATION);
+        readDataConfiguration(getDataBroker(), this.topologyIIdKeyed, result -> {
+            assertNotNull(result.getNode());
+            return result;
+        }, TIMEOUT);
+    }
+
+    @Test
+    public void testReadDataOperational() throws Exception {
+        storeTopo(LogicalDatastoreType.OPERATIONAL);
+        readDataOperational(getDataBroker(), this.topologyIIdKeyed, result -> {
+            assertNull(result.getNode());
+            return result;
+        }, TIMEOUT);
+    }
+
+    @Test
+    public void testReadDataConfiguration() throws Exception {
+        storeTopo(LogicalDatastoreType.CONFIGURATION);
+        readDataConfiguration(getDataBroker(), this.topologyIIdKeyed, result -> {
+            assertNull(result.getNode());
+            return result;
+        }, TIMEOUT);
+    }
+
+    private void storeTopo(final LogicalDatastoreType dsType) throws ExecutionException, InterruptedException {
+        final WriteTransaction wt = getDataBroker().newWriteOnlyTransaction();
+        wt.put(dsType, this.topologyIIdKeyed,
+                new TopologyBuilder()
+                        .setTopologyId(TOPOLOGY_ID)
+                        .build(), true);
+        wt.submit().get();
+    }
+
+    @Test
+    public void testCheckPresentConfiguration() throws Exception {
+        storeTopo(LogicalDatastoreType.CONFIGURATION);
+        checkPresentConfiguration(getDataBroker(), this.topologyIIdKeyed);
+    }
+    @Test
+    public void testCheckPresentOperational() throws Exception {
+        storeTopo(LogicalDatastoreType.OPERATIONAL);
+        checkPresentOperational(getDataBroker(), this.topologyIIdKeyed);
     }
 
     @Test(expected = AssertionError.class)
-    public void testCheckNotPresent() throws Exception {
-        doReturn(true).when(this.opt).isPresent();
-        checkNotPresentOperational(this.dataBroker, this.instanceIdentifier);
+    public void testCheckNotPresentOperationalFail() throws Exception {
+        storeTopo(LogicalDatastoreType.OPERATIONAL);
+        checkNotPresentOperational(getDataBroker(), this.topologyIIdKeyed);
+    }
+
+    @Test
+    public void testCheckNotPresentOperational() throws Exception {
+        checkNotPresentOperational(getDataBroker(), this.topologyIIdKeyed);
+    }
+
+    @Test
+    public void testCheckNotPresentConfiguration() throws Exception {
+        checkNotPresentConfiguration(getDataBroker(), this.topologyIIdKeyed);
     }
 
     @Test(expected = AssertionError.class)
     public void testCheckEquals() throws Exception {
-        checkEquals(()-> assertTrue(false));
+        checkEquals(() -> assertTrue(false), TIMEOUT);
     }
 
     @Test(expected = AssertionError.class)
     public void testCheckReceivedMessagesNotEqual() throws Exception {
         doReturn(0).when(this.listenerCheck).getListMessageSize();
-        checkReceivedMessages(this.listenerCheck, 1);
+        checkReceivedMessages(this.listenerCheck, 1, TIMEOUT);
     }
 
     @Test
     public void testCheckReceivedMessagesEqual() throws Exception {
         doReturn(1).when(this.listenerCheck).getListMessageSize();
-        checkReceivedMessages(this.listenerCheck, 1);
-    }
-
-    private interface MockInterface extends DataObject {
-        boolean getResult();
+        checkReceivedMessages(this.listenerCheck, 1, TIMEOUT);
     }
 }
\ No newline at end of file