From 962dd9714c6d35cacdec0ee8e601431aa349001a Mon Sep 17 00:00:00 2001 From: Jim Hahn Date: Thu, 22 Aug 2019 14:21:14 -0400 Subject: [PATCH] Remove powermock to increase coverage Removed powermock from the junit tests of these classes to increase reported sonar coverage: RESTfulPAPEngine BrmsGateway Typically added override methods to the classes under test, to replace the overrides originally provided by powermock. Also needed to add code to RESTfulPAPEngineTest to clear the XACML properties before the test ran. Modified to use the existing XACMLProperties.reloadProperties() method. Also modified PolicyNotificationMailTest to use a host/port that has no listener so that the test runs faster. Fixed some newly introduced sonar issues. Change-Id: I65e36b01e9506987032eb21baac808ed3dfd4f47 Issue-ID: POLICY-1937 Signed-off-by: Jim Hahn --- .../java/org/onap/policy/brms/api/BrmsGateway.java | 41 +++++++--- .../org/onap/policy/brms/api/BrmsGatewayTest.java | 89 +++++++++++++++++----- .../org/onap/policy/admin/RESTfulPAPEngine.java | 12 ++- .../policy/admin/PolicyNotificationMailTest.java | 11 ++- .../onap/policy/admin/RESTfulPAPEngineTest.java | 37 +++++---- 5 files changed, 141 insertions(+), 49 deletions(-) diff --git a/BRMSGateway/src/main/java/org/onap/policy/brms/api/BrmsGateway.java b/BRMSGateway/src/main/java/org/onap/policy/brms/api/BrmsGateway.java index e743794ec..bb62b96c4 100644 --- a/BRMSGateway/src/main/java/org/onap/policy/brms/api/BrmsGateway.java +++ b/BRMSGateway/src/main/java/org/onap/policy/brms/api/BrmsGateway.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. @@ -20,6 +20,7 @@ package org.onap.policy.brms.api; +import java.util.concurrent.CountDownLatch; import org.onap.policy.api.NotificationScheme; import org.onap.policy.api.PolicyEngine; import org.onap.policy.api.PolicyException; @@ -31,7 +32,7 @@ import org.onap.policy.xacml.api.XACMLErrorConstants; * BRMSGateway: This application acts as the Gateway interface between the PDP XACML and PDP Drools. * The listens for BRMS based policies and pushes them to the specified Policy Repository, from * where the PDP Drools reads the Rule Jar. - * + * * @version 0.1 */ public class BrmsGateway { @@ -41,6 +42,9 @@ public class BrmsGateway { private static PolicyEngine policyEngine = null; + // may be overridden by junit tests + private static Factory factory = new Factory(); + private BrmsGateway() { // Default private constructor } @@ -68,7 +72,7 @@ public class BrmsGateway { logger.info("Initializing BRMS Handler"); BrmsHandler brmsHandler = null; try { - brmsHandler = new BrmsHandler(configFile); + brmsHandler = factory.makeBrmsHandler(configFile); } catch (final PolicyException e) { String errorString = "Check your property file: " + e.getMessage(); logger.error(errorString); @@ -85,21 +89,34 @@ public class BrmsGateway { } // Keep Running.... + CountDownLatch latch = new CountDownLatch(1); final Runnable runnable = () -> { - while (true) { - try { - Thread.sleep(30000); - } catch (final InterruptedException e) { - logger.error(XACMLErrorConstants.ERROR_SYSTEM_ERROR + "Thread Exception " + e.getMessage()); - Thread.currentThread().interrupt(); - } + try { + // wait until interrupted + latch.await(); + } catch (final InterruptedException e) { + logger.error(XACMLErrorConstants.ERROR_SYSTEM_ERROR + "Thread Exception " + e.getMessage()); + Thread.currentThread().interrupt(); } }; - final Thread thread = new Thread(runnable); + final Thread thread = factory.makeThread(runnable); thread.start(); } public static PolicyEngine getPolicyEngine() { return policyEngine; } + + /** + * Factory to provide various data. May be overridden by junit tests. + */ + public static class Factory { + public BrmsHandler makeBrmsHandler(String configFile) throws PolicyException { + return new BrmsHandler(configFile); + } + + public Thread makeThread(Runnable runnable) { + return new Thread(runnable); + } + } } diff --git a/BRMSGateway/src/test/java/org/onap/policy/brms/api/BrmsGatewayTest.java b/BRMSGateway/src/test/java/org/onap/policy/brms/api/BrmsGatewayTest.java index fd8a7ed3b..b34b18b6d 100644 --- a/BRMSGateway/src/test/java/org/onap/policy/brms/api/BrmsGatewayTest.java +++ b/BRMSGateway/src/test/java/org/onap/policy/brms/api/BrmsGatewayTest.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. @@ -20,37 +20,88 @@ package org.onap.policy.brms.api; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.fail; -import static org.mockito.Matchers.any; +import org.junit.After; +import org.junit.AfterClass; +import org.junit.Before; +import org.junit.BeforeClass; import org.junit.Test; -import org.junit.runner.RunWith; import org.mockito.Mockito; -import org.powermock.api.mockito.PowerMockito; -import org.powermock.core.classloader.annotations.PrepareForTest; -import org.powermock.modules.junit4.PowerMockRunner; +import org.onap.policy.api.PolicyException; +import org.onap.policy.brms.api.BrmsGateway.Factory; +import org.powermock.reflect.Whitebox; -@RunWith(PowerMockRunner.class) public class BrmsGatewayTest { + private static final String FACTORY_FIELD = "factory"; + + private static Factory saveFactory; + + private Thread thread; + + @BeforeClass + public static void setUpBeforeClass() { + saveFactory = Whitebox.getInternalState(BrmsGateway.class, FACTORY_FIELD); + } + + @AfterClass + public static void tearDownAfterClass() { + Whitebox.setInternalState(BrmsGateway.class, FACTORY_FIELD, saveFactory); + } + + /** + * Installs a factory. + */ + @Before + public void setUp() { + thread = null; + + Factory factory = new Factory() { + @Override + public BrmsHandler makeBrmsHandler(String configFile) throws PolicyException { + // Mock handler + return Mockito.mock(BrmsHandler.class); + } + + @Override + public Thread makeThread(Runnable runnable) { + thread = super.makeThread(runnable); + return thread; + } + + }; + + Whitebox.setInternalState(BrmsGateway.class, FACTORY_FIELD, factory); + } + + /** + * Interrupts the thread, if there is one. + */ + @After + public void tearDown() throws InterruptedException { + if (thread != null) { + thread.interrupt(); + thread.join(5000L); + assertFalse(thread.isAlive()); + } + } + + @Test + public void testFactory() throws InterruptedException { + assertNotNull(saveFactory); + assertNotNull(saveFactory.makeThread(() -> { })); + } + @Test public void testGet() { assertNull(BrmsGateway.getPolicyEngine()); } - @PrepareForTest({Thread.class, BrmsGateway.class}) @Test public void testMain() throws Exception { - // Mock Thread - PowerMockito.spy(Thread.class); - PowerMockito.doNothing().when(Thread.class); - Thread.sleep(1000); - - // Mock handler - final BrmsHandler handler = Mockito.mock(BrmsHandler.class); - PowerMockito.whenNew(BrmsHandler.class).withArguments(any()).thenReturn(handler); - - // Run app try { final String[] args = new String[0]; BrmsGateway.main(args); diff --git a/POLICY-SDK-APP/src/main/java/org/onap/policy/admin/RESTfulPAPEngine.java b/POLICY-SDK-APP/src/main/java/org/onap/policy/admin/RESTfulPAPEngine.java index a500c1dbe..fc47179c1 100644 --- a/POLICY-SDK-APP/src/main/java/org/onap/policy/admin/RESTfulPAPEngine.java +++ b/POLICY-SDK-APP/src/main/java/org/onap/policy/admin/RESTfulPAPEngine.java @@ -36,7 +36,9 @@ import java.io.InputStream; import java.io.OutputStream; import java.io.UnsupportedEncodingException; import java.net.HttpURLConnection; +import java.net.MalformedURLException; import java.net.URL; +import java.net.URLConnection; import java.net.URLEncoder; import java.nio.charset.StandardCharsets; import java.util.Arrays; @@ -379,12 +381,10 @@ public class RESTfulPAPEngine extends StdPDPItemSetChangeNotifier implements PAP } } - URL url = new URL(fullURL); - // // Open up the connection // - connection = (HttpURLConnection) url.openConnection(); + connection = (HttpURLConnection) makeConnection(fullURL); // // Setup our method and headers // @@ -527,4 +527,10 @@ public class RESTfulPAPEngine extends StdPDPItemSetChangeNotifier implements PAP LOGGER.info("JSON response from PAP: " + json); return json; } + + // these may be overridden by junit tests + + protected URLConnection makeConnection(String fullURL) throws IOException { + return new URL(fullURL).openConnection(); + } } diff --git a/POLICY-SDK-APP/src/test/java/org/onap/policy/admin/PolicyNotificationMailTest.java b/POLICY-SDK-APP/src/test/java/org/onap/policy/admin/PolicyNotificationMailTest.java index 95296f4b0..e270583bb 100644 --- a/POLICY-SDK-APP/src/test/java/org/onap/policy/admin/PolicyNotificationMailTest.java +++ b/POLICY-SDK-APP/src/test/java/org/onap/policy/admin/PolicyNotificationMailTest.java @@ -29,6 +29,7 @@ import java.util.List; import org.junit.Before; import org.junit.Test; import static org.mockito.Mockito.when; +import org.onap.policy.common.utils.network.NetworkUtil; import org.onap.policy.controller.PolicyController; import org.onap.policy.rest.dao.CommonClassDao; import org.onap.policy.rest.jpa.PolicyVersion; @@ -46,11 +47,17 @@ public class PolicyNotificationMailTest { PolicyController.setjUnit(true); PolicyController.setSmtpApplicationName("Test"); PolicyController.setSmtpEmailExtension("test.com"); - PolicyController.setSmtpHost("test"); - PolicyController.setSmtpPort("23"); + PolicyController.setSmtpHost("localhost"); PolicyController.setSmtpPassword("test"); PolicyController.setSmtpUsername("test"); + /* + * Allocate a port to which the mail sender should connect, but don't actually + * start a listener on the port so that connection attempts will be immediately + * rejected. + */ + PolicyController.setSmtpPort(String.valueOf(NetworkUtil.allocPort())); + version = new PolicyVersion(); version.setPolicyName("com/Config_Test"); version.setModifiedBy("xyz"); diff --git a/POLICY-SDK-APP/src/test/java/org/onap/policy/admin/RESTfulPAPEngineTest.java b/POLICY-SDK-APP/src/test/java/org/onap/policy/admin/RESTfulPAPEngineTest.java index 4b307f594..e8434d46d 100644 --- a/POLICY-SDK-APP/src/test/java/org/onap/policy/admin/RESTfulPAPEngineTest.java +++ b/POLICY-SDK-APP/src/test/java/org/onap/policy/admin/RESTfulPAPEngineTest.java @@ -23,28 +23,27 @@ package org.onap.policy.admin; import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; + import java.io.ByteArrayInputStream; +import java.io.IOException; import java.io.InputStream; -import java.net.URL; -import javax.servlet.http.HttpServletResponse; import java.net.HttpURLConnection; +import java.net.URLConnection; +import javax.servlet.http.HttpServletResponse; +import org.junit.AfterClass; import org.junit.Before; +import org.junit.BeforeClass; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; -import org.junit.runner.RunWith; import org.mockito.Mockito; import org.onap.policy.rest.adapter.PolicyRestAdapter; import org.onap.policy.xacml.api.pap.OnapPDP; import org.onap.policy.xacml.api.pap.OnapPDPGroup; -import org.powermock.api.mockito.PowerMockito; -import org.powermock.core.classloader.annotations.PrepareForTest; -import org.powermock.modules.junit4.PowerMockRunner; import com.att.research.xacml.api.pap.PAPException; import com.att.research.xacml.api.pap.PDPPolicy; +import com.att.research.xacml.util.XACMLProperties; -@RunWith(PowerMockRunner.class) -@PrepareForTest({URL.class, RESTfulPAPEngine.class}) public class RESTfulPAPEngineTest { @Rule public ExpectedException thrown = ExpectedException.none(); @@ -61,13 +60,20 @@ public class RESTfulPAPEngineTest { OnapPDP pdp = Mockito.mock(OnapPDP.class); InputStream policy; + @BeforeClass + public static void setUpBeforeClass() { + XACMLProperties.reloadProperties(); + } + + @AfterClass + public static void tearDownAfterClass() { + XACMLProperties.reloadProperties(); + } + @Before public void runConstructor() throws Exception { - // Mock url and connection - URL url = PowerMockito.mock(URL.class); - PowerMockito.whenNew(URL.class).withArguments(Mockito.any()).thenReturn(url); + // Mock connection HttpURLConnection connection = Mockito.mock(HttpURLConnection.class); - Mockito.when(url.openConnection()).thenReturn(connection); Mockito.when(connection.getResponseCode()).thenReturn(HttpServletResponse.SC_NO_CONTENT); // Set the system property temporarily @@ -77,7 +83,12 @@ public class RESTfulPAPEngineTest { // Test constructor String urlName = "localhost:1234"; - engine = new RESTfulPAPEngine(urlName); + engine = new RESTfulPAPEngine(urlName) { + @Override + protected URLConnection makeConnection(String fullURL) throws IOException { + return connection; + } + }; // Initialize policy policy = new ByteArrayInputStream(policyContent.getBytes("UTF-8")); -- 2.16.6