Qpid
  1. Qpid
  2. QPID-4281

Java tests logging broken because log4j.configuration is not a valid URL

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 0.18
    • Fix Version/s: 0.21
    • Component/s: Java Tests
    • Labels:
      None

      Description

      A change was made to the log4j configuration used by our unit tests in QPID-4109, such that the log4j.configuration system property that is effective in our unit tests is "test-profiles/log4j-test.xml" rather than "file:///path/to/log4j-test.xml".

      This breaks our unit test logging configuration because Log4J requires that this system property is a valid URL.

        Activity

        Hide
        Alex Rudyy added a comment -

        Attached a patch fixing the issue

        Show
        Alex Rudyy added a comment - Attached a patch fixing the issue
        Hide
        Alex Rudyy added a comment -

        Phil,
        Could you please review the patch?

        Show
        Alex Rudyy added a comment - Phil, Could you please review the patch?
        Hide
        Philip Harvey added a comment -

        Reviewed patch - looks good. I have observed the full set of tests passing with both spawned and in-process test profiles.

        Show
        Philip Harvey added a comment - Reviewed patch - looks good. I have observed the full set of tests passing with both spawned and in-process test profiles.
        Hide
        Philip Harvey added a comment -

        Keith - please can you commit this patch, thanks.

        Show
        Philip Harvey added a comment - Keith - please can you commit this patch, thanks.
        Hide
        Keith Wall added a comment -

        Hi Phil/Alex

        Tests fail for me if I run within a directory whose parent contains spaces. This is the situation we have on Apache CI. For instance, BrokerLoggingTest:

        Exception in constructor: testBrokerStartupConfiguration (java.lang.RuntimeException: Couldn't URI from log4.conf
        iguration: file:////home/keith/src/qpid wspace/qpid/java/test-profiles/log4j-test.xml
                at org.apache.qpid.test.utils.QpidBrokerTestCase.initialiseLogConfigFile(QpidBrokerTestCase.java:220)
                at org.apache.qpid.test.utils.QpidBrokerTestCase.<init>(QpidBrokerTestCase.java:204)
                at org.apache.qpid.server.logging.AbstractTestLogging.<init>(AbstractTestLogging.java:44)
                at org.apache.qpid.server.logging.BrokerLoggingTest.<init>(BrokerLoggingTest.java:48)
                at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
                at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:39)
                at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:27)
                at java.lang.reflect.Constructor.newInstance(Constructor.java:513)
                at junit.framework.TestSuite.createTest(TestSuite.java:131)
                at junit.framework.TestSuite.addTestMethod(TestSuite.java:114)
                at junit.framework.TestSuite.<init>(TestSuite.java:75)
                at org.apache.tools.ant.taskdefs.optional.junit.JUnitTestRunner.run(JUnitTestRunner.java:398)
                at org.apache.tools.ant.taskdefs.optional.junit.JUnitTestRunner.launch(JUnitTestRunner.java:931)
                at org.apache.tools.ant.taskdefs.optional.junit.JUnitTestRunner.main(JUnitTestRunner.java:785)
        Caused by: java.net.URISyntaxException: Illegal character in path at index 30: file:////home/keith/src/qpid wspace/qpid/java/test-profiles/log4j-test.xml
                at java.net.URI$Parser.fail(URI.java:2809)
                at java.net.URI$Parser.checkChars(URI.java:2982)
                at java.net.URI$Parser.parseHierarchical(URI.java:3066)
                at java.net.URI$Parser.parse(URI.java:3014)
                at java.net.URI.<init>(URI.java:578)
                at org.apache.qpid.test.utils.QpidBrokerTestCase.initialiseLogConfigFile(QpidBrokerTestCase.java:212)
        
        
        Show
        Keith Wall added a comment - Hi Phil/Alex Tests fail for me if I run within a directory whose parent contains spaces. This is the situation we have on Apache CI. For instance, BrokerLoggingTest: Exception in constructor: testBrokerStartupConfiguration (java.lang.RuntimeException: Couldn't URI from log4.conf iguration: file:////home/keith/src/qpid wspace/qpid/java/test-profiles/log4j-test.xml at org.apache.qpid.test.utils.QpidBrokerTestCase.initialiseLogConfigFile(QpidBrokerTestCase.java:220) at org.apache.qpid.test.utils.QpidBrokerTestCase.<init>(QpidBrokerTestCase.java:204) at org.apache.qpid.server.logging.AbstractTestLogging.<init>(AbstractTestLogging.java:44) at org.apache.qpid.server.logging.BrokerLoggingTest.<init>(BrokerLoggingTest.java:48) at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method) at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:39) at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:27) at java.lang.reflect.Constructor.newInstance(Constructor.java:513) at junit.framework.TestSuite.createTest(TestSuite.java:131) at junit.framework.TestSuite.addTestMethod(TestSuite.java:114) at junit.framework.TestSuite.<init>(TestSuite.java:75) at org.apache.tools.ant.taskdefs.optional.junit.JUnitTestRunner.run(JUnitTestRunner.java:398) at org.apache.tools.ant.taskdefs.optional.junit.JUnitTestRunner.launch(JUnitTestRunner.java:931) at org.apache.tools.ant.taskdefs.optional.junit.JUnitTestRunner.main(JUnitTestRunner.java:785) Caused by: java.net.URISyntaxException: Illegal character in path at index 30: file:////home/keith/src/qpid wspace/qpid/java/test-profiles/log4j-test.xml at java.net.URI$Parser.fail(URI.java:2809) at java.net.URI$Parser.checkChars(URI.java:2982) at java.net.URI$Parser.parseHierarchical(URI.java:3066) at java.net.URI$Parser.parse(URI.java:3014) at java.net.URI.<init>(URI.java:578) at org.apache.qpid.test.utils.QpidBrokerTestCase.initialiseLogConfigFile(QpidBrokerTestCase.java:212)
        Hide
        Philip Harvey added a comment -

        Note that there are changes being made in QPID-4335 to TestApplicationRegistry to use a hard-coded log4j.xml path. That change will need to be revisited as part of this JIRA.

        Show
        Philip Harvey added a comment - Note that there are changes being made in QPID-4335 to TestApplicationRegistry to use a hard-coded log4j.xml path. That change will need to be revisited as part of this JIRA.
        Hide
        Philip Harvey added a comment -

        Attached new patch that addresses problems in the previous one when running from a directory containing spaces.

        Dealing with spaces properly required a non-trivial refactoring of the way QpidBrokerTestCase constructs the broker command.

        Show
        Philip Harvey added a comment - Attached new patch that addresses problems in the previous one when running from a directory containing spaces. Dealing with spaces properly required a non-trivial refactoring of the way QpidBrokerTestCase constructs the broker command.
        Hide
        Philip Harvey added a comment -

        Hi Robbie, please can you review the updated patch.

        Show
        Philip Harvey added a comment - Hi Robbie, please can you review the updated patch.
        Hide
        Robbie Gemmell added a comment -

        Marking as RtR, 0.21 fix-for.

        Show
        Robbie Gemmell added a comment - Marking as RtR, 0.21 fix-for.
        Hide
        Alex Rudyy added a comment -

        I am going to review and commit the patch attached.

        Show
        Alex Rudyy added a comment - I am going to review and commit the patch attached.
        Hide
        Alex Rudyy added a comment -

        Phil,
        I committed a modified version of your original patch in revision https://svn.apache.org/repos/asf/qpid/trunk@1438053.

        I had to add a synchronized block into code starting the external broker in order to avoid adding into QPID_OPTS of JVM properties "test.virtualhosts" and "test.config" set by a concurrent start-up process. Otherwise, test HAClusterTwoNodeTest#testClusterRestartWithoutDesignatedPrimary was failing.

        Show
        Alex Rudyy added a comment - Phil, I committed a modified version of your original patch in revision https://svn.apache.org/repos/asf/qpid/trunk@1438053 . I had to add a synchronized block into code starting the external broker in order to avoid adding into QPID_OPTS of JVM properties "test.virtualhosts" and "test.config" set by a concurrent start-up process. Otherwise, test HAClusterTwoNodeTest#testClusterRestartWithoutDesignatedPrimary was failing.

          People

          • Assignee:
            Alex Rudyy
            Reporter:
            Philip Harvey
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development