ISPN-9067 Fix test node names

* Only set the node name once
* Fail the test if a node doesn't have a proper name set
* Don't start the cache manager in the configuration parsing tests

core/src/test/java/org/infinispan/configuration/StringPropertyReplacementTest.java

@@ -5,9 +5,11 @@
 import java.util.Properties;
 
 import org.infinispan.commons.executors.BlockingThreadPoolExecutorFactory;
-import org.infinispan.manager.EmbeddedCacheManager;
-import org.infinispan.test.SingleCacheManagerTest;
-import org.infinispan.test.fwk.TestCacheManagerFactory;
+import org.infinispan.configuration.cache.Configuration;
+import org.infinispan.configuration.global.GlobalConfiguration;
+import org.infinispan.configuration.parsing.ConfigurationBuilderHolder;
+import org.infinispan.configuration.parsing.ParserRegistry;
+import org.infinispan.test.AbstractInfinispanTest;
 import org.infinispan.util.concurrent.IsolationLevel;
 import org.testng.annotations.Test;
 
@@ -19,35 +21,33 @@
  * @author Mircea.Markus@jboss.com
  */
 @Test (groups = "functional", testName = "config.StringPropertyReplacementTest")
-public class StringPropertyReplacementTest extends SingleCacheManagerTest {
-
-
-   @Override
-   protected EmbeddedCacheManager createCacheManager() throws Exception {
-      System.setProperty("test.property.asyncListenerMaxThreads","2");
-      System.setProperty("test.property.persistenceMaxThreads","4");
-      System.setProperty("test.property.IsolationLevel","READ_COMMITTED");
-      System.setProperty("test.property.writeSkewCheck","true");
-      System.setProperty("test.property.SyncCommitPhase","true");
-      return TestCacheManagerFactory.fromXml("configs/string-property-replaced.xml");
+public class StringPropertyReplacementTest extends AbstractInfinispanTest {
+
+   protected ConfigurationBuilderHolder parse() throws Exception {
+      System.setProperty("StringPropertyReplacementTest.asyncListenerMaxThreads","2");
+      System.setProperty("StringPropertyReplacementTest.persistenceMaxThreads","4");
+      System.setProperty("StringPropertyReplacementTest.IsolationLevel","READ_COMMITTED");
+      System.setProperty("StringPropertyReplacementTest.writeSkewCheck","true");
+      System.setProperty("StringPropertyReplacementTest.SyncCommitPhase","true");
+      ParserRegistry parserRegistry = new ParserRegistry(Thread.currentThread().getContextClassLoader(), true);
+      return parserRegistry.parseFile("configs/string-property-replaced.xml");
    }
 
-   public void testGlobalConfig() {
+   public void testGlobalConfig() throws Exception {
+      ConfigurationBuilderHolder holder = parse();
+      GlobalConfiguration gc = holder.getGlobalConfigurationBuilder().build();
       BlockingThreadPoolExecutorFactory listenerThreadPool =
-            cacheManager.getCacheManagerConfiguration().listenerThreadPool().threadPoolFactory();
+            gc.listenerThreadPool().threadPoolFactory();
       assertEquals(2, listenerThreadPool.maxThreads());
 
       BlockingThreadPoolExecutorFactory persistenceThreadPool =
-            cacheManager.getCacheManagerConfiguration().persistenceThreadPool().threadPoolFactory();
+            gc.persistenceThreadPool().threadPoolFactory();
       assertEquals(4, persistenceThreadPool.maxThreads());
 
-      Properties transportProps = cacheManager.getCacheManagerConfiguration().transport().properties();
-      // Should be "jgroups-tcp.xml", but gets overriden by test cache manager factory
-      assert transportProps.get("configurationFile") == null;
-   }
+      Properties transportProps = gc.transport().properties();
+      assertEquals("jgroups-tcp.xml", transportProps.get("configurationFile"));
 
-   public void testDefaultCache() {
-      org.infinispan.configuration.cache.Configuration configuration = cacheManager.getCache().getCacheConfiguration();
-      assert configuration.locking().isolationLevel().equals(IsolationLevel.READ_COMMITTED);
+      Configuration configuration = holder.getDefaultConfigurationBuilder().build();
+      assertEquals(IsolationLevel.READ_COMMITTED, configuration.locking().isolationLevel());
    }
 }

core/src/test/java/org/infinispan/health/ClusterHealthImplTest.java

@@ -14,36 +14,32 @@
 import org.infinispan.Cache;
 import org.infinispan.configuration.cache.CacheMode;
 import org.infinispan.configuration.cache.ConfigurationBuilder;
-import org.infinispan.configuration.global.GlobalConfigurationBuilder;
 import org.infinispan.distribution.DistributionManager;
 import org.infinispan.health.impl.ClusterHealthImpl;
 import org.infinispan.manager.DefaultCacheManager;
 import org.infinispan.manager.EmbeddedCacheManager;
 import org.infinispan.registry.InternalCacheRegistry;
+import org.infinispan.test.AbstractInfinispanTest;
+import org.infinispan.test.fwk.TestCacheManagerFactory;
 import org.testng.annotations.AfterClass;
 import org.testng.annotations.AfterMethod;
 import org.testng.annotations.BeforeClass;
 import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.Test;
 
 @Test(testName = "health.ClusterHealthImplTest", groups = "functional")
-public class ClusterHealthImplTest {
+public class ClusterHealthImplTest extends AbstractInfinispanTest {
 
    private static final String INTERNAL_CACHE_NAME = "internal_cache";
    private static final String CACHE_NAME = "test_cache";
-   private static final String CLUSTER_NAME = "testCluster";
-   private static final String NODE_NAME = "testNode";
    private EmbeddedCacheManager cacheManager;
    private DefaultCacheManager mockedCacheManager;
    private ClusterHealth clusterHealth;
    private InternalCacheRegistry internalCacheRegistry;
 
    @BeforeClass
    private void init() {
-      GlobalConfigurationBuilder globalConfigurationBuilder = new GlobalConfigurationBuilder().clusteredDefault();
-      globalConfigurationBuilder.transport().clusterName(CLUSTER_NAME).nodeName(NODE_NAME);
-
-      cacheManager = new DefaultCacheManager(globalConfigurationBuilder.build());
+      cacheManager = TestCacheManagerFactory.createClusteredCacheManager();
       internalCacheRegistry = cacheManager.getGlobalComponentRegistry().getComponent(InternalCacheRegistry.class);
       clusterHealth = new ClusterHealthImpl(cacheManager);
    }
@@ -61,7 +57,7 @@ private void configureBeforeMethod() {
       cacheManager.defineConfiguration(CACHE_NAME, new ConfigurationBuilder().build());
    }
 
-   @AfterMethod
+   @AfterMethod(alwaysRun = true)
    private void cleanAfterMethod() {
       cacheManager.administration().removeCache(CACHE_NAME);
       cacheManager.undefineConfiguration(CACHE_NAME);
@@ -70,7 +66,7 @@ private void cleanAfterMethod() {
       internalCacheRegistry.unregisterInternalCache(INTERNAL_CACHE_NAME);
    }
 
-   @AfterClass
+   @AfterClass(alwaysRun = true)
    private void cleanUp() {
       if (cacheManager != null) {
          cacheManager.stop();
@@ -79,7 +75,7 @@ private void cleanUp() {
    }
 
    public void testGetClusterName() throws Exception {
-      assertEquals(CLUSTER_NAME, clusterHealth.getClusterName());
+      assertEquals(cacheManager.getClusterName(), clusterHealth.getClusterName());
    }
 
    public void testCallingGetHealthStatusDoesNotCreateAnyCache() throws Exception {
@@ -150,7 +146,7 @@ public void testRebalancingStatusWhenInternalCacheIsRebalancing() throws Excepti
    }
 
    public void testGetNodeNames() throws Exception {
-      assertTrue(clusterHealth.getNodeNames().get(0).contains(NODE_NAME));
+      assertEquals(cacheManager.getAddress().toString(), clusterHealth.getNodeNames().get(0));
    }
 
    public void testGetNumberOfNodes() throws Exception {

core/src/test/java/org/infinispan/statetransfer/ConcurrentStartTest.java

@@ -18,7 +18,6 @@
 import org.infinispan.configuration.cache.ConfigurationBuilder;
 import org.infinispan.configuration.global.GlobalConfigurationBuilder;
 import org.infinispan.lifecycle.ComponentStatus;
-import org.infinispan.manager.DefaultCacheManager;
 import org.infinispan.manager.EmbeddedCacheManager;
 import org.infinispan.remoting.inboundhandler.DeliverOrder;
 import org.infinispan.remoting.inboundhandler.InboundInvocationHandler;
@@ -117,8 +116,10 @@ private EmbeddedCacheManager createCacheManager(int index) {
       GlobalConfigurationBuilder gcb = new GlobalConfigurationBuilder();
       gcb.transport().defaultTransport();
       TestCacheManagerFactory.amendGlobalConfiguration(gcb, new TransportFlags().withPortRange(index));
-      EmbeddedCacheManager cm = new DefaultCacheManager(gcb.build(), false);
+      ConfigurationBuilder defaultCacheConfig = new ConfigurationBuilder();
+      EmbeddedCacheManager cm = TestCacheManagerFactory.newDefaultCacheManager(false, gcb, defaultCacheConfig, false);
       registerCacheManager(cm);
+
       Configuration replCfg = new ConfigurationBuilder().clustering().cacheMode(CacheMode.REPL_SYNC).build();
       cm.defineConfiguration(REPL_CACHE_NAME, replCfg);
       Configuration distCfg = new ConfigurationBuilder().clustering().cacheMode(CacheMode.DIST_SYNC).build();

core/src/test/java/org/infinispan/test/AbstractInfinispanTest.java

@@ -137,15 +137,7 @@ protected final void testClassFinished(ITestContext context) {
    }
 
    public String getTestName() {
-      // will qualified test name and parameters, thread names can be quite long when debugging
-      boolean shortTestName = Boolean.getBoolean("test.infinispan.shortTestName");
-      String className;
-      if (shortTestName) {
-         className = "Test";
-      } else {
-         className = getClass().getName();
-      }
-
+      String className = getClass().getName();
       String parameters = parameters();
       return parameters == null ? className : className + parameters;
    }

core/src/test/java/org/infinispan/test/MultipleCacheManagersTest.java

@@ -1,6 +1,8 @@
 package org.infinispan.test;
 
 import static java.util.Arrays.asList;
+import static org.infinispan.test.fwk.TestResourceTracker.getCurrentTestShortName;
+import static org.testng.AssertJUnit.assertTrue;
 
 import java.lang.annotation.Annotation;
 import java.lang.reflect.Constructor;
@@ -135,6 +137,13 @@ protected void destroy() {
          TestingUtil.clearContent(cacheManagers);
          TestingUtil.killCacheManagers(cacheManagers);
       }
+
+      for (EmbeddedCacheManager cm : cacheManagers) {
+         String nodeName = cm.getCacheManagerConfiguration().transport().nodeName();
+         assertTrue("Invalid node name for test " + getCurrentTestShortName() + ": " + nodeName,
+                    nodeName != null && nodeName.contains(getCurrentTestShortName()));
+      }
+
       cacheManagers.clear();
       listeners.clear();
    }

core/src/test/java/org/infinispan/test/fwk/TestCacheManagerFactory.java

@@ -6,7 +6,6 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.util.UUID;
-
 import javax.xml.stream.FactoryConfigurationError;
 import javax.xml.stream.XMLStreamException;
 
@@ -20,9 +19,11 @@
 import org.infinispan.configuration.cache.Index;
 import org.infinispan.configuration.global.GlobalConfiguration;
 import org.infinispan.configuration.global.GlobalConfigurationBuilder;
+import org.infinispan.configuration.global.ThreadPoolConfiguration;
 import org.infinispan.configuration.internal.PrivateGlobalConfigurationBuilder;
 import org.infinispan.configuration.parsing.ConfigurationBuilderHolder;
 import org.infinispan.configuration.parsing.ParserRegistry;
+import org.infinispan.factories.threads.DefaultThreadFactory;
 import org.infinispan.commons.jmx.MBeanServerLookup;
 import org.infinispan.commons.jmx.PerThreadMBeanServerLookup;
 import org.infinispan.manager.DefaultCacheManager;
@@ -40,18 +41,13 @@
  * @author Galder Zamarreño
  */
 public class TestCacheManagerFactory {
+   private static final int NAMED_EXECUTORS_THREADS_NO_QUEUE = 6;
+   private static final int NAMED_EXECUTORS_THREADS_WITH_QUEUE = 4;
+   private static final int NAMED_EXECUTORS_QUEUE_SIZE = 10;
+   private static final int NAMED_EXECUTORS_KEEP_ALIVE = 30000;
 
-   public static final int ASYNC_EXEC_MAX_THREADS = 4;
-   public static final int ASYNC_EXEC_QUEUE_SIZE = 10000;
-   public static final int REMOTE_EXEC_MAX_THREADS = 6;
-   public static final int REMOTE_EXEC_QUEUE_SIZE = 0;
-   public static final int STATE_TRANSFER_EXEC_MAX_THREADS = 4;
-   public static final int STATE_TRANSFER_EXEC_QUEUE_SIZE = 0;
-   public static final int TRANSPORT_EXEC_MAX_THREADS = 6;
-   public static final int TRANSPORT_EXEC_QUEUE_SIZE = 10000;
-   public static final int KEEP_ALIVE = 30000;
-
-   public static final String MARSHALLER = LegacyKeySupportSystemProperties.getProperty("infinispan.test.marshaller.class", "infinispan.marshaller.class");
+   private static final String MARSHALLER = LegacyKeySupportSystemProperties.getProperty(
+      "infinispan.test.marshaller.class", "infinispan.marshaller.class");
    private static final Log log = LogFactory.getLog(TestCacheManagerFactory.class);
 
    /**
@@ -117,9 +113,26 @@ public static EmbeddedCacheManager fromStream(InputStream is, boolean keepJmxDom
    public static EmbeddedCacheManager fromStream(InputStream is, boolean keepJmxDomainName, boolean defaultParsersOnly, boolean start) throws IOException {
       ParserRegistry parserRegistry = new ParserRegistry(Thread.currentThread().getContextClassLoader(), defaultParsersOnly);
       ConfigurationBuilderHolder holder = parserRegistry.parse(is);
+
+      // The node name is set in each DefaultThreadFactory individually, override it here
+      String testShortName = TestResourceTracker.getCurrentTestShortName();
+      GlobalConfiguration gc = holder.getGlobalConfigurationBuilder().build();
+      updateNodeName(testShortName, gc.listenerThreadPool());
+      updateNodeName(testShortName, gc.expirationThreadPool());
+      updateNodeName(testShortName, gc.persistenceThreadPool());
+      updateNodeName(testShortName, gc.stateTransferThreadPool());
+      updateNodeName(testShortName, gc.asyncThreadPool());
+      updateNodeName(testShortName, gc.transport().transportThreadPool());
+
       return createClusteredCacheManager(start, holder, keepJmxDomainName);
    }
 
+   private static void updateNodeName(String nodeName, ThreadPoolConfiguration threadPoolConfiguration) {
+      if (threadPoolConfiguration.threadFactory() instanceof DefaultThreadFactory) {
+         ((DefaultThreadFactory) threadPoolConfiguration.threadFactory()).setNode(nodeName);
+      }
+   }
+
    public static EmbeddedCacheManager fromString(String config) throws IOException {
       return fromStream(new ByteArrayInputStream(config.getBytes()));
    }
@@ -247,6 +260,7 @@ public static EmbeddedCacheManager createCacheManager(GlobalConfigurationBuilder
       if (globalBuilder.transport().build().transport().transport() != null) {
          throw new IllegalArgumentException("Use TestCacheManagerFactory.createClusteredCacheManager(...) for clustered cache managers");
       }
+      amendTransport(globalBuilder);
       return newDefaultCacheManager(true, globalBuilder, builder, false);
    }
 
@@ -360,40 +374,39 @@ public static void amendTransport(GlobalConfigurationBuilder cfg) {
    }
 
    private static void amendTransport(GlobalConfigurationBuilder builder, TransportFlags flags) {
-      org.infinispan.configuration.global.GlobalConfiguration gc = builder.build();
-      if (gc.transport().transport() != null) { //this is local
-         String testName = TestResourceTracker.getCurrentTestName();
-         String nextNodeName = TestResourceTracker.getNextNodeName();
+      String testName = TestResourceTracker.getCurrentTestName();
 
+      GlobalConfiguration gc = builder.build();
+      if (gc.transport().transport() != null) {
          // Remove any configuration file that might have been set.
          builder.transport().removeProperty(JGroupsTransport.CONFIGURATION_FILE);
 
-         builder
-               .transport()
-               .addProperty(JGroupsTransport.CONFIGURATION_STRING, getJGroupsConfig(testName, flags))
-               .nodeName(nextNodeName);
+         builder.transport().addProperty(JGroupsTransport.CONFIGURATION_STRING, getJGroupsConfig(testName, flags));
       }
    }
 
-   public static void minimizeThreads(GlobalConfigurationBuilder builder) {
-      BlockingThreadPoolExecutorFactory executorFactory;
-
-      executorFactory = new BlockingThreadPoolExecutorFactory(ASYNC_EXEC_MAX_THREADS,
-            ASYNC_EXEC_MAX_THREADS, ASYNC_EXEC_QUEUE_SIZE, KEEP_ALIVE);
-      builder.asyncThreadPool().threadPoolFactory(executorFactory);
+   public static void setNodeName(GlobalConfigurationBuilder builder) {
+      String nextNodeName = TestResourceTracker.getNextNodeName();
 
-      executorFactory = new BlockingThreadPoolExecutorFactory(STATE_TRANSFER_EXEC_MAX_THREADS,
-            STATE_TRANSFER_EXEC_MAX_THREADS, STATE_TRANSFER_EXEC_QUEUE_SIZE, KEEP_ALIVE);
-      builder.stateTransferThreadPool().threadPoolFactory(executorFactory);
+      // Set the node name even for local managers in order to set the name of the worker threads
+      builder.transport().nodeName(nextNodeName);
+   }
 
-      executorFactory = new BlockingThreadPoolExecutorFactory(REMOTE_EXEC_MAX_THREADS,
-            REMOTE_EXEC_MAX_THREADS, REMOTE_EXEC_QUEUE_SIZE, KEEP_ALIVE);
-      builder.transport().remoteCommandThreadPool().threadPoolFactory(executorFactory);
+   public static void minimizeThreads(GlobalConfigurationBuilder builder) {
+      BlockingThreadPoolExecutorFactory executorFactoryNoQueue =
+         new BlockingThreadPoolExecutorFactory(NAMED_EXECUTORS_THREADS_NO_QUEUE, 0, 0, NAMED_EXECUTORS_KEEP_ALIVE);
+      BlockingThreadPoolExecutorFactory executorFactoryWithQueue =
+         new BlockingThreadPoolExecutorFactory(NAMED_EXECUTORS_THREADS_WITH_QUEUE, NAMED_EXECUTORS_THREADS_WITH_QUEUE,
+                                               NAMED_EXECUTORS_QUEUE_SIZE, NAMED_EXECUTORS_KEEP_ALIVE);
 
-      executorFactory = new BlockingThreadPoolExecutorFactory(
-            TRANSPORT_EXEC_MAX_THREADS, TRANSPORT_EXEC_MAX_THREADS, TRANSPORT_EXEC_QUEUE_SIZE, KEEP_ALIVE);
-      builder.transport().transportThreadPool().threadPoolFactory(executorFactory);
+      builder.transport().remoteCommandThreadPool().threadPoolFactory(executorFactoryNoQueue);
+      builder.stateTransferThreadPool().threadPoolFactory(executorFactoryNoQueue);
 
+      builder.asyncThreadPool().threadPoolFactory(executorFactoryWithQueue);
+      builder.listenerThreadPool().threadPoolFactory(executorFactoryWithQueue);
+      builder.transport().transportThreadPool().threadPoolFactory(executorFactoryWithQueue);
+      // TODO Scheduled thread pools don't have a threads limit
+      // Timeout thread pool is not configurable at all
    }
 
    public static void amendMarshaller(GlobalConfigurationBuilder builder) {
@@ -408,14 +421,15 @@ public static void amendMarshaller(GlobalConfigurationBuilder builder) {
    }
 
    private static DefaultCacheManager newDefaultCacheManager(boolean start, GlobalConfigurationBuilder gc, ConfigurationBuilder c) {
-      gc.transport().nodeName(TestResourceTracker.getNextNodeName());
+      setNodeName(gc);
       GlobalConfiguration globalConfiguration = gc.build();
       DefaultCacheManager defaultCacheManager = new DefaultCacheManager(globalConfiguration, c.build(globalConfiguration), start);
       TestResourceTracker.addResource(new TestResourceTracker.CacheManagerCleaner(defaultCacheManager));
       return defaultCacheManager;
    }
 
    private static DefaultCacheManager newDefaultCacheManager(boolean start, ConfigurationBuilderHolder holder) {
+      setNodeName(holder.getGlobalConfigurationBuilder());
       DefaultCacheManager defaultCacheManager = new DefaultCacheManager(holder, start);
       TestResourceTracker.addResource(new TestResourceTracker.CacheManagerCleaner(defaultCacheManager));
       return defaultCacheManager;