Add junit coverage to drools-pdp

Added coverage to distributed locking.
Added coverage to test-transaction - still more to do.
Added coverage to healthcheck - still more to do.
Also fixed "code smell" in pooling.
Also fixed "code smell" in session persistence.
Fixed typo in comment.
Removed unneeded setUp() method from test.
Fixed new checkstyle errors.
Fixed another new checkstyle error.

Change-Id: I0d2ab13fcbd64486af88affb02e114e624c6a3d1
Issue-ID: POLICY-1148
Signed-off-by: Jim Hahn <jrh3@att.com>

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));
+    }
+}