From 68705a5a63dd8ef611369d55f930c066b6fb7b6e Mon Sep 17 00:00:00 2001 From: Clebert Suconic Date: Wed, 15 Nov 2017 17:10:53 -0500 Subject: [PATCH] NO-JIRA Making sure System.properties are cleaned up between tests --- .../utils/CleanupSystemPropertiesRule.java | 91 +++++++++++++++++++ .../artemis/tests/util/ActiveMQTestBase.java | 6 ++ .../openwire/AdvisoryOpenWireTest.java | 12 ++- 3 files changed, 104 insertions(+), 5 deletions(-) create mode 100644 artemis-commons/src/test/java/org/apache/activemq/artemis/utils/CleanupSystemPropertiesRule.java diff --git a/artemis-commons/src/test/java/org/apache/activemq/artemis/utils/CleanupSystemPropertiesRule.java b/artemis-commons/src/test/java/org/apache/activemq/artemis/utils/CleanupSystemPropertiesRule.java new file mode 100644 index 0000000000..ced11d247d --- /dev/null +++ b/artemis-commons/src/test/java/org/apache/activemq/artemis/utils/CleanupSystemPropertiesRule.java @@ -0,0 +1,91 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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. + */ + +package org.apache.activemq.artemis.utils; + +import java.util.HashSet; +import java.util.Map; +import java.util.Properties; + +import org.jboss.logging.Logger; +import org.junit.rules.ExternalResource; + +/** + * This will make sure any properties changed through tests are cleaned up between tests. + */ +public class CleanupSystemPropertiesRule extends ExternalResource { + + private static Logger log = Logger.getLogger(CleanupSystemPropertiesRule.class); + + private Properties originalProperties; + + /** + * Override to set up your specific external resource. + * + * @throws if setup fails (which will disable {@code after} + */ + @Override + protected void before() throws Throwable { + // do nothing + + originalProperties = new Properties(); + originalProperties.putAll(System.getProperties()); + + } + + /** + * Override to tear down your specific external resource. + */ + @Override + protected void after() { + + Properties changed = new Properties(); + HashSet newProperties = new HashSet(); + for (Map.Entry entry : System.getProperties().entrySet()) { + Object originalValue = originalProperties.get(entry.getKey()); + if (originalValue == null) { + newProperties.add(entry.getKey()); + } else if (!originalValue.equals(entry.getValue())) { + changed.put(entry.getKey(), originalValue); + } + } + + if (!newProperties.isEmpty() || !changed.isEmpty()) { + + System.out.println("======================================================================================================"); + + for (Object key : newProperties) { + System.out.println("Cleaning up system property " + key); + System.clearProperty(key.toString()); + } + + for (Map.Entry entry : changed.entrySet()) { + System.out.println("Setting up old system property, key=" + entry.getKey() + ", value = " + entry.getValue()); + System.setProperty(entry.getKey().toString(), entry.getValue().toString()); + } + + System.out.println("======================================================================================================"); + } + + + // lets give GC a hand + originalProperties.clear(); + originalProperties = null; + + } + +} diff --git a/artemis-server/src/test/java/org/apache/activemq/artemis/tests/util/ActiveMQTestBase.java b/artemis-server/src/test/java/org/apache/activemq/artemis/tests/util/ActiveMQTestBase.java index 2d6f0033af..85ad922225 100644 --- a/artemis-server/src/test/java/org/apache/activemq/artemis/tests/util/ActiveMQTestBase.java +++ b/artemis-server/src/test/java/org/apache/activemq/artemis/tests/util/ActiveMQTestBase.java @@ -134,6 +134,7 @@ import org.apache.activemq.artemis.spi.core.security.ActiveMQJAASSecurityManager import org.apache.activemq.artemis.spi.core.security.ActiveMQSecurityManager; import org.apache.activemq.artemis.spi.core.security.jaas.InVMLoginModule; import org.apache.activemq.artemis.utils.ActiveMQThreadFactory; +import org.apache.activemq.artemis.utils.CleanupSystemPropertiesRule; import org.apache.activemq.artemis.utils.Env; import org.apache.activemq.artemis.utils.FileUtil; import org.apache.activemq.artemis.utils.ThreadDumpUtil; @@ -163,9 +164,14 @@ public abstract class ActiveMQTestBase extends Assert { private static final Logger logger = Logger.getLogger(ActiveMQTestBase.class); + /** This will make sure threads are not leaking between tests */ @Rule public ThreadLeakCheckRule leakCheckRule = new ThreadLeakCheckRule(); + /** This will cleanup any system property changed inside tests */ + @Rule + public CleanupSystemPropertiesRule propertiesRule = new CleanupSystemPropertiesRule(); + public static final String TARGET_TMP = "./target/tmp"; public static final String INVM_ACCEPTOR_FACTORY = InVMAcceptorFactory.class.getCanonicalName(); public static final String INVM_CONNECTOR_FACTORY = InVMConnectorFactory.class.getCanonicalName(); diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/openwire/AdvisoryOpenWireTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/openwire/AdvisoryOpenWireTest.java index dc84d94e83..3ad060c8cf 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/openwire/AdvisoryOpenWireTest.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/openwire/AdvisoryOpenWireTest.java @@ -62,8 +62,9 @@ public class AdvisoryOpenWireTest extends BasicOpenWireTest { QueueControl queueControl = (QueueControl) queueResource; Wait.waitFor(() -> queueControl.getMessageCount() == 0); assertNotNull("addressControl for temp advisory", queueControl); - assertEquals(0, queueControl.getMessageCount()); - assertEquals(2, queueControl.getMessagesAdded()); + + Wait.assertEquals(0, queueControl::getMessageCount); + Wait.assertEquals(2, queueControl::getMessagesAdded); } } } finally { @@ -94,8 +95,9 @@ public class AdvisoryOpenWireTest extends BasicOpenWireTest { QueueControl queueControl = (QueueControl) queueResource; Wait.waitFor(() -> queueControl.getMessageCount() == 0); assertNotNull("addressControl for temp advisory", queueControl); - assertEquals(0, queueControl.getMessageCount()); - assertEquals(2, queueControl.getMessagesAdded()); + Wait.assertEquals(0, queueControl::getMessageCount); + Wait.assertEquals(2, queueControl::getMessagesAdded); + } } } finally { @@ -130,7 +132,7 @@ public class AdvisoryOpenWireTest extends BasicOpenWireTest { AddressControl addressControl = (AddressControl) addressResource; Wait.waitFor(() -> addressControl.getMessageCount() == 0); assertNotNull("addressControl for temp advisory", addressControl); - assertEquals(0, addressControl.getMessageCount()); + Wait.assertEquals(0, addressControl::getMessageCount); } }