From 19663843f4fc612f73df8ba970800f2f1a5166a2 Mon Sep 17 00:00:00 2001 From: Jim Hahn Date: Tue, 7 Apr 2020 09:18:12 -0400 Subject: Address sonar issues in policy-management Addressed the following sonar issues: - modified code to specify the correct class name in the getLogger() call - use equals() instead of "==" for string comparison - remove deprecated code - use ',' instead of "," in indexOf - remove code that is commented out; typically bogus, so the comment was adjusted to satisfy sonar - missing assert in junits - use "{}" instead of concatenation when using logger - either log or rethrow - put arguments for assertEquals() in the correct order - remove "return" statements from the end of void methods - don't always return the same value; just disabled sonar as refactoring would have obfuscated the code - cognitive complexity; used eclipse auto-refactoring to extract out chunks of code into separate methods - don't pass array of classes to class.getDeclaredMethod(); use ellided arguments instead - fix argument count in logger calls - remove unnecessary casts - don't use "volatile" - make methods "synchronized" to match parent class definitions Issue-ID: POLICY-2305 Change-Id: Ie96418f696da4ae6c2ca8d4a914371469e695419 Signed-off-by: Jim Hahn --- .../internal/MavenDroolsController2Test.java | 3 ++- .../controller/internal/NullDroolsControllerTest.java | 11 +++++++---- .../configuration/ControllerConfigurationTest.java | 16 ++++++++-------- .../configuration/DroolsConfigurationTest.java | 19 +++++++++---------- .../protocol/configuration/PdpdConfigurationTest.java | 12 ++++++------ .../policy/drools/system/PolicyEngineManagerTest.java | 3 ++- .../onap/policy/drools/system/PolicyEngineTest.java | 1 + .../internal/AggregatedPolicyControllerTest.java | 7 +++++-- .../drools/system/internal/FeatureLockImplTest.java | 3 ++- .../drools/system/internal/SimpleLockManagerTest.java | 3 ++- 10 files changed, 44 insertions(+), 34 deletions(-) (limited to 'policy-management/src/test') diff --git a/policy-management/src/test/java/org/onap/policy/drools/controller/internal/MavenDroolsController2Test.java b/policy-management/src/test/java/org/onap/policy/drools/controller/internal/MavenDroolsController2Test.java index 2c248483..26103aa2 100644 --- a/policy-management/src/test/java/org/onap/policy/drools/controller/internal/MavenDroolsController2Test.java +++ b/policy-management/src/test/java/org/onap/policy/drools/controller/internal/MavenDroolsController2Test.java @@ -20,6 +20,7 @@ package org.onap.policy.drools.controller.internal; +import static org.assertj.core.api.Assertions.assertThatCode; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; import static org.assertj.core.api.Assertions.assertThatIllegalStateException; import static org.junit.Assert.assertEquals; @@ -613,7 +614,7 @@ public class MavenDroolsController2Test { } }; - drools.updateToVersion(GROUP, ARTIFACT, VERSION2, null, null); + assertThatCode(() -> drools.updateToVersion(GROUP, ARTIFACT, VERSION2, null, null)).doesNotThrowAnyException(); } @Test diff --git a/policy-management/src/test/java/org/onap/policy/drools/controller/internal/NullDroolsControllerTest.java b/policy-management/src/test/java/org/onap/policy/drools/controller/internal/NullDroolsControllerTest.java index 4f9b59d2..ec761ebf 100644 --- a/policy-management/src/test/java/org/onap/policy/drools/controller/internal/NullDroolsControllerTest.java +++ b/policy-management/src/test/java/org/onap/policy/drools/controller/internal/NullDroolsControllerTest.java @@ -20,6 +20,8 @@ package org.onap.policy.drools.controller.internal; +import static org.assertj.core.api.Assertions.assertThatCode; + import org.junit.Assert; import org.junit.Test; import org.onap.policy.common.utils.gson.GsonTestUtils; @@ -43,7 +45,8 @@ public class NullDroolsControllerTest { @Test public void testSerialize() { - new GsonTestUtils().compareGson(new NullDroolsController(), NullDroolsControllerTest.class); + assertThatCode(() -> new GsonTestUtils().compareGson(new NullDroolsController(), + NullDroolsControllerTest.class)).doesNotThrowAnyException(); } @Test @@ -57,17 +60,17 @@ public class NullDroolsControllerTest { @Test public void getGroupId() { - Assert.assertEquals(new NullDroolsController().getGroupId(), DroolsControllerConstants.NO_GROUP_ID); + Assert.assertEquals(DroolsControllerConstants.NO_GROUP_ID, new NullDroolsController().getGroupId()); } @Test public void getArtifactId() { - Assert.assertEquals(new NullDroolsController().getArtifactId(), DroolsControllerConstants.NO_ARTIFACT_ID); + Assert.assertEquals(DroolsControllerConstants.NO_ARTIFACT_ID, new NullDroolsController().getArtifactId()); } @Test public void getVersion() { - Assert.assertEquals(new NullDroolsController().getVersion(), DroolsControllerConstants.NO_VERSION); + Assert.assertEquals(DroolsControllerConstants.NO_VERSION, new NullDroolsController().getVersion()); } @Test diff --git a/policy-management/src/test/java/org/onap/policy/drools/protocol/configuration/ControllerConfigurationTest.java b/policy-management/src/test/java/org/onap/policy/drools/protocol/configuration/ControllerConfigurationTest.java index a3d53eba..29650bf0 100644 --- a/policy-management/src/test/java/org/onap/policy/drools/protocol/configuration/ControllerConfigurationTest.java +++ b/policy-management/src/test/java/org/onap/policy/drools/protocol/configuration/ControllerConfigurationTest.java @@ -2,7 +2,7 @@ * ============LICENSE_START======================================================= * Configuration Test * ================================================================================ - * Copyright (C) 2017-2019 AT&T Intellectual Property. All rights reserved. + * Copyright (C) 2017-2020 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. @@ -74,9 +74,9 @@ public class ControllerConfigurationTest { controllerConfig2.setOperation(OPERATION2); controllerConfig2.setDrools(DROOLS_CONFIG2); - assertEquals(controllerConfig2.getName(), NAME2); - assertEquals(controllerConfig2.getOperation(), OPERATION2); - assertEquals(controllerConfig2.getDrools(), DROOLS_CONFIG2); + assertEquals(NAME2, controllerConfig2.getName()); + assertEquals(OPERATION2, controllerConfig2.getOperation()); + assertEquals(DROOLS_CONFIG2, controllerConfig2.getDrools()); assertEquals(controllerConfig2, controllerConfig2.withName(NAME2)); assertEquals(controllerConfig2, controllerConfig2.withOperation(OPERATION2)); @@ -94,10 +94,10 @@ public class ControllerConfigurationTest { assertFalse(controllerConfig2.declaredProperty("dummy", NAME)); - assertEquals(controllerConfig2.declaredPropertyOrNotFound(NAME, NAME2), NAME2); - assertEquals(controllerConfig2.declaredPropertyOrNotFound(OPERATION, OPERATION2), OPERATION2); - assertEquals(controllerConfig2.declaredPropertyOrNotFound(DROOLS_STRING, DROOLS_CONFIG2), DROOLS_CONFIG2); - assertEquals(controllerConfig2.declaredPropertyOrNotFound("dummy", NAME), NAME); + assertEquals(NAME2, controllerConfig2.declaredPropertyOrNotFound(NAME, NAME2)); + assertEquals(OPERATION2, controllerConfig2.declaredPropertyOrNotFound(OPERATION, OPERATION2)); + assertEquals(DROOLS_CONFIG2, controllerConfig2.declaredPropertyOrNotFound(DROOLS_STRING, DROOLS_CONFIG2)); + assertEquals(NAME, controllerConfig2.declaredPropertyOrNotFound("dummy", NAME)); int hashCode = new HashCodeBuilder().append(NAME2).append(OPERATION2).append(DROOLS_CONFIG2) .append(additionalProperties).toHashCode(); diff --git a/policy-management/src/test/java/org/onap/policy/drools/protocol/configuration/DroolsConfigurationTest.java b/policy-management/src/test/java/org/onap/policy/drools/protocol/configuration/DroolsConfigurationTest.java index 4ad4f880..a085c950 100644 --- a/policy-management/src/test/java/org/onap/policy/drools/protocol/configuration/DroolsConfigurationTest.java +++ b/policy-management/src/test/java/org/onap/policy/drools/protocol/configuration/DroolsConfigurationTest.java @@ -2,7 +2,7 @@ * ============LICENSE_START======================================================= * Configuration Test * ================================================================================ - * Copyright (C) 2017-2019 AT&T Intellectual Property. All rights reserved. + * Copyright (C) 2017-2020 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. @@ -59,7 +59,7 @@ public class DroolsConfigurationTest { assertTrue(droolsConfig.equals(droolsConfig)); droolsConfig.set(ARTIFACT_ID_STRING, "foobar"); - assertEquals(droolsConfig.get(ARTIFACT_ID_STRING), "foobar"); + assertEquals("foobar", droolsConfig.get(ARTIFACT_ID_STRING)); assertEquals(droolsConfig.with(ARTIFACT_ID_STRING, "foobar2"), droolsConfig); @@ -68,9 +68,9 @@ public class DroolsConfigurationTest { droolsConfig2.setGroupId(GROUPID2); droolsConfig2.setVersion(VERSION2); - assertEquals(droolsConfig2.getArtifactId(), ARTIFACT2); - assertEquals(droolsConfig2.getGroupId(), GROUPID2); - assertEquals(droolsConfig2.getVersion(), VERSION2); + assertEquals(ARTIFACT2, droolsConfig2.getArtifactId()); + assertEquals(GROUPID2, droolsConfig2.getGroupId()); + assertEquals(VERSION2, droolsConfig2.getVersion()); assertEquals(droolsConfig2.withArtifactId(ARTIFACT2), droolsConfig2); assertEquals(droolsConfig2.withGroupId(GROUPID2), droolsConfig2); @@ -87,11 +87,10 @@ public class DroolsConfigurationTest { assertTrue(droolsConfig2.declaredProperty(VERSION_STRING, VERSION2)); assertFalse(droolsConfig2.declaredProperty("dummy", NAME)); - assertEquals(droolsConfig2.declaredPropertyOrNotFound(ARTIFACT_ID_STRING, ARTIFACT2), - ARTIFACT2); - assertEquals(droolsConfig2.declaredPropertyOrNotFound(GROUP_ID_STRING, GROUPID2), GROUPID2); - assertEquals(droolsConfig2.declaredPropertyOrNotFound(VERSION_STRING, VERSION2), VERSION2); - assertEquals(droolsConfig2.declaredPropertyOrNotFound("dummy", ARTIFACT2), ARTIFACT2); + assertEquals(ARTIFACT2, droolsConfig2.declaredPropertyOrNotFound(ARTIFACT_ID_STRING, ARTIFACT2)); + assertEquals(GROUPID2, droolsConfig2.declaredPropertyOrNotFound(GROUP_ID_STRING, GROUPID2)); + assertEquals(VERSION2, droolsConfig2.declaredPropertyOrNotFound(VERSION_STRING, VERSION2)); + assertEquals(ARTIFACT2, droolsConfig2.declaredPropertyOrNotFound("dummy", ARTIFACT2)); final int hashCode = new HashCodeBuilder().append(ARTIFACT2).append(GROUPID2).append(VERSION2) .append(additionalProperties).toHashCode(); diff --git a/policy-management/src/test/java/org/onap/policy/drools/protocol/configuration/PdpdConfigurationTest.java b/policy-management/src/test/java/org/onap/policy/drools/protocol/configuration/PdpdConfigurationTest.java index a21cd5b8..5da8e2e5 100644 --- a/policy-management/src/test/java/org/onap/policy/drools/protocol/configuration/PdpdConfigurationTest.java +++ b/policy-management/src/test/java/org/onap/policy/drools/protocol/configuration/PdpdConfigurationTest.java @@ -2,7 +2,7 @@ * ============LICENSE_START======================================================= * Configuration Test * ================================================================================ - * Copyright (C) 2017-2019 AT&T Intellectual Property. All rights reserved. + * Copyright (C) 2017-2020 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. @@ -107,7 +107,7 @@ public class PdpdConfigurationTest { // // Test get additional properties // - assertEquals(drools.getAdditionalProperties().size(), 1); + assertEquals(1, drools.getAdditionalProperties().size()); // // Test Not found @@ -165,7 +165,7 @@ public class PdpdConfigurationTest { // // Test additional properties // - assertEquals(controller.getAdditionalProperties().size(), 1); + assertEquals(1, controller.getAdditionalProperties().size()); // // Not found @@ -230,7 +230,7 @@ public class PdpdConfigurationTest { // Test additional properties // - assertEquals(config.getAdditionalProperties().size(), 1); + assertEquals(1, config.getAdditionalProperties().size()); // // Test NOT FOUND @@ -249,8 +249,8 @@ public class PdpdConfigurationTest { @Test public void testConstructor() { PdpdConfiguration config = new PdpdConfiguration(REQUEST_ID, ENTITY, null); - assertEquals(config.getRequestId(), REQUEST_ID); - assertEquals(config.getEntity(), ENTITY); + assertEquals(REQUEST_ID, config.getRequestId()); + assertEquals(ENTITY, config.getEntity()); } @Test diff --git a/policy-management/src/test/java/org/onap/policy/drools/system/PolicyEngineManagerTest.java b/policy-management/src/test/java/org/onap/policy/drools/system/PolicyEngineManagerTest.java index b5c83e43..81714cfd 100644 --- a/policy-management/src/test/java/org/onap/policy/drools/system/PolicyEngineManagerTest.java +++ b/policy-management/src/test/java/org/onap/policy/drools/system/PolicyEngineManagerTest.java @@ -20,6 +20,7 @@ package org.onap.policy.drools.system; +import static org.assertj.core.api.Assertions.assertThatCode; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; import static org.assertj.core.api.Assertions.assertThatIllegalStateException; import static org.assertj.core.api.Assertions.assertThatThrownBy; @@ -315,7 +316,7 @@ public class PolicyEngineManagerTest { @Test public void testSerialize() { mgr.configure(properties); - gson.compareGson(mgr, PolicyEngineManagerTest.class); + assertThatCode(() -> gson.compareGson(mgr, PolicyEngineManagerTest.class)).doesNotThrowAnyException(); } @Test diff --git a/policy-management/src/test/java/org/onap/policy/drools/system/PolicyEngineTest.java b/policy-management/src/test/java/org/onap/policy/drools/system/PolicyEngineTest.java index 5f4815f7..ad0f4148 100644 --- a/policy-management/src/test/java/org/onap/policy/drools/system/PolicyEngineTest.java +++ b/policy-management/src/test/java/org/onap/policy/drools/system/PolicyEngineTest.java @@ -305,5 +305,6 @@ public class PolicyEngineTest { PolicyEngineConstants.getManager().stop(); await().atMost(10, TimeUnit.SECONDS).until(() -> !PolicyEngineConstants.getManager().isAlive()); + assertFalse(PolicyEngineConstants.getManager().isAlive()); } } diff --git a/policy-management/src/test/java/org/onap/policy/drools/system/internal/AggregatedPolicyControllerTest.java b/policy-management/src/test/java/org/onap/policy/drools/system/internal/AggregatedPolicyControllerTest.java index cd0d0571..0b959f05 100644 --- a/policy-management/src/test/java/org/onap/policy/drools/system/internal/AggregatedPolicyControllerTest.java +++ b/policy-management/src/test/java/org/onap/policy/drools/system/internal/AggregatedPolicyControllerTest.java @@ -20,6 +20,7 @@ package org.onap.policy.drools.system.internal; +import static org.assertj.core.api.Assertions.assertThatCode; import static org.assertj.core.api.Assertions.assertThatIllegalStateException; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.Assert.assertEquals; @@ -44,6 +45,7 @@ import org.onap.policy.common.endpoints.event.comm.Topic.CommInfrastructure; import org.onap.policy.common.endpoints.event.comm.TopicEndpoint; import org.onap.policy.common.endpoints.event.comm.TopicSink; import org.onap.policy.common.endpoints.event.comm.TopicSource; +import org.onap.policy.common.utils.gson.GsonTestUtils; import org.onap.policy.drools.controller.DroolsController; import org.onap.policy.drools.controller.DroolsControllerFactory; import org.onap.policy.drools.features.PolicyControllerFeatureApi; @@ -262,8 +264,9 @@ public class AggregatedPolicyControllerTest { @Test public void testSerialize() { - new GsonMgmtTestBuilder().addDroolsControllerMock().addTopicSinkMock().addTopicSourceMock().build() - .compareGson(apc, AggregatedPolicyControllerTest.class); + GsonTestUtils gson = new GsonMgmtTestBuilder().addDroolsControllerMock().addTopicSinkMock().addTopicSourceMock() + .build(); + assertThatCode(() -> gson.compareGson(apc, AggregatedPolicyControllerTest.class)).doesNotThrowAnyException(); } @Test diff --git a/policy-management/src/test/java/org/onap/policy/drools/system/internal/FeatureLockImplTest.java b/policy-management/src/test/java/org/onap/policy/drools/system/internal/FeatureLockImplTest.java index b438853f..791f720f 100644 --- a/policy-management/src/test/java/org/onap/policy/drools/system/internal/FeatureLockImplTest.java +++ b/policy-management/src/test/java/org/onap/policy/drools/system/internal/FeatureLockImplTest.java @@ -21,6 +21,7 @@ package org.onap.policy.drools.system.internal; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -347,7 +348,7 @@ public class FeatureLockImplTest { MyLockStdSession lock = new MyLockStdSession(LockState.WAITING, RESOURCE, OWNER_KEY, HOLD_SEC, callback); // this should invoke the real policy session without throwing an exception - lock.grant(); + assertThatCode(() -> lock.grant()).doesNotThrowAnyException(); } @Test diff --git a/policy-management/src/test/java/org/onap/policy/drools/system/internal/SimpleLockManagerTest.java b/policy-management/src/test/java/org/onap/policy/drools/system/internal/SimpleLockManagerTest.java index f60c4595..09b73ac7 100644 --- a/policy-management/src/test/java/org/onap/policy/drools/system/internal/SimpleLockManagerTest.java +++ b/policy-management/src/test/java/org/onap/policy/drools/system/internal/SimpleLockManagerTest.java @@ -21,6 +21,7 @@ package org.onap.policy.drools.system.internal; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.Assert.assertEquals; @@ -398,7 +399,7 @@ public class SimpleLockManagerTest { feature.createLock(RESOURCE, OWNER_KEY, HOLD_SEC, callback, false); // should shut down thread pool - feature.stop(); + assertThatCode(() -> feature.stop()).doesNotThrowAnyException(); } @Test -- cgit 1.2.3-korg