QPID-4281: Fix setting of log4j configuration in system tests. While testing the fix, some tests were found to be failing on spawned profile due to config problems - also fixed these.

Applied patch from Philip Harvey <phil@philharveyonline.com> and myself.

git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk/qpid@1438053 13f79535-47bb-0310-9956-ffa450edef68

11 files changed, 302 insertions, 73 deletions

diff --git a/java/broker/src/test/java/org/apache/qpid/server/util/TestApplicationRegistry.java b/java/broker/src/test/java/org/apache/qpid/server/util/TestApplicationRegistry.java
index 600f0d591d..fd3791a8e1 100644
--- a/java/broker/src/test/java/org/apache/qpid/server/util/TestApplicationRegistry.java
+++ b/java/broker/src/test/java/org/apache/qpid/server/util/TestApplicationRegistry.java
@@ -38,6 +38,7 @@ import org.apache.qpid.server.security.auth.manager.AuthenticationManager;
 import org.apache.qpid.server.security.auth.manager.IAuthenticationManagerRegistry;
 import org.apache.qpid.server.security.auth.manager.PrincipalDatabaseAuthenticationManager;
 import org.apache.qpid.server.security.group.GroupPrincipalAccessor;
+import org.apache.qpid.test.utils.QpidTestCase;
 
 import java.util.Properties;
 
@@ -54,7 +55,7 @@ public class TestApplicationRegistry extends ApplicationRegistry
     {
         CurrentActor.setDefault(new BrokerActor(new NullRootMessageLogger()));
         GenericActor.setDefaultMessageLogger(new NullRootMessageLogger());
-        LoggingManagementFacade.configure("test-profiles/log4j-test.xml");
+        LoggingManagementFacade.configure(QpidTestCase.LOG4J_CONFIG_FILE_PATH);
 
         super.initialise();
     }
diff --git a/java/common/src/test/java/org/apache/qpid/test/utils/QpidTestCase.java b/java/common/src/test/java/org/apache/qpid/test/utils/QpidTestCase.java
index ec06400b7d..c16e42eb7c 100644
--- a/java/common/src/test/java/org/apache/qpid/test/utils/QpidTestCase.java
+++ b/java/common/src/test/java/org/apache/qpid/test/utils/QpidTestCase.java
@@ -41,6 +41,7 @@ public class QpidTestCase extends TestCase
     public static final String QPID_HOME = System.getProperty("QPID_HOME");
     public static final String TEST_RESOURCES_DIR = QPID_HOME + "/../test-profiles/test_resources/";
     public static final String TMP_FOLDER = System.getProperty("java.io.tmpdir");
+    public static final String LOG4J_CONFIG_FILE_PATH = System.getProperty("log4j.configuration.file");
 
     private static final Logger _logger = Logger.getLogger(QpidTestCase.class);
 
@@ -204,6 +205,8 @@ public class QpidTestCase extends TestCase
         {
             System.setProperty(property, value);
         }
+
+        _logger.info("Set system property \"" + property + "\" to: \"" + value + "\"");
     }
 
     /**
diff --git a/java/module.xml b/java/module.xml
index 997ce944f4..00c6d73959 100644
--- a/java/module.xml
+++ b/java/module.xml
@@ -398,6 +398,7 @@
 
     <propertyref name="log4j.debug"/>
     <propertyref name="log4j.configuration"/>
+    <propertyref name="log4j.configuration.file"/>
 
     <propertyref name="root.logging.level"/>
     <propertyref name="java.naming.factory.initial"/>
diff --git a/java/systests/src/main/java/org/apache/qpid/server/BrokerStartupTest.java b/java/systests/src/main/java/org/apache/qpid/server/BrokerStartupTest.java
index 9f3994fc91..eba2a638c0 100644
--- a/java/systests/src/main/java/org/apache/qpid/server/BrokerStartupTest.java
+++ b/java/systests/src/main/java/org/apache/qpid/server/BrokerStartupTest.java
@@ -30,6 +30,7 @@ import org.apache.qpid.util.LogMonitor;
 import javax.jms.Connection;
 import javax.jms.Queue;
 import javax.jms.Session;
+import java.io.File;
 import java.util.List;
 
 /**
@@ -72,10 +73,7 @@ public class BrokerStartupTest extends AbstractTestLogging
         if (isJavaBroker() && isExternalBroker() && !isInternalBroker())
         {
             //Remove test Log4j config from the commandline
-            _brokerCommand = _brokerCommand.substring(0, _brokerCommand.indexOf("-l"));
-
-            // Add an invalid value
-            _brokerCommand += " -l invalid";
+            setBrokerCommandLog4JFile(new File("invalid file"));
 
             // The  broker has a built in default log4j configuration set up
             // so if the the broker cannot load the -l value it will use default
@@ -145,4 +143,4 @@ public class BrokerStartupTest extends AbstractTestLogging
         }
     }
 
-}
\ No newline at end of file
+}
diff --git a/java/systests/src/main/java/org/apache/qpid/server/logging/BrokerLoggingTest.java b/java/systests/src/main/java/org/apache/qpid/server/logging/BrokerLoggingTest.java
index dee593b12b..24c0aed611 100644
--- a/java/systests/src/main/java/org/apache/qpid/server/logging/BrokerLoggingTest.java
+++ b/java/systests/src/main/java/org/apache/qpid/server/logging/BrokerLoggingTest.java
@@ -155,8 +155,7 @@ public class BrokerLoggingTest extends AbstractTestLogging
         {
             String TESTID = "BRK-1007";
 
-            //Remove test Log4j config from the commandline
-            _brokerCommand = _brokerCommand.substring(0, _brokerCommand.indexOf("-l"));
+            _brokerCommandHelper.removeBrokerCommandLog4JFile();
 
             startBroker();
 
@@ -243,9 +242,6 @@ public class BrokerLoggingTest extends AbstractTestLogging
         // This logging startup code only occurs when you run a Java broker
         if (isJavaBroker() && isExternalBroker())
         {
-            // Get custom -l value used during testing for the broker startup
-            String customLog4j = _brokerCommand.substring(_brokerCommand.indexOf("-l") + 2).trim();
-
             String TESTID = "BRK-1007";
 
             startBroker();
@@ -293,8 +289,11 @@ public class BrokerLoggingTest extends AbstractTestLogging
                                  1, findMatches(TESTID).size());
 
                     //3
-                    assertTrue("Log4j file details not correctly logged:" + getMessageString(log),
-                               getMessageString(log).endsWith(customLog4j));
+                    String messageString = getMessageString(log);
+                    String customLog4j = getBrokerCommandLog4JFile().getAbsolutePath();
+                    assertTrue("Log4j file details not correctly logged. Message '" + messageString
+                             + "' should contain '" + customLog4j + "'",
+                               messageString.contains(customLog4j));
 
                     validation = true;
                 }
diff --git a/java/systests/src/main/java/org/apache/qpid/systest/management/jmx/LoggingManagementTest.java b/java/systests/src/main/java/org/apache/qpid/systest/management/jmx/LoggingManagementTest.java
index 2a112eab35..3c3bbdca41 100644
--- a/java/systests/src/main/java/org/apache/qpid/systest/management/jmx/LoggingManagementTest.java
+++ b/java/systests/src/main/java/org/apache/qpid/systest/management/jmx/LoggingManagementTest.java
@@ -55,9 +55,8 @@ public class LoggingManagementTest extends QpidBrokerTestCase
 
         File tmpLogFile = File.createTempFile("log4j" + "." + getName(), ".xml");
         tmpLogFile.deleteOnExit();
-        FileUtils.copy(_logConfigFile, tmpLogFile);
-
-        _logConfigFile = tmpLogFile;
+        FileUtils.copy(getBrokerCommandLog4JFile(), tmpLogFile);
+        setBrokerCommandLog4JFile(tmpLogFile);
 
         super.setUp();
         _jmxUtils.open();
@@ -105,7 +104,7 @@ public class LoggingManagementTest extends QpidBrokerTestCase
         _monitor.markDiscardPoint();
         _loggingManagement.setRuntimeLoggerLevel(logger, "OFF");
 
-        List<String> matches = _monitor.findMatches("Setting level to OFF for logger 'org.apache.qpid'");
+        List<String> matches = _monitor.waitAndFindMatches("Setting level to OFF for logger 'org.apache.qpid'", 5000);
         assertEquals(1, matches.size());
 
         TabularData table = _loggingManagement.viewEffectiveRuntimeLoggerLevels();
@@ -121,7 +120,7 @@ public class LoggingManagementTest extends QpidBrokerTestCase
         _monitor.markDiscardPoint();
         _loggingManagement.setConfigFileLoggerLevel(operationalLoggingLogger, "OFF");
 
-        List<String> matches = _monitor.findMatches("Setting level to OFF for logger 'qpid.message'");
+        List<String> matches = _monitor.waitAndFindMatches("Setting level to OFF for logger 'qpid.message'", 5000);
         assertEquals(1, matches.size());
 
         assertEffectiveLoggingLevel(operationalLoggingLogger, "INFO");
diff --git a/java/systests/src/main/java/org/apache/qpid/test/utils/BrokerCommandHelper.java b/java/systests/src/main/java/org/apache/qpid/test/utils/BrokerCommandHelper.java
new file mode 100644
index 0000000000..4d5718f014
--- /dev/null
+++ b/java/systests/src/main/java/org/apache/qpid/test/utils/BrokerCommandHelper.java
@@ -0,0 +1,107 @@
+/* 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.qpid.test.utils;
+
+import org.apache.commons.lang.StringUtils;
+import org.apache.log4j.Logger;
+
+import java.io.File;
+import java.util.Arrays;
+import java.util.ArrayList;
+import java.util.LinkedList;
+import java.util.List;
+
+/**
+ * Generates the command to start a broker by substituting the tokens
+ * in the provided broker command.
+ *
+ * The command is returned as a list so that it can be easily used by a
+ * {@link java.lang.ProcessBuilder}.
+ */
+public class BrokerCommandHelper
+{
+    private static final Logger _logger = Logger.getLogger(BrokerCommandHelper.class);
+
+    /** use a LinkedList so we can call remove(int) */
+    private final LinkedList<String> _brokerCommandTemplateAsList;
+
+    public BrokerCommandHelper(String brokerCommandTemplate)
+    {
+        _brokerCommandTemplateAsList = new LinkedList(Arrays.asList(brokerCommandTemplate.split("\\s+")));
+    }
+
+    public List<String> getBrokerCommand(
+        int port, int sslPort, int managementPort,
+        File configFile, File logConfigFile,
+        String protocolIncludesList, String protocolExcludesList)
+    {
+        List<String> substitutedCommands = new ArrayList<String>();
+
+        // Replace the tokens in two passes.
+        // The first pass is for protocol include/exclude lists, each of which needs to be
+        // split into separate entries in the final list.
+        // The second pass is for the remaining options, which include file paths that
+        // are quoted in case they contain whitespace.
+
+        for (String commandPart : _brokerCommandTemplateAsList)
+        {
+            String substitutedCommandPart = commandPart
+                    .replace("@EXCLUDES", protocolExcludesList)
+                    .replace("@INCLUDES", protocolIncludesList);
+
+            String[] splitCommandPart = StringUtils.split(substitutedCommandPart);
+            substitutedCommands.addAll(Arrays.asList(splitCommandPart));
+        }
+
+        int i = 0;
+        for (String commandPart : substitutedCommands)
+        {
+            String substitutedCommandPart = commandPart
+                    .replace("@PORT", "" + port)
+                    .replace("@SSL_PORT", "" + sslPort)
+                    .replace("@MPORT", "" + managementPort)
+                    .replace("@CONFIG_FILE", '"' + configFile.getPath() + '"')
+                    .replace("@LOG_CONFIG_FILE", '"' + logConfigFile.getAbsolutePath() + '"');
+
+            substitutedCommands.set(i, substitutedCommandPart);
+            i++;
+        }
+
+        return substitutedCommands;
+    }
+
+    private int getBrokerCommandLogOptionIndex()
+    {
+        String logOption = "-l";
+
+        int logOptionIndex = _brokerCommandTemplateAsList.indexOf(logOption);
+        if(logOptionIndex == -1)
+        {
+            throw new RuntimeException("Could not find option " + logOption + " in " + _brokerCommandTemplateAsList);
+        }
+        return logOptionIndex;
+    }
+
+    public void removeBrokerCommandLog4JFile()
+    {
+        int logOptionIndex = getBrokerCommandLogOptionIndex();
+        _brokerCommandTemplateAsList.remove(logOptionIndex); // removes the "-l" entry
+        _brokerCommandTemplateAsList.remove(logOptionIndex); // removes log file name that followed the -l
+        _logger.info("Removed broker command log4j config file");
+    }
+}
diff --git a/java/systests/src/main/java/org/apache/qpid/test/utils/BrokerCommandHelperTest.java b/java/systests/src/main/java/org/apache/qpid/test/utils/BrokerCommandHelperTest.java
new file mode 100644
index 0000000000..332059dbc0
--- /dev/null
+++ b/java/systests/src/main/java/org/apache/qpid/test/utils/BrokerCommandHelperTest.java
@@ -0,0 +1,79 @@
+/* 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.qpid.test.utils;
+
+import static org.mockito.Mockito.*;
+
+import org.apache.commons.lang.StringUtils;
+import org.apache.log4j.Logger;
+
+import java.io.File;
+import java.util.Arrays;
+import java.util.List;
+
+public class BrokerCommandHelperTest extends QpidTestCase
+{
+    private BrokerCommandHelper _brokerCommandHelper = new BrokerCommandHelper("qpid -p @PORT -s @SSL_PORT -m @MPORT @INCLUDES @EXCLUDES -c @CONFIG_FILE -l @LOG_CONFIG_FILE");
+
+    private File logConfigFile = mock(File.class);
+    private File configFile = mock(File.class);
+
+    @Override
+    public void setUp()
+    {
+        when(logConfigFile.getAbsolutePath()).thenReturn("logConfigFile");
+        when(configFile.getPath()).thenReturn("configFile");
+    }
+
+    public void testGetBrokerCommand()
+    {
+        List<String> brokerCommand = _brokerCommandHelper.getBrokerCommand(
+            1, 2, 3, configFile, logConfigFile, "includes", "excludes");
+
+        assertEquals(Arrays.asList("qpid", "-p", "1", "-s", "2", "-m", "3",
+                                   "includes", "excludes",
+                                   "-c", "\"configFile\"", "-l", "\"logConfigFile\""),
+                     brokerCommand);
+    }
+
+    public void testGetBrokerCommandForMultipleProtocolIncludesAndExcludes()
+    {
+        List<String> brokerCommand = _brokerCommandHelper.getBrokerCommand(
+            1, 2, 3, configFile, logConfigFile, "includes1 includes2", "excludes1 excludes2");
+
+        assertEquals("The space-separated protocol include/exclude lists should be converted into separate entries in the returned list so that ProcessBuilder treats them as separate arguments",
+                     Arrays.asList("qpid", "-p", "1", "-s", "2", "-m", "3",
+                                   "includes1", "includes2",  "excludes1", "excludes2",
+                                   "-c", "\"configFile\"", "-l", "\"logConfigFile\""),
+                     brokerCommand);
+    }
+
+    public void testRemoveBrokerCommandLog4JFile()
+    {
+        _brokerCommandHelper.removeBrokerCommandLog4JFile();
+
+        List<String> brokerCommand = _brokerCommandHelper.getBrokerCommand(
+            1, 2, 3, configFile, logConfigFile, "includes", "excludes");
+
+        assertEquals("The broker command list should not contain a log4j config option",
+                     Arrays.asList("qpid", "-p", "1", "-s", "2", "-m", "3",
+                                   "includes", "excludes",
+                                   "-c", "\"configFile\""),
+                     brokerCommand);
+    }
+}
diff --git a/java/systests/src/main/java/org/apache/qpid/test/utils/QpidBrokerTestCase.java b/java/systests/src/main/java/org/apache/qpid/test/utils/QpidBrokerTestCase.java
index b9f3bd7c23..bad1551c59 100644..100755
--- a/java/systests/src/main/java/org/apache/qpid/test/utils/QpidBrokerTestCase.java
+++ b/java/systests/src/main/java/org/apache/qpid/test/utils/QpidBrokerTestCase.java
@@ -21,7 +21,10 @@ import java.io.File;
 import java.io.FileOutputStream;
 import java.io.PrintStream;
 import java.net.MalformedURLException;
+import java.net.URI;
+import java.net.URISyntaxException;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
@@ -43,6 +46,7 @@ import javax.jms.Session;
 import javax.jms.StreamMessage;
 import javax.jms.TextMessage;
 import javax.jms.Topic;
+import javax.naming.Context;
 import javax.naming.InitialContext;
 import javax.naming.NamingException;
 
@@ -76,7 +80,7 @@ public class QpidBrokerTestCase extends QpidTestCase
     public enum BrokerType
     {
         EXTERNAL /** Test case relies on a Broker started independently of the test-suite */,
-        INTERNAL /** Test case starts an embedded broker within this JVM */, 
+        INTERNAL /** Test case starts an embedded broker within this JVM */,
         SPAWNED /** Test case spawns a new broker as a separate process */
     }
 
@@ -85,7 +89,7 @@ public class QpidBrokerTestCase extends QpidTestCase
 
     protected final static String QpidHome = System.getProperty("QPID_HOME");
     protected File _configFile = new File(System.getProperty("broker.config"));
-    protected File _logConfigFile = new File(System.getProperty("log4j.configuration"));
+    private File _logConfigFile;
 
     protected static final Logger _logger = Logger.getLogger(QpidBrokerTestCase.class);
     protected static final int LOGMONITOR_TIMEOUT = 5000;
@@ -104,11 +108,11 @@ public class QpidBrokerTestCase extends QpidTestCase
 
     static
     {
-        String initialContext = System.getProperty(InitialContext.INITIAL_CONTEXT_FACTORY);
+        String initialContext = System.getProperty(Context.INITIAL_CONTEXT_FACTORY);
 
         if (initialContext == null || initialContext.length() == 0)
         {
-            System.setProperty(InitialContext.INITIAL_CONTEXT_FACTORY, DEFAULT_INITIAL_CONTEXT);
+            System.setProperty(Context.INITIAL_CONTEXT_FACTORY, DEFAULT_INITIAL_CONTEXT);
         }
     }
 
@@ -145,7 +149,8 @@ public class QpidBrokerTestCase extends QpidTestCase
 
     protected String _brokerLanguage = System.getProperty(BROKER_LANGUAGE, JAVA);
     protected BrokerType _brokerType = BrokerType.valueOf(System.getProperty(BROKER_TYPE, "").toUpperCase());
-    protected String _brokerCommand = System.getProperty(BROKER_COMMAND);
+
+    protected BrokerCommandHelper _brokerCommandHelper = new BrokerCommandHelper(System.getProperty(BROKER_COMMAND));
     private Boolean _brokerCleanBetweenTests = Boolean.getBoolean(BROKER_CLEAN_BETWEEN_TESTS);
     private final AmqpProtocolVersion _brokerVersion = AmqpProtocolVersion.valueOf(System.getProperty(BROKER_VERSION, ""));
     protected String _output = System.getProperty(TEST_OUTPUT, System.getProperty("java.io.tmpdir"));
@@ -190,11 +195,32 @@ public class QpidBrokerTestCase extends QpidTestCase
     public QpidBrokerTestCase(String name)
     {
         super(name);
+        initialiseLogConfigFile();
     }
-    
+
     public QpidBrokerTestCase()
     {
         super();
+        initialiseLogConfigFile();
+    }
+
+    private void initialiseLogConfigFile()
+    {
+        try
+        {
+            _logger.info("About to initialise log config file from system property: " + LOG4J_CONFIG_FILE_PATH);
+
+            URI uri = new URI("file", LOG4J_CONFIG_FILE_PATH, null);
+            _logConfigFile = new File(uri);
+            if(!_logConfigFile.exists())
+            {
+                throw new RuntimeException("Log config file " + _logConfigFile.getAbsolutePath() + " does not exist");
+            }
+        }
+        catch (URISyntaxException e)
+        {
+            throw new RuntimeException("Couldn't create URI from log4.configuration: " + LOG4J_CONFIG_FILE_PATH, e);
+        }
     }
 
     public Logger getLogger()
@@ -351,38 +377,43 @@ public class QpidBrokerTestCase extends QpidTestCase
         }
     }
 
-    protected String getBrokerCommand(int port) throws MalformedURLException
+    public void startBroker() throws Exception
+    {
+        startBroker(0);
+    }
+
+    public void startBroker(int port) throws Exception
+    {
+        startBroker(port, _testConfiguration, _testVirtualhosts);
+    }
+
+    private List<String> getBrokerCommand(int port)
     {
         final int sslPort = port-1;
         final String protocolExcludesList = getProtocolExcludesList(port, sslPort);
         final String protocolIncludesList = getProtocolIncludesList(port, sslPort);
-
-        return _brokerCommand
-                .replace("@PORT", "" + port)
-                .replace("@SSL_PORT", "" + sslPort)
-                .replace("@MPORT", "" + getManagementPort(port))
-                .replace("@CONFIG_FILE", _configFile.toString())
-                .replace("@LOG_CONFIG_FILE", _logConfigFile.toString())
-                .replace("@EXCLUDES", protocolExcludesList)
-                .replace("@INCLUDES", protocolIncludesList);
+        return _brokerCommandHelper.getBrokerCommand(
+            port, sslPort, getManagementPort(port),
+            _configFile, _logConfigFile,
+            protocolIncludesList, protocolExcludesList);
     }
 
-    public void startBroker() throws Exception
+    protected File getBrokerCommandLog4JFile()
     {
-        startBroker(0);
+        return _logConfigFile;
     }
 
-    public void startBroker(int port) throws Exception
+    protected void setBrokerCommandLog4JFile(File file)
     {
-        startBroker(port, _testConfiguration, _testVirtualhosts);
+        _logConfigFile = file;
+        _logger.info("Modified log config file to: " + file);
     }
 
     public void startBroker(int port, XMLConfiguration testConfiguration, XMLConfiguration virtualHosts) throws Exception
     {
         port = getPort(port);
 
-        // Save any configuration changes that have been made
-        String testConfig = saveTestConfiguration(port, testConfiguration);
+        // Save only virtual host configuration changes that have been made
         String virtualHostsConfig = saveTestVirtualhosts(port, virtualHosts);
 
         if(_brokers.get(port) != null)
@@ -394,10 +425,8 @@ public class QpidBrokerTestCase extends QpidTestCase
 
         if (_brokerType.equals(BrokerType.INTERNAL) && !existingInternalBroker())
         {
-            setConfigurationProperty(ServerConfiguration.MGMT_CUSTOM_REGISTRY_SOCKET, String.valueOf(false));
-            testConfig = saveTestConfiguration(port, testConfiguration);
-            _logger.info("Set test.config property to: " + testConfig);
-            _logger.info("Set test.virtualhosts property to: " + virtualHostsConfig);
+            testConfiguration.setProperty(ServerConfiguration.MGMT_CUSTOM_REGISTRY_SOCKET, String.valueOf(false));
+            String testConfig = saveTestConfiguration(port, testConfiguration);
             setSystemProperty(TEST_CONFIG, testConfig);
             setSystemProperty(TEST_VIRTUALHOSTS, virtualHostsConfig);
 
@@ -415,7 +444,7 @@ public class QpidBrokerTestCase extends QpidTestCase
             options.setLogConfigFile(_logConfigFile.getAbsolutePath());
 
             Broker broker = new Broker();
-            _logger.info("starting internal broker (same JVM)");
+            _logger.info("Starting internal broker (same JVM)");
             broker.startup(options);
 
             _brokers.put(port, new InternalBrokerHolder(broker, System.getProperty("QPID_WORK"), portsUsedByBroker));
@@ -424,9 +453,9 @@ public class QpidBrokerTestCase extends QpidTestCase
         {
             // Add the port to QPID_WORK to ensure unique working dirs for multi broker tests
             final String qpidWork = getQpidWork(_brokerType, port);
-            String cmd = getBrokerCommand(port);
-            _logger.info("starting external broker: " + cmd);
-            ProcessBuilder pb = new ProcessBuilder(cmd.split("\\s+"));
+            List<String> cmd = getBrokerCommand(port);
+            _logger.info("Starting external broker using command: " + StringUtils.join(cmd, " "));
+            ProcessBuilder pb = new ProcessBuilder(cmd);
             pb.redirectErrorStream(true);
             Map<String, String> processEnv = pb.environment();
             String qpidHome = System.getProperty(QPID_HOME);
@@ -456,28 +485,37 @@ public class QpidBrokerTestCase extends QpidTestCase
                 }
             }
 
+            String testConfig = saveTestConfiguration(port, testConfiguration);
+            String qpidOpts = "";
 
-            // Add default test logging levels that are used by the log4j-test
-            // Use the convenience methods to push the current logging setting
-            // in to the external broker's QPID_OPTS string.
-            if (System.getProperty("amqj.protocol.logging.level") != null)
-            {
-                setSystemProperty("amqj.protocol.logging.level");
-            }
-            if (System.getProperty("root.logging.level") != null)
+            // a synchronized hack to avoid adding into QPID_OPTS the values
+            // of JVM properties "test.virtualhosts" and "test.config" set by a concurrent startup process
+            synchronized (_propertiesSetForBroker)
             {
-                setSystemProperty("root.logging.level");
-            }
+                // Add default test logging levels that are used by the log4j-test
+                // Use the convenience methods to push the current logging setting
+                // in to the external broker's QPID_OPTS string.
+                if (System.getProperty("amqj.protocol.logging.level") != null)
+                {
+                    setSystemProperty("amqj.protocol.logging.level");
+                }
+                if (System.getProperty("root.logging.level") != null)
+                {
+                    setSystemProperty("root.logging.level");
+                }
 
-            // set test.config and test.virtualhosts
-            String qpidOpts = " -D" + TEST_CONFIG + "=" + testConfig + " -D" + TEST_VIRTUALHOSTS + "=" + virtualHostsConfig;
+                setSystemProperty(TEST_CONFIG, testConfig);
+                setSystemProperty(TEST_VIRTUALHOSTS, virtualHostsConfig);
+                // set test.config and test.virtualhosts
+                qpidOpts += " -D" + TEST_CONFIG + "=" + testConfig + " -D" + TEST_VIRTUALHOSTS + "=" + virtualHostsConfig;
 
-            // Add all the specified system properties to QPID_OPTS
-            if (!_propertiesSetForBroker.isEmpty())
-            {
-                for (String key : _propertiesSetForBroker.keySet())
+                // Add all the specified system properties to QPID_OPTS
+                if (!_propertiesSetForBroker.isEmpty())
                 {
-                    qpidOpts += " -D" + key + "=" + _propertiesSetForBroker.get(key);
+                    for (String key : _propertiesSetForBroker.keySet())
+                    {
+                        qpidOpts += " -D" + key + "=" + _propertiesSetForBroker.get(key);
+                    }
                 }
             }
             if (processEnv.containsKey("QPID_OPTS"))
@@ -486,9 +524,6 @@ public class QpidBrokerTestCase extends QpidTestCase
             }
             processEnv.put("QPID_OPTS", qpidOpts);
 
-            _logger.info("Set test.config property to: " + testConfig);
-            _logger.info("Set test.virtualhosts property to: " + virtualHostsConfig);
-
             // cpp broker requires that the work directory is created
             createBrokerWork(qpidWork);
 
@@ -633,7 +668,6 @@ public class QpidBrokerTestCase extends QpidTestCase
     protected void saveTestConfiguration() throws ConfigurationException
     {
         String relative = saveTestConfiguration(getPort(), _testConfiguration);
-        _logger.info("Set test.config property to: " + relative);
         setSystemProperty(TEST_CONFIG, relative);
     }
 
@@ -657,7 +691,6 @@ public class QpidBrokerTestCase extends QpidTestCase
     protected void saveTestVirtualhosts() throws ConfigurationException
     {
         String relative = saveTestVirtualhosts(getPort(), _testVirtualhosts);
-        _logger.info("Set test.virtualhosts property to: " + relative);
         setSystemProperty(TEST_VIRTUALHOSTS, relative);
     }
 
@@ -667,7 +700,7 @@ public class QpidBrokerTestCase extends QpidTestCase
         String testVirtualhosts = getTestVirtualhostsFile(port);
         String relative = relativeToQpidHome(testVirtualhosts);
 
-        _logger.info("Set test.virtualhosts property to: " + testVirtualhosts);
+        _logger.info("Path to virtualhosts configuration: " + testVirtualhosts);
 
         // Create the file if configuration does not exist
         if (virtualHostConfiguration.isEmpty())
@@ -933,7 +966,7 @@ public class QpidBrokerTestCase extends QpidTestCase
      *
      * When the test run is complete the value will be reverted.
      *
-     * The values set using this method will also be propogated to the external
+     * The values set using this method will also be propagated to the external
      * Java Broker via a -D value defined in QPID_OPTS.
      *
      * If the value should not be set on the broker then use
diff --git a/java/systests/src/main/java/org/apache/qpid/util/LogMonitor.java b/java/systests/src/main/java/org/apache/qpid/util/LogMonitor.java
index 2b99289cd1..d6b8a8400c 100644
--- a/java/systests/src/main/java/org/apache/qpid/util/LogMonitor.java
+++ b/java/systests/src/main/java/org/apache/qpid/util/LogMonitor.java
@@ -42,6 +42,8 @@ import java.util.List;
  */
 public class LogMonitor
 {
+    private static final Logger _logger = Logger.getLogger(LogMonitor.class);
+
     // The file that the log statements will be written to.
     private final File _logfile;
 
@@ -90,6 +92,8 @@ public class LogMonitor
             _appender.setImmediateFlush(true);
             Logger.getRootLogger().addAppender(_appender);
         }
+
+        _logger.info("Created LogMonitor. Monitoring file: " + _logfile.getAbsolutePath());
     }
 
     /**
diff --git a/java/test-profiles/testprofile.defaults b/java/test-profiles/testprofile.defaults
index 47992b9334..e205de2a18 100644
--- a/java/test-profiles/testprofile.defaults
+++ b/java/test-profiles/testprofile.defaults
@@ -33,7 +33,12 @@ amqj.logging.level=${log}
 amqj.server.logging.level=${log}
 amqj.protocol.logging.level=${log}
 root.logging.level=warn
-log4j.configuration=test-profiles/log4j-test.xml
+
+# System property log4j.configuration is used by log4j.
+# QpidBrokerTestCase uses log4j.configuration.file to construct a java.io.File, eg for log configuration of spawned brokers.
+log4j.configuration.file=${test.profiles}/log4j-test.xml
+log4j.configuration=file:///${log4j.configuration.file}
+
 log4j.debug=false
 
 # Note test-provider.properties also has variables of same name.