Merge "Add junit coverage to drools-pdp"

15 files changed, 609 insertions, 52 deletions

diff --git a/feature-distributed-locking/pom.xml b/feature-distributed-locking/pom.xml
index a7eeed6d..0c61e34c 100644
--- a/feature-distributed-locking/pom.xml
+++ b/feature-distributed-locking/pom.xml
@@ -119,6 +119,11 @@
             <scope>test</scope>
         </dependency>
         <dependency>
+            <groupId>org.powermock</groupId>
+            <artifactId>powermock-api-mockito</artifactId>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
             <groupId>com.h2database</groupId>
             <artifactId>h2</artifactId>
             <scope>test</scope>
diff --git a/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/DistributedLockingFeature.java b/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/DistributedLockingFeature.java
index 86b5a66c..ddfe6a5a 100644
--- a/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/DistributedLockingFeature.java
+++ b/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/DistributedLockingFeature.java
@@ -134,7 +134,7 @@ public class DistributedLockingFeature implements PolicyEngineFeatureAPI, Policy
      * @return a new, pooled data source
      * @throws Exception exception
      */
-    private BasicDataSource makeDataSource() throws Exception {
+    protected BasicDataSource makeDataSource() throws Exception {
         Properties props = new Properties();
         props.put("driverClassName", lockProps.getDbDriver());
         props.put("url", lockProps.getDbUrl());
diff --git a/feature-distributed-locking/src/test/java/org/onap/policy/distributed/locking/DistributedLockingFeatureTest.java b/feature-distributed-locking/src/test/java/org/onap/policy/distributed/locking/DistributedLockingFeatureTest.java
new file mode 100644
index 00000000..90e8bfc0
--- /dev/null
+++ b/feature-distributed-locking/src/test/java/org/onap/policy/distributed/locking/DistributedLockingFeatureTest.java
@@ -0,0 +1,107 @@
+/*
+ * ============LICENSE_START=======================================================
+ * ONAP
+ * ================================================================================
+ * Copyright (C) 2018 AT&T Intellectual Property. All rights reserved.
+ * ================================================================================
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ * ============LICENSE_END=========================================================
+ */
+
+package org.onap.policy.distributed.locking;
+
+import static org.junit.Assert.assertEquals;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.sql.SQLException;
+import org.apache.commons.dbcp2.BasicDataSource;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.onap.policy.common.utils.properties.exception.PropertyException;
+import org.onap.policy.drools.persistence.SystemPersistence;
+
+/**
+ * Partially tests DistributedLockingFeature; most of the methods are tested via
+ * {@link TargetLockTest}.
+ */
+public class DistributedLockingFeatureTest {
+    private static final String EXPECTED = "expected exception";
+
+    private BasicDataSource dataSrc;
+
+    @BeforeClass
+    public static void setUpBeforeClass() throws Exception {
+        SystemPersistence.manager.setConfigurationDir("src/test/resources");
+    }
+
+    @Before
+    public void setUp() throws Exception {
+        dataSrc = mock(BasicDataSource.class);
+    }
+
+    @Test
+    public void testGetSequenceNumber() {
+        assertEquals(1000, new DistributedLockingFeature().getSequenceNumber());
+    }
+
+    @Test(expected = DistributedLockingFeatureException.class)
+    public void testAfterStart_PropEx() {
+        new DistributedLockingFeatureImpl(new PropertyException("prop", "val")).afterStart(null);
+    }
+
+    @Test(expected = DistributedLockingFeatureException.class)
+    public void testAfterStart_InterruptEx() {
+        new DistributedLockingFeatureImpl(new InterruptedException(EXPECTED)).afterStart(null);
+    }
+
+    @Test(expected = DistributedLockingFeatureException.class)
+    public void testAfterStart_OtherEx() {
+        new DistributedLockingFeatureImpl(new RuntimeException(EXPECTED)).afterStart(null);
+    }
+
+    @Test
+    public void testCleanLockTable() throws Exception {
+        when(dataSrc.getConnection()).thenThrow(new SQLException(EXPECTED));
+
+        new DistributedLockingFeatureImpl().afterStart(null);
+    }
+
+    /**
+     * Feature that overrides {@link #makeDataSource()}.
+     */
+    private class DistributedLockingFeatureImpl extends DistributedLockingFeature {
+        /**
+         * Exception to throw when {@link #makeDataSource()} is invoked.
+         */
+        private final Exception makeEx;
+
+        public DistributedLockingFeatureImpl() {
+            makeEx = null;
+        }
+
+        public DistributedLockingFeatureImpl(Exception ex) {
+            this.makeEx = ex;
+        }
+
+        @Override
+        protected BasicDataSource makeDataSource() throws Exception {
+            if (makeEx != null) {
+                throw makeEx;
+            }
+
+            return dataSrc;
+        }
+    }
+}
diff --git a/feature-distributed-locking/src/test/java/org/onap/policy/distributed/locking/TargetLockTest.java b/feature-distributed-locking/src/test/java/org/onap/policy/distributed/locking/TargetLockTest.java
index 49f1026d..6fa1febb 100644
--- a/feature-distributed-locking/src/test/java/org/onap/policy/distributed/locking/TargetLockTest.java
+++ b/feature-distributed-locking/src/test/java/org/onap/policy/distributed/locking/TargetLockTest.java
@@ -21,12 +21,19 @@
 package org.onap.policy.distributed.locking;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Matchers.anyString;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
 import java.sql.Connection;
 import java.sql.DriverManager;
 import java.sql.PreparedStatement;
 import java.sql.SQLException;
+import java.util.UUID;
 import java.util.concurrent.ExecutionException;
+import org.apache.commons.dbcp2.BasicDataSource;
 import org.junit.AfterClass;
 import org.junit.Before;
 import org.junit.BeforeClass;
@@ -43,6 +50,10 @@ public class TargetLockTest {
             "jdbc:h2:mem:pooling;INIT=CREATE SCHEMA IF NOT EXISTS pooling\\;SET SCHEMA pooling";
     private static final String DB_USER = "user";
     private static final String DB_PASSWORD = "password";
+    private static final String EXPECTED = "expected exception";
+    private static final String MY_RESOURCE = "my-resource-id";
+    private static final String MY_OWNER = "my-owner";
+    private static final UUID MY_UUID = UUID.randomUUID();
     private static Connection conn = null;
     private static DistributedLockingFeature distLockFeat;
 
@@ -139,7 +150,64 @@ public class TargetLockTest {
     }
 
     @Test
-    public void testUpdateLock() throws InterruptedException, ExecutionException {
+    public void testSecondGrab_UpdateOk() throws Exception {
+        PreparedStatement grabLockInsert = mock(PreparedStatement.class);
+        when(grabLockInsert.executeUpdate()).thenThrow(new SQLException(EXPECTED));
+
+        PreparedStatement secondGrabUpdate = mock(PreparedStatement.class);
+        when(secondGrabUpdate.executeUpdate()).thenReturn(1);
+
+        Connection connMock = mock(Connection.class);
+        when(connMock.prepareStatement(anyString())).thenReturn(grabLockInsert, secondGrabUpdate);
+
+        BasicDataSource dataSrc = mock(BasicDataSource.class);
+        when(dataSrc.getConnection()).thenReturn(connMock);
+
+        assertTrue(new TargetLock(MY_RESOURCE, MY_UUID, MY_OWNER, dataSrc).lock(MAX_AGE_SEC));
+    }
+
+    @Test
+    public void testSecondGrab_UpdateFail_InsertOk() throws Exception {
+        PreparedStatement grabLockInsert = mock(PreparedStatement.class);
+        when(grabLockInsert.executeUpdate()).thenThrow(new SQLException(EXPECTED));
+
+        PreparedStatement secondGrabUpdate = mock(PreparedStatement.class);
+        when(secondGrabUpdate.executeUpdate()).thenReturn(0);
+
+        PreparedStatement secondGrabInsert = mock(PreparedStatement.class);
+        when(secondGrabInsert.executeUpdate()).thenReturn(1);
+
+        Connection connMock = mock(Connection.class);
+        when(connMock.prepareStatement(anyString())).thenReturn(grabLockInsert, secondGrabUpdate, secondGrabInsert);
+
+        BasicDataSource dataSrc = mock(BasicDataSource.class);
+        when(dataSrc.getConnection()).thenReturn(connMock);
+
+        assertTrue(new TargetLock(MY_RESOURCE, MY_UUID, MY_OWNER, dataSrc).lock(MAX_AGE_SEC));
+    }
+
+    @Test
+    public void testSecondGrab_UpdateFail_InsertFail() throws Exception {
+        PreparedStatement grabLockInsert = mock(PreparedStatement.class);
+        when(grabLockInsert.executeUpdate()).thenThrow(new SQLException(EXPECTED));
+
+        PreparedStatement secondGrabUpdate = mock(PreparedStatement.class);
+        when(secondGrabUpdate.executeUpdate()).thenReturn(0);
+
+        PreparedStatement secondGrabInsert = mock(PreparedStatement.class);
+        when(secondGrabInsert.executeUpdate()).thenReturn(0);
+
+        Connection connMock = mock(Connection.class);
+        when(connMock.prepareStatement(anyString())).thenReturn(grabLockInsert, secondGrabUpdate, secondGrabInsert);
+
+        BasicDataSource dataSrc = mock(BasicDataSource.class);
+        when(dataSrc.getConnection()).thenReturn(connMock);
+
+        assertFalse(new TargetLock(MY_RESOURCE, MY_UUID, MY_OWNER, dataSrc).lock(MAX_AGE_SEC));
+    }
+
+    @Test
+    public void testUpdateLock() throws Exception {
         // not locked yet - refresh should fail
         assertEquals(
                 OperResult.OPER_DENIED, distLockFeat.beforeRefresh("resource1", "owner1", MAX_AGE_SEC));
@@ -160,10 +228,6 @@ public class TargetLockTest {
                   "UPDATE pooling.locks SET expirationTime = timestampadd(second, -1, now()) WHERE resourceId = ?"); ) {
             updateStatement.setString(1, "resource1");
             updateStatement.executeUpdate();
-
-        } catch (SQLException e) {
-            logger.error("Error in TargetLockTest.testGrabLockSuccess()", e);
-            throw new RuntimeException(e);
         }
 
         // refresh should fail now
@@ -171,19 +235,29 @@ public class TargetLockTest {
                 OperResult.OPER_DENIED, distLockFeat.beforeRefresh("resource1", "owner1", MAX_AGE_SEC));
 
         assertEquals(OperResult.OPER_DENIED, distLockFeat.beforeIsLockedBy("resource1", "owner1"));
+
+        // test exception case
+        BasicDataSource dataSrc = mock(BasicDataSource.class);
+        when(dataSrc.getConnection()).thenThrow(new SQLException(EXPECTED));
+        assertFalse(new TargetLock(MY_RESOURCE, MY_UUID, MY_OWNER, dataSrc).refresh(MAX_AGE_SEC));
     }
 
     @Test
-    public void testUnlock() throws InterruptedException, ExecutionException {
+    public void testUnlock() throws Exception {
         distLockFeat.beforeLock("resource1", "owner1", MAX_AGE_SEC);
 
         assertEquals(OperResult.OPER_ACCEPTED, distLockFeat.beforeUnlock("resource1", "owner1"));
         assertEquals(
                 OperResult.OPER_ACCEPTED, distLockFeat.beforeLock("resource1", "owner2", MAX_AGE_SEC));
+
+        // test exception case
+        BasicDataSource dataSrc = mock(BasicDataSource.class);
+        when(dataSrc.getConnection()).thenThrow(new SQLException(EXPECTED));
+        assertFalse(new TargetLock(MY_RESOURCE, MY_UUID, MY_OWNER, dataSrc).unlock());
     }
 
     @Test
-    public void testIsActive() {
+    public void testIsActive() throws Exception {
         assertEquals(OperResult.OPER_DENIED, distLockFeat.beforeIsLockedBy("resource1", "owner1"));
         distLockFeat.beforeLock("resource1", "owner1", MAX_AGE_SEC);
         assertEquals(OperResult.OPER_ACCEPTED, distLockFeat.beforeIsLockedBy("resource1", "owner1"));
@@ -195,10 +269,6 @@ public class TargetLockTest {
                   "UPDATE pooling.locks SET expirationTime = timestampadd(second, -5, now()) WHERE resourceId = ?"); ) {
             updateStatement.setString(1, "resource1");
             updateStatement.executeUpdate();
-
-        } catch (SQLException e) {
-            logger.error("Error in TargetLockTest.testIsActive()", e);
-            throw new RuntimeException(e);
         }
 
         assertEquals(OperResult.OPER_DENIED, distLockFeat.beforeIsLockedBy("resource1", "owner1"));
@@ -207,6 +277,20 @@ public class TargetLockTest {
         // Unlock record, next isActive attempt should fail
         distLockFeat.beforeUnlock("resource1", "owner1");
         assertEquals(OperResult.OPER_DENIED, distLockFeat.beforeIsLockedBy("resource1", "owner1"));
+
+        // test exception case for outer "try"
+        BasicDataSource dataSrc = mock(BasicDataSource.class);
+        when(dataSrc.getConnection()).thenThrow(new SQLException(EXPECTED));
+        assertFalse(new TargetLock(MY_RESOURCE, MY_UUID, MY_OWNER, dataSrc).isActive());
+
+        // test exception case for inner "try"
+        PreparedStatement stmt = mock(PreparedStatement.class);
+        when(stmt.executeQuery()).thenThrow(new SQLException(EXPECTED));
+        Connection connMock = mock(Connection.class);
+        when(connMock.prepareStatement(anyString())).thenReturn(stmt);
+        dataSrc = mock(BasicDataSource.class);
+        when(dataSrc.getConnection()).thenReturn(connMock);
+        assertFalse(new TargetLock(MY_RESOURCE, MY_UUID, MY_OWNER, dataSrc).isActive());
     }
 
     @Test
@@ -218,10 +302,24 @@ public class TargetLockTest {
     }
 
     @Test
-    public void testIsLocked() {
+    public void testIsLocked() throws Exception {
         assertEquals(OperResult.OPER_DENIED, distLockFeat.beforeIsLocked("resource1"));
         distLockFeat.beforeLock("resource1", "owner1", MAX_AGE_SEC);
         assertEquals(OperResult.OPER_ACCEPTED, distLockFeat.beforeIsLocked("resource1"));
+
+        // test exception case for outer "try"
+        BasicDataSource dataSrc = mock(BasicDataSource.class);
+        when(dataSrc.getConnection()).thenThrow(new SQLException(EXPECTED));
+        assertFalse(new TargetLock(MY_RESOURCE, MY_UUID, MY_OWNER, dataSrc).isLocked());
+
+        // test exception case for inner "try"
+        PreparedStatement stmt = mock(PreparedStatement.class);
+        when(stmt.executeQuery()).thenThrow(new SQLException(EXPECTED));
+        Connection connMock = mock(Connection.class);
+        when(connMock.prepareStatement(anyString())).thenReturn(stmt);
+        dataSrc = mock(BasicDataSource.class);
+        when(dataSrc.getConnection()).thenReturn(connMock);
+        assertFalse(new TargetLock(MY_RESOURCE, MY_UUID, MY_OWNER, dataSrc).isLocked());
     }
 
     private static void getDbConnection() {
diff --git a/feature-healthcheck/pom.xml b/feature-healthcheck/pom.xml
index 3ed46abc..17a5053a 100644
--- a/feature-healthcheck/pom.xml
+++ b/feature-healthcheck/pom.xml
@@ -154,6 +154,11 @@
 		<artifactId>junit</artifactId>
 		<scope>test</scope>
 	</dependency>
+    <dependency>
+        <groupId>org.powermock</groupId>
+        <artifactId>powermock-api-mockito</artifactId>
+        <scope>test</scope>
+    </dependency>
   </dependencies>
 
 </project>
diff --git a/feature-healthcheck/src/main/java/org/onap/policy/drools/healthcheck/HealthCheckFeature.java b/feature-healthcheck/src/main/java/org/onap/policy/drools/healthcheck/HealthCheckFeature.java
index 3a79a55c..8e7a66ef 100644
--- a/feature-healthcheck/src/main/java/org/onap/policy/drools/healthcheck/HealthCheckFeature.java
+++ b/feature-healthcheck/src/main/java/org/onap/policy/drools/healthcheck/HealthCheckFeature.java
@@ -48,7 +48,7 @@ public class HealthCheckFeature implements PolicyEngineFeatureAPI {
     @Override
     public boolean afterStart(PolicyEngine engine) {
         try {
-            HealthCheck.monitor.start();
+            getMonitor().start();
         } catch (IllegalStateException e) {
             logger.error("Healthcheck Monitor cannot be started", e);
         }
@@ -59,7 +59,7 @@ public class HealthCheckFeature implements PolicyEngineFeatureAPI {
     @Override
     public boolean afterShutdown(PolicyEngine engine) {
         try {
-            HealthCheck.monitor.stop();
+            getMonitor().stop();
         } catch (IllegalStateException e) {
             logger.error("Healthcheck Monitor cannot be stopped", e);
         }
diff --git a/feature-healthcheck/src/test/java/org/onap/policy/drools/healthcheck/HealthCheckFeatureTest.java b/feature-healthcheck/src/test/java/org/onap/policy/drools/healthcheck/HealthCheckFeatureTest.java
index e0312f1f..42d77f26 100644
--- a/feature-healthcheck/src/test/java/org/onap/policy/drools/healthcheck/HealthCheckFeatureTest.java
+++ b/feature-healthcheck/src/test/java/org/onap/policy/drools/healthcheck/HealthCheckFeatureTest.java
@@ -7,9 +7,9 @@
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at
- * 
+ *
  *      http://www.apache.org/licenses/LICENSE-2.0
- * 
+ *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
  * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@@ -21,7 +21,12 @@
 package org.onap.policy.drools.healthcheck;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
 
 import java.io.File;
 import java.io.FileWriter;
@@ -30,7 +35,6 @@ import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.Properties;
-
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
 import org.junit.Test;
@@ -50,11 +54,13 @@ public class HealthCheckFeatureTest {
      */
     private static final String HEALTH_CHECK_PROPERTIES_FILE = "feature-healthcheck.properties";
 
-    private static final Path healthCheckPropsPath =
-            Paths.get(SystemPersistence.manager.getConfigurationPath().toString(), HEALTH_CHECK_PROPERTIES_FILE);
+    private static final Path healthCheckPropsPath = Paths
+                    .get(SystemPersistence.manager.getConfigurationPath().toString(), HEALTH_CHECK_PROPERTIES_FILE);
+
+    private static final Path healthCheckPropsBackupPath = Paths.get(
+                    SystemPersistence.manager.getConfigurationPath().toString(), HEALTH_CHECK_PROPERTIES_FILE + ".bak");
 
-    private static final Path healthCheckPropsBackupPath = Paths
-            .get(SystemPersistence.manager.getConfigurationPath().toString(), HEALTH_CHECK_PROPERTIES_FILE + ".bak");
+    private static final String EXPECTED = "expected exception";
 
 
     /**
@@ -65,47 +71,47 @@ public class HealthCheckFeatureTest {
     private static Properties httpProperties = new Properties();
 
     /**
-     * Set up. 
+     * Set up.
      */
     @BeforeClass
     public static void setup() {
 
         httpProperties.setProperty(PolicyEndPointProperties.PROPERTY_HTTP_SERVER_SERVICES, "HEALTHCHECK");
         httpProperties.setProperty(PolicyEndPointProperties.PROPERTY_HTTP_SERVER_SERVICES + "." + "HEALTHCHECK"
-                + PolicyEndPointProperties.PROPERTY_HTTP_HOST_SUFFIX, "localhost");
+                        + PolicyEndPointProperties.PROPERTY_HTTP_HOST_SUFFIX, "localhost");
         httpProperties.setProperty(PolicyEndPointProperties.PROPERTY_HTTP_SERVER_SERVICES + "." + "HEALTHCHECK"
-                + PolicyEndPointProperties.PROPERTY_HTTP_PORT_SUFFIX, "7777");
+                        + PolicyEndPointProperties.PROPERTY_HTTP_PORT_SUFFIX, "7777");
         httpProperties.setProperty(PolicyEndPointProperties.PROPERTY_HTTP_SERVER_SERVICES + "." + "HEALTHCHECK"
-                + PolicyEndPointProperties.PROPERTY_HTTP_AUTH_USERNAME_SUFFIX, "username");
+                        + PolicyEndPointProperties.PROPERTY_HTTP_AUTH_USERNAME_SUFFIX, "username");
         httpProperties.setProperty(PolicyEndPointProperties.PROPERTY_HTTP_SERVER_SERVICES + "." + "HEALTHCHECK"
-                + PolicyEndPointProperties.PROPERTY_HTTP_AUTH_PASSWORD_SUFFIX, "password");
+                        + PolicyEndPointProperties.PROPERTY_HTTP_AUTH_PASSWORD_SUFFIX, "password");
         httpProperties.setProperty(
-                PolicyEndPointProperties.PROPERTY_HTTP_SERVER_SERVICES + "." + "HEALTHCHECK"
-                        + PolicyEndPointProperties.PROPERTY_HTTP_REST_CLASSES_SUFFIX,
-                org.onap.policy.drools.healthcheck.RestMockHealthCheck.class.getName());
+                        PolicyEndPointProperties.PROPERTY_HTTP_SERVER_SERVICES + "." + "HEALTHCHECK"
+                                        + PolicyEndPointProperties.PROPERTY_HTTP_REST_CLASSES_SUFFIX,
+                        org.onap.policy.drools.healthcheck.RestMockHealthCheck.class.getName());
         httpProperties.setProperty(
-                PolicyEndPointProperties.PROPERTY_HTTP_SERVER_SERVICES + "." + "HEALTHCHECK"
-                    + PolicyEndPointProperties.PROPERTY_HTTP_FILTER_CLASSES_SUFFIX,
-                org.onap.policy.drools.healthcheck.TestAafHealthCheckFilter.class.getName());
+                        PolicyEndPointProperties.PROPERTY_HTTP_SERVER_SERVICES + "." + "HEALTHCHECK"
+                                        + PolicyEndPointProperties.PROPERTY_HTTP_FILTER_CLASSES_SUFFIX,
+                        org.onap.policy.drools.healthcheck.TestAafHealthCheckFilter.class.getName());
         httpProperties.setProperty(PolicyEndPointProperties.PROPERTY_HTTP_SERVER_SERVICES + "." + "HEALTHCHECK"
-                + PolicyEndPointProperties.PROPERTY_MANAGED_SUFFIX, "true");
+                        + PolicyEndPointProperties.PROPERTY_MANAGED_SUFFIX, "true");
 
 
         httpProperties.setProperty(PolicyEndPointProperties.PROPERTY_HTTP_CLIENT_SERVICES, "HEALTHCHECK");
         httpProperties.setProperty(PolicyEndPointProperties.PROPERTY_HTTP_CLIENT_SERVICES + "." + "HEALTHCHECK"
-                + PolicyEndPointProperties.PROPERTY_HTTP_HOST_SUFFIX, "localhost");
+                        + PolicyEndPointProperties.PROPERTY_HTTP_HOST_SUFFIX, "localhost");
         httpProperties.setProperty(PolicyEndPointProperties.PROPERTY_HTTP_CLIENT_SERVICES + "." + "HEALTHCHECK"
-                + PolicyEndPointProperties.PROPERTY_HTTP_PORT_SUFFIX, "7777");
+                        + PolicyEndPointProperties.PROPERTY_HTTP_PORT_SUFFIX, "7777");
         httpProperties.setProperty(PolicyEndPointProperties.PROPERTY_HTTP_CLIENT_SERVICES + "." + "HEALTHCHECK"
-                + PolicyEndPointProperties.PROPERTY_HTTP_URL_SUFFIX, "healthcheck/test");
+                        + PolicyEndPointProperties.PROPERTY_HTTP_URL_SUFFIX, "healthcheck/test");
         httpProperties.setProperty(PolicyEndPointProperties.PROPERTY_HTTP_CLIENT_SERVICES + "." + "HEALTHCHECK"
-                + PolicyEndPointProperties.PROPERTY_HTTP_HTTPS_SUFFIX, "false");
+                        + PolicyEndPointProperties.PROPERTY_HTTP_HTTPS_SUFFIX, "false");
         httpProperties.setProperty(PolicyEndPointProperties.PROPERTY_HTTP_CLIENT_SERVICES + "." + "HEALTHCHECK"
-                + PolicyEndPointProperties.PROPERTY_HTTP_AUTH_USERNAME_SUFFIX, "username");
+                        + PolicyEndPointProperties.PROPERTY_HTTP_AUTH_USERNAME_SUFFIX, "username");
         httpProperties.setProperty(PolicyEndPointProperties.PROPERTY_HTTP_CLIENT_SERVICES + "." + "HEALTHCHECK"
-                + PolicyEndPointProperties.PROPERTY_HTTP_AUTH_PASSWORD_SUFFIX, "password");
+                        + PolicyEndPointProperties.PROPERTY_HTTP_AUTH_PASSWORD_SUFFIX, "password");
         httpProperties.setProperty(PolicyEndPointProperties.PROPERTY_HTTP_CLIENT_SERVICES + "." + "HEALTHCHECK"
-                + PolicyEndPointProperties.PROPERTY_MANAGED_SUFFIX, "true");
+                        + PolicyEndPointProperties.PROPERTY_MANAGED_SUFFIX, "true");
 
         configDirSetup();
 
@@ -148,6 +154,41 @@ public class HealthCheckFeatureTest {
 
     }
 
+    @Test
+    public void testGetSequenceNumber() {
+        assertEquals(1000, new HealthCheckFeature().getSequenceNumber());
+    }
+
+    @Test
+    public void testAfterStart() {
+        HealthCheck checker = mock(HealthCheck.class);
+        HealthCheckFeature feature = new HealthCheckFeatureImpl(checker);
+
+        // without exception
+        assertFalse(feature.afterStart(null));
+        verify(checker).start();
+        verify(checker, never()).stop();
+
+        // with exception
+        doThrow(new IllegalStateException(EXPECTED)).when(checker).start();
+        assertFalse(feature.afterStart(null));
+    }
+
+    @Test
+    public void testAfterShutdown() {
+        HealthCheck checker = mock(HealthCheck.class);
+        HealthCheckFeature feature = new HealthCheckFeatureImpl(checker);
+
+        // without exception
+        assertFalse(feature.afterShutdown(null));
+        verify(checker).stop();
+        verify(checker, never()).start();
+
+        // with exception
+        doThrow(new IllegalStateException(EXPECTED)).when(checker).stop();
+        assertFalse(feature.afterShutdown(null));
+    }
+
 
     /**
      * setup up config directory.
@@ -191,4 +232,20 @@ public class HealthCheckFeatureTest {
         }
     }
 
+    /**
+     * Feature that returns a particular monitor.
+     */
+    private static class HealthCheckFeatureImpl extends HealthCheckFeature {
+        private final HealthCheck checker;
+
+        public HealthCheckFeatureImpl(HealthCheck checker) {
+            this.checker = checker;
+        }
+
+        @Override
+        public HealthCheck getMonitor() {
+            return checker;
+        }
+
+    }
 }
diff --git a/feature-healthcheck/src/test/java/org/onap/policy/drools/healthcheck/HealthCheckTest.java b/feature-healthcheck/src/test/java/org/onap/policy/drools/healthcheck/HealthCheckTest.java
new file mode 100644
index 00000000..850b1edf
--- /dev/null
+++ b/feature-healthcheck/src/test/java/org/onap/policy/drools/healthcheck/HealthCheckTest.java
@@ -0,0 +1,89 @@
+/*
+ * ============LICENSE_START=======================================================
+ * ONAP
+ * ================================================================================
+ * Copyright (C) 2018 AT&T Intellectual Property. All rights reserved.
+ * ================================================================================
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ * ============LICENSE_END=========================================================
+ */
+
+package org.onap.policy.drools.healthcheck;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+
+import java.util.Collections;
+import java.util.List;
+import org.junit.Test;
+import org.onap.policy.drools.healthcheck.HealthCheck.Report;
+import org.onap.policy.drools.healthcheck.HealthCheck.Reports;
+
+public class HealthCheckTest {
+
+    private static final int RPT_CODE = 100;
+    private static final String RPT_MSG = "report-message";
+    private static final String RPT_NAME = "report-name";
+    private static final String RPT_URL = "report-url";
+
+    @Test
+    public void testHealthCheck_Report() {
+        Report rpt = new Report();
+
+        // toString should work with un-populated data
+        assertNotNull(rpt.toString());
+
+        rpt.setCode(RPT_CODE);
+        rpt.setHealthy(true);
+        rpt.setMessage(RPT_MSG);
+        rpt.setName(RPT_NAME);
+        rpt.setUrl(RPT_URL);
+
+        assertEquals(RPT_CODE, rpt.getCode());
+        assertEquals(true, rpt.isHealthy());
+        assertEquals(RPT_MSG, rpt.getMessage());
+        assertEquals(RPT_NAME, rpt.getName());
+        assertEquals(RPT_URL, rpt.getUrl());
+
+        // flip the flag
+        rpt.setHealthy(false);
+        assertEquals(false, rpt.isHealthy());
+
+        // toString should work with populated data
+        assertNotNull(rpt.toString());
+    }
+
+    @Test
+    public void testHealthCheck_Reports() {
+        Reports reports = new Reports();
+
+        // toString should work with un-populated data
+        assertNotNull(reports.toString());
+
+        List<Report> lst = Collections.emptyList();
+        reports.setDetails(lst);
+        reports.setHealthy(true);
+
+        assertTrue(lst == reports.getDetails());
+        assertEquals(true, reports.isHealthy());
+
+        // flip the flag
+        reports.setHealthy(false);
+        assertEquals(false, reports.isHealthy());
+
+        // toString should work with populated data
+        assertNotNull(reports.toString());
+    }
+
+}
diff --git a/feature-pooling-dmaap/src/main/java/org/onap/policy/drools/pooling/PoolingManagerImpl.java b/feature-pooling-dmaap/src/main/java/org/onap/policy/drools/pooling/PoolingManagerImpl.java
index 17d520ad..6b5828c7 100644
--- a/feature-pooling-dmaap/src/main/java/org/onap/policy/drools/pooling/PoolingManagerImpl.java
+++ b/feature-pooling-dmaap/src/main/java/org/onap/policy/drools/pooling/PoolingManagerImpl.java
@@ -205,10 +205,12 @@ public class PoolingManagerImpl implements PoolingManager, TopicListener {
         }
     }
 
+    @Override
     public String getHost() {
         return host;
     }
 
+    @Override
     public String getTopic() {
         return topic;
     }
@@ -232,10 +234,8 @@ public class PoolingManagerImpl implements PoolingManager, TopicListener {
     /**
      * Indicates that the controller is about to start. Starts the publisher for the
      * internal topic, and creates a thread pool for the timers.
-     * 
-     * @throws PoolingFeatureException if the internal topic publisher cannot be started
      */
-    public void beforeStart() throws PoolingFeatureException {
+    public void beforeStart() {
         synchronized (curLocker) {
             if (scheduler == null) {
                 dmaapMgr.startPublisher();
diff --git a/feature-pooling-dmaap/src/test/java/org/onap/policy/drools/pooling/PoolingFeatureTest.java b/feature-pooling-dmaap/src/test/java/org/onap/policy/drools/pooling/PoolingFeatureTest.java
index 07fc757b..a943575c 100644
--- a/feature-pooling-dmaap/src/test/java/org/onap/policy/drools/pooling/PoolingFeatureTest.java
+++ b/feature-pooling-dmaap/src/test/java/org/onap/policy/drools/pooling/PoolingFeatureTest.java
@@ -478,11 +478,11 @@ public class PoolingFeatureTest {
         assertFalse(pool.beforeStart(controllerDisabled));
     }
 
-    @Test(expected = PoolingFeatureRtException.class)
+    @Test(expected = RuntimeException.class)
     public void testDoManager_Ex() throws Exception {
 
         // generate exception
-        doThrow(new PoolingFeatureException()).when(mgr1).beforeStart();
+        doThrow(new RuntimeException()).when(mgr1).beforeStart();
 
         pool.beforeStart(controller1);
     }
diff --git a/feature-session-persistence/src/main/java/org/onap/policy/drools/persistence/PersistenceFeature.java b/feature-session-persistence/src/main/java/org/onap/policy/drools/persistence/PersistenceFeature.java
index 332e2984..13264d60 100644
--- a/feature-session-persistence/src/main/java/org/onap/policy/drools/persistence/PersistenceFeature.java
+++ b/feature-session-persistence/src/main/java/org/onap/policy/drools/persistence/PersistenceFeature.java
@@ -543,8 +543,7 @@ public class PersistenceFeature implements PolicySessionFeatureAPI, PolicyEngine
                 logger.error("Clean up of sessioninfo table failed", e);
             }
 
-            // TODO: delete DroolsSessionEntity where sessionId not in
-            // (sessinfo.xxx)
+            // delete DroolsSessionEntity where sessionId not in (sessinfo.xxx)?
 
             sessInfoCleaned = true;
         }
diff --git a/feature-session-persistence/src/test/java/org/onap/policy/drools/persistence/DroolsSessionEntityTest.java b/feature-session-persistence/src/test/java/org/onap/policy/drools/persistence/DroolsSessionEntityTest.java
index b2dcfb52..5fbb905b 100644
--- a/feature-session-persistence/src/test/java/org/onap/policy/drools/persistence/DroolsSessionEntityTest.java
+++ b/feature-session-persistence/src/test/java/org/onap/policy/drools/persistence/DroolsSessionEntityTest.java
@@ -103,6 +103,9 @@ public class DroolsSessionEntityTest {
     @Test
     public void testEqualsObject() {
         DroolsSessionEntity entity = makeEnt("mynameA", 1);
+        
+        // diff object type
+        assertFalse(entity.equals("hello"));
 
         // reflexive
         assertTrue(entity.equals(entity));
diff --git a/feature-test-transaction/pom.xml b/feature-test-transaction/pom.xml
index f16a50f2..2848e733 100644
--- a/feature-test-transaction/pom.xml
+++ b/feature-test-transaction/pom.xml
@@ -134,5 +134,10 @@
             <artifactId>junit</artifactId>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>org.powermock</groupId>
+            <artifactId>powermock-api-mockito</artifactId>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
 </project>
diff --git a/feature-test-transaction/src/main/java/org/onap/policy/drools/testtransaction/TestTransactionFeature.java b/feature-test-transaction/src/main/java/org/onap/policy/drools/testtransaction/TestTransactionFeature.java
index 956304b9..bb8a7598 100644
--- a/feature-test-transaction/src/main/java/org/onap/policy/drools/testtransaction/TestTransactionFeature.java
+++ b/feature-test-transaction/src/main/java/org/onap/policy/drools/testtransaction/TestTransactionFeature.java
@@ -40,7 +40,7 @@ public class TestTransactionFeature implements PolicyControllerFeatureAPI {
         logger.info("TEST_TRANSACTION FEATURE LOADED");
 
         if (controller.isAlive() && !controller.isLocked() && controller.getDrools().isBrained()) {
-            TestTransaction.manager.register(controller);
+            getTestTransMgr().register(controller);
         }
         return false;
     }
@@ -49,7 +49,7 @@ public class TestTransactionFeature implements PolicyControllerFeatureAPI {
     public boolean afterLock(PolicyController controller) {
         logger.info("controller {} locked", controller.getName());
 
-        TestTransaction.manager.unregister(controller);
+        getTestTransMgr().unregister(controller);
         return false;
     }
 
@@ -58,7 +58,7 @@ public class TestTransactionFeature implements PolicyControllerFeatureAPI {
         logger.info("controller {} unlocked", controller.getName());
 
         if (controller.isAlive() && !controller.isLocked() && controller.getDrools().isBrained()) {
-            TestTransaction.manager.register(controller);
+            getTestTransMgr().register(controller);
         }
 
         return false;
@@ -68,7 +68,7 @@ public class TestTransactionFeature implements PolicyControllerFeatureAPI {
     public boolean beforeStop(PolicyController controller) {
         logger.info("controller {} stopping", controller.getName());
 
-        TestTransaction.manager.unregister(controller);
+        getTestTransMgr().unregister(controller);
 
         return false;
     }
@@ -77,4 +77,12 @@ public class TestTransactionFeature implements PolicyControllerFeatureAPI {
     public int getSequenceNumber() {
         return 1000;
     }
+
+    /**
+     * Gets the test transaction manager.
+     * @return the test transaction manager
+     */
+    protected TestTransaction getTestTransMgr() {
+        return TestTransaction.manager;
+    }
 }
diff --git a/feature-test-transaction/src/test/java/org/onap/policy/drools/testtransaction/TestTransactionFeatureTest.java b/feature-test-transaction/src/test/java/org/onap/policy/drools/testtransaction/TestTransactionFeatureTest.java
new file mode 100644
index 00000000..6bb09903
--- /dev/null
+++ b/feature-test-transaction/src/test/java/org/onap/policy/drools/testtransaction/TestTransactionFeatureTest.java
@@ -0,0 +1,181 @@
+/*
+ * ============LICENSE_START=======================================================
+ * ONAP
+ * ================================================================================
+ * Copyright (C) 2018 AT&T Intellectual Property. All rights reserved.
+ * ================================================================================
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ * ============LICENSE_END=========================================================
+ */
+
+package org.onap.policy.drools.testtransaction;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.function.Function;
+import org.junit.Before;
+import org.junit.Test;
+import org.onap.policy.drools.controller.DroolsController;
+import org.onap.policy.drools.system.PolicyController;
+
+public class TestTransactionFeatureTest {
+
+    private AtomicInteger regCount;
+    private AtomicInteger unregCount;
+    private TestTransaction mgr;
+    private DroolsController drools;
+    private PolicyController ctlr;
+    private TestTransactionFeature feat;
+
+    /**
+     * Initialize objects for each test.
+     */
+    @Before
+    public void setUp() {
+        regCount = new AtomicInteger(0);
+        unregCount = new AtomicInteger(0);
+        mgr = mock(TestTransaction.class);
+        drools = mock(DroolsController.class);
+        ctlr = mock(PolicyController.class);
+
+        feat = new TestTransactionFeature() {
+            @Override
+            protected TestTransaction getTestTransMgr() {
+                return mgr;
+            }
+        };
+
+        when(ctlr.getDrools()).thenReturn(drools);
+
+        doAnswer(args -> {
+            regCount.incrementAndGet();
+            return null;
+        }).when(mgr).register(ctlr);
+
+        doAnswer(args -> {
+            unregCount.incrementAndGet();
+            return null;
+        }).when(mgr).unregister(ctlr);
+    }
+
+    @Test
+    public void testAfterStart() {
+        // try each combination of alive, locked, and brained
+        checkCombos(regCount, ctlr -> feat.afterStart(ctlr));
+    }
+
+    @Test
+    public void testAfterLock() {
+        checkSimple(unregCount, ctlr -> feat.afterLock(ctlr));
+    }
+
+    @Test
+    public void testAfterUnlock() {
+        // try each combination of alive, locked, and brained
+        checkCombos(regCount, ctlr -> feat.afterUnlock(ctlr));
+    }
+
+    @Test
+    public void testBeforeStop() {
+        checkSimple(unregCount, ctlr -> feat.beforeStop(ctlr));
+    }
+
+    @Test
+    public void testGetSequenceNumber() {
+        assertEquals(1000, feat.getSequenceNumber());
+    }
+
+    @Test
+    public void testGetTestTransMgr() {
+        assertNotNull(new TestTransactionFeature().getTestTransMgr());
+    }
+
+    /**
+     * Try each combination of alive, locked, and brained.
+     *
+     * @param counter counter to check after each invocation
+     * @param method method to invoke
+     */
+    private void checkCombos(AtomicInteger counter, Function<PolicyController, Boolean> method) {
+        when(ctlr.isAlive()).thenReturn(true);
+        when(ctlr.isLocked()).thenReturn(true);
+        when(drools.isBrained()).thenReturn(true);
+        assertFalse(method.apply(ctlr));
+        assertEquals(0, counter.getAndSet(0));
+
+        when(ctlr.isAlive()).thenReturn(true);
+        when(ctlr.isLocked()).thenReturn(true);
+        when(drools.isBrained()).thenReturn(false);
+        assertFalse(method.apply(ctlr));
+        assertEquals(0, counter.getAndSet(0));
+
+        // this is the only one that should cause it to register
+        when(ctlr.isAlive()).thenReturn(true);
+        when(ctlr.isLocked()).thenReturn(false);
+        when(drools.isBrained()).thenReturn(true);
+        assertFalse(method.apply(ctlr));
+        assertEquals(1, counter.getAndSet(0));
+
+        when(ctlr.isAlive()).thenReturn(true);
+        when(ctlr.isLocked()).thenReturn(false);
+        when(drools.isBrained()).thenReturn(false);
+        assertFalse(method.apply(ctlr));
+        assertEquals(0, counter.getAndSet(0));
+
+        when(ctlr.isAlive()).thenReturn(false);
+        when(ctlr.isLocked()).thenReturn(true);
+        when(drools.isBrained()).thenReturn(true);
+        assertFalse(method.apply(ctlr));
+        assertEquals(0, counter.getAndSet(0));
+
+        when(ctlr.isAlive()).thenReturn(false);
+        when(ctlr.isLocked()).thenReturn(true);
+        when(drools.isBrained()).thenReturn(false);
+        assertFalse(method.apply(ctlr));
+        assertEquals(0, counter.getAndSet(0));
+
+        when(ctlr.isAlive()).thenReturn(false);
+        when(ctlr.isLocked()).thenReturn(false);
+        when(drools.isBrained()).thenReturn(true);
+        assertFalse(method.apply(ctlr));
+        assertEquals(0, counter.getAndSet(0));
+
+        when(ctlr.isAlive()).thenReturn(false);
+        when(ctlr.isLocked()).thenReturn(false);
+        when(drools.isBrained()).thenReturn(false);
+        assertFalse(method.apply(ctlr));
+        assertEquals(0, counter.getAndSet(0));
+    }
+
+    /**
+     * Check the simple case that doesn't depend on the controller state.
+     *
+     * @param counter counter to check after each invocation
+     * @param method method to invoke
+     */
+    private void checkSimple(AtomicInteger counter, Function<PolicyController, Boolean> method) {
+        when(ctlr.isAlive()).thenReturn(true);
+        assertFalse(method.apply(ctlr));
+        assertEquals(1, counter.getAndSet(0));
+
+        when(ctlr.isAlive()).thenReturn(false);
+        assertFalse(method.apply(ctlr));
+        assertEquals(1, counter.getAndSet(0));
+    }
+}