Cleanup of itests to not influence each other when setting server ide…

…ntity

and manipulating plugins.

Also fixed the PluginManagerBeanTest#isPluginReadyForPurge() by realizing
that other tests (that now correctly clean up after themselves) may have
already deleted the plugins that were marked to be deleted in the test
method the isPluginReadyForPurge depends on.

modules/enterprise/server/itests-2/src/test/java/org/rhq/enterprise/server/core/CoreServerServiceImplTest.java

@@ -103,10 +103,8 @@
 @Test(groups = { "core.agent.registration" })
 public class CoreServerServiceImplTest extends AbstractEJB3Test {
     private static final String TEST_AGENT_NAME_PREFIX = "CoreServerServiceImplTest.Agent";
-    private static final String RHQ_SERVER_NAME_PROPERTY = "rhq.server.high-availability.name";
     private AgentVersion agentVersion;
     private Server server;
-    private String oldServerNamePropertyValue = null;
 
     private static final int A_PORT = 11111;
     private static final String A_HOST = "hostA";
@@ -154,9 +152,7 @@ public void afterClassStandIn() throws Exception {
     @Override
     protected void beforeMethod() throws Exception {
         // mock the name of our server via the sysprop (in production, this is normally set in rhq-server.properties)
-        oldServerNamePropertyValue = System.getProperty(RHQ_SERVER_NAME_PROPERTY);
-        String newServerNamePropertyValue = "CoreServerServiceImplTest.Server";
-        System.setProperty(RHQ_SERVER_NAME_PROPERTY, newServerNamePropertyValue);
+        setServerIdentity("CoreServerServiceImplTest.Server");
 
         // mock up our core server MBean that provides information about where the jboss home dir is
         DummyCoreServer mbean = new DummyCoreServer();
@@ -198,7 +194,7 @@ protected void beforeMethod() throws Exception {
 
         // mock our server
         server = new Server();
-        server.setName(newServerNamePropertyValue);
+        server.setName(getServerIdentity());
         server.setAddress("CoreServerServiceImplTest.localhost");
         server.setPort(12345);
 
@@ -220,11 +216,6 @@ protected void afterMethod() throws Exception {
         unprepareCustomServerService(CoreServerMBean.OBJECT_NAME);
 
         unprepareForTestAgents();
-
-        // in case this was set before our tests, put it back the way it was
-        if (oldServerNamePropertyValue != null) {
-            System.setProperty(RHQ_SERVER_NAME_PROPERTY, oldServerNamePropertyValue);
-        }
     }
 
     @Test

modules/enterprise/server/itests-2/src/test/java/org/rhq/enterprise/server/measurement/MeasurementDataManagerBeanTest.java

@@ -98,8 +98,6 @@
  */
 public class MeasurementDataManagerBeanTest extends AbstractEJB3Test {
 
-    // this must match the constant found in ServerManagerBean
-    private static final String RHQ_SERVER_NAME_PROPERTY = "rhq.server.high-availability.name";
     private static final String RHQ_SERVER_NAME_PROPERTY_VALUE = "TestServer";
 
     //private final Log log = LogFactory.getLog(MeasurementDataManagerBeanTest.class);
@@ -154,6 +152,8 @@ public class MeasurementDataManagerBeanTest extends AbstractEJB3Test {
 
     private TestServerCommunicationsService agentServiceContainer;
 
+    private String oldServerName;
+
     private Subject getOverlord() {
         return subjectManager.getOverlord();
     }
@@ -172,7 +172,7 @@ protected void beforeMethod() throws Exception {
         prepareCustomServerPluginService(driftServerPluginService);
         driftServerPluginService.masterConfig.getPluginDirectory().mkdirs();
 
-        System.setProperty(RHQ_SERVER_NAME_PROPERTY, RHQ_SERVER_NAME_PROPERTY_VALUE);
+        setServerIdentity(RHQ_SERVER_NAME_PROPERTY_VALUE);
 
         createInventory();
         insertDummyReport();

modules/enterprise/server/itests-2/src/test/java/org/rhq/enterprise/server/resource/metadata/MetadataBeanTest.java

@@ -32,7 +32,9 @@
 
 import org.rhq.core.clientapi.descriptor.plugin.PluginDescriptor;
 import org.rhq.core.domain.auth.Subject;
+import org.rhq.core.domain.cloud.Server;
 import org.rhq.core.domain.criteria.ResourceTypeCriteria;
+import org.rhq.core.domain.criteria.ServerCriteria;
 import org.rhq.core.domain.plugin.Plugin;
 import org.rhq.core.domain.resource.ResourceType;
 import org.rhq.core.util.MessageDigestGenerator;
@@ -70,6 +72,10 @@ protected void afterClassWork() throws Exception {
         Subject overlord = LookupUtil.getSubjectManager().getOverlord();
         List<Integer> doomedPlugins = new ArrayList<Integer>(pluginIds);
         pluginMgr.deletePlugins(overlord, doomedPlugins);
+
+        //the following 3 lines ensure we truly delete the above plugins
+        //from the database
+        ackDeletedPlugins();
         new PurgeResourceTypesJob().executeJobCode(null);
         new PurgePluginsJob().executeJobCode(null);
 

modules/enterprise/server/itests-2/src/test/java/org/rhq/enterprise/server/resource/metadata/PluginManagerBeanTest.java

@@ -28,6 +28,9 @@
 
 import org.testng.annotations.Test;
 
+import org.rhq.core.domain.auth.Subject;
+import org.rhq.core.domain.cloud.Server;
+import org.rhq.core.domain.criteria.ServerCriteria;
 import org.rhq.core.domain.plugin.Plugin;
 import org.rhq.core.domain.plugin.PluginStatusType;
 import org.rhq.core.domain.resource.ResourceType;
@@ -229,14 +232,35 @@ public void isPluginReadyForPurge() throws Exception {
         InventoryManagerLocal inventoryManager = LookupUtil.getInventoryManager();
 
         Plugin plugin = getDeletedPlugin(PLUGIN_1);
+        if (plugin == null) {
+            //ok so there's no delete plugin like that. Let's check that there's no installed plugin either
+            plugin = getPlugin(PLUGIN_1);
+            if (plugin != null) {
+                fail(PLUGIN_1 + "should have been delete in PluginManagerBeanTest#deletePlugins()");
+            }
+
+            //So there's no such plugin at all. This means that some other test intertwined between this test and
+            //deletePlugins.
+            //
+            //Because tests are configured to clean up after themselves (mainly in the afterClassWork() method)
+            //it may happen that the plugin we marked for deletion in deletePlugins has actually been deleted
+            //by the clean up methods.
+            //
+            //The point of this test is in that case fulfilled anyway because by the plugin disappearing,
+            //we proved that at some point in time between deletePlugins and this test it was indeed purgeable.
+            return;
+        }
+
         List<ResourceType> resourceTypes = resourceTypeManager.getResourceTypesByPlugin(plugin.getName());
         List<ResourceType> deletedTypes = inventoryManager.getDeletedTypes();
 
         assertTrue("All of the resource types declared in " + plugin + " should have already been deleted",
             deletedTypes.containsAll(resourceTypes));
 
+        ackDeletedPlugins();
+
         assertFalse("A plugin is not ready to be purged until all of its resource types have already been purged "
-            + "and until the plugin itself has been marked for purge", pluginMgr.isReadyForPurge(plugin));
+            + "and until the plugin itself has been acked for deletion by all servers", pluginMgr.isReadyForPurge(plugin));
     }
 
     private Plugin getDeletedPlugin(String name) {
@@ -271,8 +295,10 @@ public void pluginPurgeCheckShouldUseExactMatchesInQuery() throws Exception {
         inventoryManager.purgeDeletedResourceType(resourceType);
         inventoryManager.purgeDeletedResourceType(resourceTypeIgnored);
 
+        ackDeletedPlugins();
+
         assertTrue("Expected " + plugin3 + " to be ready for purge since all its resource types have been purged "
-            + "and the plugin has been marked for purge", pluginMgr.isReadyForPurge(plugin3));
+            + "and the servers acked its deletion", pluginMgr.isReadyForPurge(plugin3));
     }
 
     //TODO make this work again

modules/enterprise/server/itests-2/src/test/java/org/rhq/enterprise/server/resource/metadata/test/UpdatePluginMetadataTestBase.java

@@ -62,8 +62,6 @@
 
 public class UpdatePluginMetadataTestBase extends AbstractEJB3Test {
 
-    // this must match the constant found in ServerManagerBean
-    protected static final String RHQ_SERVER_NAME_PROPERTY = "rhq.server.high-availability.name";
     protected static final String RHQ_SERVER_NAME_PROPERTY_VALUE = "TestServer";
 
     protected TestServerCommunicationsService agentServiceContainer;
@@ -93,7 +91,7 @@ protected void beforeMethod() throws Exception {
         resourceTypeManager = LookupUtil.getResourceTypeManager();
         resourceManager = LookupUtil.getResourceManager();
 
-        System.setProperty(RHQ_SERVER_NAME_PROPERTY, RHQ_SERVER_NAME_PROPERTY_VALUE);
+        setServerIdentity(RHQ_SERVER_NAME_PROPERTY_VALUE);
     }
 
     @Override
@@ -106,8 +104,6 @@ protected void afterMethod() throws Exception {
         unprepareForTestAgents();
 
         deleteServerIdentity();
-
-        System.setProperty(RHQ_SERVER_NAME_PROPERTY, "");
     }
 
     protected void prepareMockAgentServiceContainer() {

modules/enterprise/server/itests-2/src/test/java/org/rhq/enterprise/server/test/AbstractEJB3Test.java

@@ -73,6 +73,8 @@
 
 import org.rhq.core.db.DatabaseTypeFactory;
 import org.rhq.core.domain.auth.Subject;
+import org.rhq.core.domain.cloud.Server;
+import org.rhq.core.domain.criteria.ServerCriteria;
 import org.rhq.core.domain.shared.BuilderException;
 import org.rhq.core.domain.shared.ResourceBuilder;
 import org.rhq.core.domain.shared.ResourceTypeBuilder;
@@ -86,6 +88,7 @@
 import org.rhq.enterprise.server.core.plugin.PluginDeploymentScannerMBean;
 import org.rhq.enterprise.server.plugin.pc.ServerPluginService;
 import org.rhq.enterprise.server.plugin.pc.ServerPluginServiceMBean;
+import org.rhq.enterprise.server.resource.metadata.PluginManagerLocal;
 import org.rhq.enterprise.server.scheduler.SchedulerService;
 import org.rhq.enterprise.server.scheduler.SchedulerServiceMBean;
 import org.rhq.enterprise.server.storage.FakeStorageClusterSettingsManagerBean;
@@ -111,6 +114,8 @@ public abstract class AbstractEJB3Test extends Arquillian {
     private ServerPluginService serverPluginService;
     private PluginDeploymentScannerMBean pluginScannerService;
 
+    private String originalServerIdentity;
+
     @PersistenceContext(unitName = RHQConstants.PERSISTENCE_UNIT_NAME)
     protected EntityManager em;
 
@@ -532,6 +537,7 @@ protected void __beforeMethod(Method method) throws Throwable {
                         }
                     }
                 }
+                originalServerIdentity = System.getProperty(TestConstants.RHQ_SERVER_NAME_PROPERTY);
                 storageClientManager.init();
                 beforeMethod();
                 beforeMethod(method);
@@ -555,6 +561,10 @@ protected void __beforeMethod(Method method) throws Throwable {
     protected void __afterMethod(ITestResult result, Method method) throws Throwable {
         try {
             if (inContainer()) {
+                if (originalServerIdentity != null) {
+                    System.setProperty(TestConstants.RHQ_SERVER_NAME_PROPERTY, originalServerIdentity);
+                }
+
                 afterMethod();
                 afterMethod(result, method);
             }
@@ -600,6 +610,24 @@ protected void afterMethod(ITestResult result, Method meth) throws Exception {
         // do nothing if we're not overridden
     }
 
+    /**
+     * Safe to be used inside {@link #beforeMethod()}, {@link #afterMethod()} and tests.
+     * Sets the HA identity of the current server. Does <b>NOT</b> define the server in the database, merely sets
+     * its identity as a system property.
+     * <p/>
+     * The value is reset after each test, so tests don't influence each other.
+     */
+    protected void setServerIdentity(String name) {
+        System.setProperty(TestConstants.RHQ_SERVER_NAME_PROPERTY, name);
+    }
+
+    /**
+     * Gets the current server identity. This is just a value of the system property, no DB row may exist for the server.
+     */
+    protected String getServerIdentity() {
+        return System.getProperty(TestConstants.RHQ_SERVER_NAME_PROPERTY);
+    }
+
     protected void startTransaction() throws Exception {
         getTransactionManager().begin();
     }
@@ -1155,4 +1183,11 @@ public File getTempDir() {
         return new File(tmpdirRoot, this.getClass().getSimpleName());
     }
 
+    protected void ackDeletedPlugins() {
+        Subject overlord = LookupUtil.getSubjectManager().getOverlord();
+        PluginManagerLocal pluginMgr = LookupUtil.getPluginManager();
+        for(Server server : LookupUtil.getTopologyManager().findServersByCriteria(overlord, new ServerCriteria())) {
+            pluginMgr.acknowledgeDeletedPluginsBy(server.getId());
+        }
+    }
 }