Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 2.0 (2.2 plugin)
    • Fix Version/s: 2.3.1
    • Component/s: JUnit 3.x support
    • Labels:
      None
    • Environment:
      maven2.0.4, sun-jdk-1.5.0.09, maven-surefire-plugin 2.2, surefire 2.0, gentoo linux x86
    • Flags:
      Patch

      Description

      Surefire incorrectly interprets classpath ordering.
      Steps to reproduce:
      1. unzip my-app.zip - it's a simple mvn project created with
      mvn archetype:create -DgroupId=com.mycompany.app -DartifactId=my-app
      and lightly patched
      2. mvn test
      in my case, it prints out
      jar:file:/home/vyzivus/.m2/repository/jxta/jxta/2.0/jxta-2.0.jar!/log4j.properties
      jar:file:/home/vyzivus/.m2/repository/jxta/jxta/2.0/jxta-2.0.jar!/log4j.properties
      which is incorrect. log4j.properties is located both in jxta.jar and src/test/resources, but I think that src/test/resources takes precedence over jxta. This ordering is set correctly in surefire36745tmp file I think, but surefire seems to ignore the ordering.

      1. my-app.zip
        5 kB
        Martin Vysny
      2. output.log
        22 kB
      3. SUREFIRE61_barrettas_surefire_surefire-booter_for_rev_489098.patch
        2 kB
        Barrett Snyder
      4. SUREFIRE61-surefire-booter-r558713.patch
        2 kB
        Paul Gier

        Issue Links

          Activity

          Hide
          Barrett Snyder added a comment -

          This seems to be due to adding classpath entries into the surefire properties file in the form of "classpath.[number]" and then these are getting sorted with a string sort. Therefore 29 classpath entries would be sorted as:
          classPathUrl.9
          classPathUrl.8
          classPathUrl.7
          classPathUrl.6
          classPathUrl.5
          classPathUrl.4
          classPathUrl.3
          classPathUrl.2
          classPathUrl.1
          classPathUrl.0
          classPathUrl.29
          classPathUrl.28
          classPathUrl.27
          classPathUrl.26
          classPathUrl.25
          classPathUrl.24
          classPathUrl.23
          classPathUrl.22
          classPathUrl.21
          classPathUrl.20
          classPathUrl.19
          classPathUrl.18
          classPathUrl.17
          classPathUrl.16
          classPathUrl.15
          classPathUrl.14
          classPathUrl.13
          classPathUrl.12
          classPathUrl.11
          classPathUrl.10

          This then gets read back in via the SurefireBooter.main method (only if execution was forked) and never re-sorted by number to correct the order, thus causing the classpath to get loaded in the incorrect order.

          Show
          Barrett Snyder added a comment - This seems to be due to adding classpath entries into the surefire properties file in the form of "classpath. [number] " and then these are getting sorted with a string sort. Therefore 29 classpath entries would be sorted as: classPathUrl.9 classPathUrl.8 classPathUrl.7 classPathUrl.6 classPathUrl.5 classPathUrl.4 classPathUrl.3 classPathUrl.2 classPathUrl.1 classPathUrl.0 classPathUrl.29 classPathUrl.28 classPathUrl.27 classPathUrl.26 classPathUrl.25 classPathUrl.24 classPathUrl.23 classPathUrl.22 classPathUrl.21 classPathUrl.20 classPathUrl.19 classPathUrl.18 classPathUrl.17 classPathUrl.16 classPathUrl.15 classPathUrl.14 classPathUrl.13 classPathUrl.12 classPathUrl.11 classPathUrl.10 This then gets read back in via the SurefireBooter.main method (only if execution was forked) and never re-sorted by number to correct the order, thus causing the classpath to get loaded in the incorrect order.
          Hide
          Barrett Snyder added a comment -

          After a bit more research and testing I was able to create a patch for the surefire-booter revision 489098. Patch is attached.

          Show
          Barrett Snyder added a comment - After a bit more research and testing I was able to create a patch for the surefire-booter revision 489098. Patch is attached.
          Hide
          Barrett Snyder added a comment -

          Applies to revision 489098.

          Show
          Barrett Snyder added a comment - Applies to revision 489098.
          Hide
          asgeirn added a comment -

          Any hope on getting this fixed and released on the maven-surefire-plugin 2.2 branch?

          Show
          asgeirn added a comment - Any hope on getting this fixed and released on the maven-surefire-plugin 2.2 branch?
          Hide
          Kenney Westerhof added a comment -

          Cannot reproduce with surefire 2.4-snapshot from trunk. The classpath seems ok:

          • first target/test-classes
          • then target/classes
          • then dependencies

          It prints out the path to the target/test-classes/log4j.properties.

          There's no 'sort' anywhere in the surefire sources.

          Can you please check this again with the latest surefire snapshots to see if it's solved?

          Show
          Kenney Westerhof added a comment - Cannot reproduce with surefire 2.4-snapshot from trunk. The classpath seems ok: first target/test-classes then target/classes then dependencies It prints out the path to the target/test-classes/log4j.properties. There's no 'sort' anywhere in the surefire sources. Can you please check this again with the latest surefire snapshots to see if it's solved?
          Hide
          Matt Read added a comment -

          2.4-SNAPSHOT gives me the opposite result, i.e. target/class first, then target/test-classes, then dependencies. As does 2.3.

          I have attached output of the following commands:

          mvn archetype:create -DgroupId=test -DartifactId=classpath_test -DarchetypeArtifactId=maven-archetype-quickstart
          cd classpath_test
          mvn -X test > output.log

          Note this bit:

          [DEBUG] Test Classpath :
          [DEBUG] C:\TEMP\a_directory_of_my_choosing\classpath_test\target\classes
          [DEBUG] C:\TEMP\a_directory_of_my_choosing\classpath_test\target\test-classes
          [DEBUG] C:\data\sandbox\maven2\repo\junit\junit\3.8.1\junit-3.8.1.jar

          Show
          Matt Read added a comment - 2.4-SNAPSHOT gives me the opposite result, i.e. target/class first, then target/test-classes, then dependencies. As does 2.3. I have attached output of the following commands: mvn archetype:create -DgroupId=test -DartifactId=classpath_test -DarchetypeArtifactId=maven-archetype-quickstart cd classpath_test mvn -X test > output.log Note this bit: [DEBUG] Test Classpath : [DEBUG] C:\TEMP\a_directory_of_my_choosing\classpath_test\target\classes [DEBUG] C:\TEMP\a_directory_of_my_choosing\classpath_test\target\test-classes [DEBUG] C:\data\sandbox\maven2\repo\junit\junit\3.8.1\junit-3.8.1.jar
          Hide
          Matt Read added a comment - - edited

          This is causing me some major problems now. Can anyone comment in any way on why I'm getting an ordering of target/classes before target/test-classes in the example above? I have to go back to 2.1.3 to find a version that puts target/test-classes in front of target/classes.

          Show
          Matt Read added a comment - - edited This is causing me some major problems now. Can anyone comment in any way on why I'm getting an ordering of target/classes before target/test-classes in the example above? I have to go back to 2.1.3 to find a version that puts target/test-classes in front of target/classes.
          Hide
          Matt Read added a comment -

          I've checked out the surefire code and SurefirePlugin.java - line 649 says:

          // no need to add classes/test classes directory here - they are in the classpath elements already

          I'm having major problems digging out exactly what is putting these directories on the classpath in the first place - it doesn't seem to be the responsibility of the SureFire plugin but perhaps passed in from Plexus? Can anyone point me in the right direction?

          This issue is blocking a major release for me and I'm looking for as much help as possible. i'll certainly supply the patch if I get to the bottom of this!

          Show
          Matt Read added a comment - I've checked out the surefire code and SurefirePlugin.java - line 649 says: // no need to add classes/test classes directory here - they are in the classpath elements already I'm having major problems digging out exactly what is putting these directories on the classpath in the first place - it doesn't seem to be the responsibility of the SureFire plugin but perhaps passed in from Plexus? Can anyone point me in the right direction? This issue is blocking a major release for me and I'm looking for as much help as possible. i'll certainly supply the patch if I get to the bottom of this!
          Hide
          Paul Gier added a comment -

          The classes directory is put in front of the test-classes directory in MavenProject.java.
          I created another issue under maven-project to fix this.

          Show
          Paul Gier added a comment - The classes directory is put in front of the test-classes directory in MavenProject.java. I created another issue under maven-project to fix this.
          Hide
          Paul Gier added a comment -

          The issue can be seen mainly when the fork mode is set to never. Because if the surefire plugin forks, then the ordering gets rearranged similar to what Barrett described. However, I think this is caused by loading the paths into the properties object and then using an enumeration to iterate through them. The Properties object doesn't retain the order of the loaded props, so they really could come out in any order, not just the sorted order that is shown above.

          So I think that Barrett's approach is the right one. The classpath elements really need to be sorted after they are loaded from properties file. Hopefully someone will apply his patch or something similar to it soon

          Show
          Paul Gier added a comment - The issue can be seen mainly when the fork mode is set to never. Because if the surefire plugin forks, then the ordering gets rearranged similar to what Barrett described. However, I think this is caused by loading the paths into the properties object and then using an enumeration to iterate through them. The Properties object doesn't retain the order of the loaded props, so they really could come out in any order, not just the sorted order that is shown above. So I think that Barrett's approach is the right one. The classpath elements really need to be sorted after they are loaded from properties file. Hopefully someone will apply his patch or something similar to it soon
          Hide
          Paul Gier added a comment -

          Attaching a slightly cleaned up patch.

          Show
          Paul Gier added a comment - Attaching a slightly cleaned up patch.
          Hide
          Brett Porter added a comment -

          will review patch for 2.3.1

          Show
          Brett Porter added a comment - will review patch for 2.3.1
          Hide
          Brett Porter added a comment -

          applied, thanks

          Show
          Brett Porter added a comment - applied, thanks

            People

            • Assignee:
              Brett Porter
              Reporter:
              Martin Vysny
            • Votes:
              5 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development