bz-63958 Correctly parse the forked mode launch definition in cases w…

…here a listener element is a sibling of either test or testclasses element
apache · Jan 11, 2020 · 6a06674 · 6a06674
1 parent c4aeb9c
commit 6a06674
Showing 5 changed files with 65 additions and 16 deletions.
diff --git a/WHATSNEW b/WHATSNEW
@@ -29,6 +29,11 @@ Fixed bugs:
  * Fixes a potential ConcurrentModificationException in XMLLogger.
    Bugzilla Report 63921
 
+ * Fixes a bug in junitlauncher task in forked mode, where if a listener element
+   was used as a sibling element for either the test or testclasses element,
+   then the forked mode launch would fail.
+   Bugzilla Report 63958
+
 Other changes:
 --------------
 

diff --git a/src/etc/testcases/taskdefs/optional/junitlauncher.xml b/src/etc/testcases/taskdefs/optional/junitlauncher.xml
@@ -351,5 +351,24 @@
             </testclasses>
         </junitlauncher>
     </target>
+
+    <target name="bz-63958">
+        <junitlauncher>
+            <classpath refid="test.classpath"/>
+            <testclasses>
+                <listener classname="org.example.junitlauncher.Tracker"
+                          outputDir="${output.dir}"
+                          resultFile="${bz-63958.tracker}"
+                          if="bz-63958.tracker"/>
+                <fileset dir="${build.classes.dir}">
+                    <include name="**/ForkedTest.class"/>
+                </fileset>
+                <fork>
+                    <sysproperty key="junitlauncher.test.sysprop.one" value="forked"/>
+                </fork>
+            </testclasses>
+            <listener type="legacy-plain" sendSysOut="true" />
+        </junitlauncher>
+    </target>
 </project>
 
diff --git a/src/main/org/apache/tools/ant/taskdefs/optional/junitlauncher/StandaloneLauncher.java b/src/main/org/apache/tools/ant/taskdefs/optional/junitlauncher/StandaloneLauncher.java
@@ -156,24 +156,26 @@ private static ForkedLaunch parseLaunchDefinition(final Path pathToLaunchDefXml)
             if (printSummary != null) {
                 forkedLaunch.setPrintSummary(Boolean.parseBoolean(printSummary));
             }
-            reader.nextTag();
-            reader.require(START_ELEMENT, null, null);
-            final String elementName = reader.getLocalName();
-            switch (elementName) {
-                case LD_XML_ELM_TEST: {
-                    forkedLaunch.addTests(Collections.singletonList(SingleTestClass.fromForkedRepresentation(reader)));
-                    break;
-                }
-                case LD_XML_ELM_TEST_CLASSES: {
-                    forkedLaunch.addTests(TestClasses.fromForkedRepresentation(reader));
-                    break;
-                }
-                case LD_XML_ELM_LISTENER: {
-                    forkedLaunch.addListener(ListenerDefinition.fromForkedRepresentation(reader));
-                    break;
+            int nextTag = reader.nextTag();
+            while (nextTag == START_ELEMENT) {
+                reader.require(START_ELEMENT, null, null);
+                final String elementName = reader.getLocalName();
+                switch (elementName) {
+                    case LD_XML_ELM_TEST: {
+                        forkedLaunch.addTests(Collections.singletonList(SingleTestClass.fromForkedRepresentation(reader)));
+                        break;
+                    }
+                    case LD_XML_ELM_TEST_CLASSES: {
+                        forkedLaunch.addTests(TestClasses.fromForkedRepresentation(reader));
+                        break;
+                    }
+                    case LD_XML_ELM_LISTENER: {
+                        forkedLaunch.addListener(ListenerDefinition.fromForkedRepresentation(reader));
+                        break;
+                    }
                 }
+                nextTag = reader.nextTag();
             }
-            reader.nextTag();
             reader.require(END_ELEMENT, null, LD_XML_ELM_LAUNCH_DEF);
             reader.next();
             reader.require(END_DOCUMENT, null, null);

diff --git a/...ain/org/apache/tools/ant/taskdefs/optional/junitlauncher/confined/ListenerDefinition.java b/...ain/org/apache/tools/ant/taskdefs/optional/junitlauncher/confined/ListenerDefinition.java
@@ -28,6 +28,7 @@
 
 import static org.apache.tools.ant.taskdefs.optional.junitlauncher.confined.Constants.LD_XML_ATTR_CLASS_NAME;
 import static org.apache.tools.ant.taskdefs.optional.junitlauncher.confined.Constants.LD_XML_ATTR_LISTENER_RESULT_FILE;
+import static org.apache.tools.ant.taskdefs.optional.junitlauncher.confined.Constants.LD_XML_ATTR_OUTPUT_DIRECTORY;
 import static org.apache.tools.ant.taskdefs.optional.junitlauncher.confined.Constants.LD_XML_ATTR_SEND_SYS_ERR;
 import static org.apache.tools.ant.taskdefs.optional.junitlauncher.confined.Constants.LD_XML_ATTR_SEND_SYS_OUT;
 import static org.apache.tools.ant.taskdefs.optional.junitlauncher.confined.Constants.LD_XML_ELM_LISTENER;
@@ -156,6 +157,9 @@ void toForkedRepresentation(final XMLStreamWriter writer) throws XMLStreamExcept
         writer.writeAttribute(LD_XML_ATTR_CLASS_NAME, this.className);
         writer.writeAttribute(LD_XML_ATTR_SEND_SYS_ERR, Boolean.toString(this.sendSysErr));
         writer.writeAttribute(LD_XML_ATTR_SEND_SYS_OUT, Boolean.toString(this.sendSysOut));
+        if (this.outputDir != null) {
+            writer.writeAttribute(LD_XML_ATTR_OUTPUT_DIRECTORY, this.outputDir);
+        }
         if (this.resultFile != null) {
             writer.writeAttribute(LD_XML_ATTR_LISTENER_RESULT_FILE, this.resultFile);
         }
@@ -175,6 +179,10 @@ public static ListenerDefinition fromForkedRepresentation(final XMLStreamReader
         if (sendSysOut != null) {
             listenerDef.setSendSysOut(Boolean.parseBoolean(sendSysOut));
         }
+        final String outputDir = reader.getAttributeValue(null, LD_XML_ATTR_OUTPUT_DIRECTORY);
+        if (outputDir != null) {
+            listenerDef.setOutputDir(outputDir);
+        }
         final String resultFile = reader.getAttributeValue(null, LD_XML_ATTR_LISTENER_RESULT_FILE);
         if (resultFile != null) {
             listenerDef.setResultFile(resultFile);

diff --git a/...sts/junit/org/apache/tools/ant/taskdefs/optional/junitlauncher/JUnitLauncherTaskTest.java b/...sts/junit/org/apache/tools/ant/taskdefs/optional/junitlauncher/JUnitLauncherTaskTest.java
@@ -465,6 +465,21 @@ public void testMethodWithTagFileSetFork() throws Exception {
         Files.deleteIfExists(tracker);
     }
 
+
+    /**
+     * Tests that the forked test works fine when the {@code testclasses} element is used
+     * as a sibling of a {@code listener} element
+     */
+    @Test
+    public void testBz63958() throws Exception {
+        final String targetName = "bz-63958";
+        final Path trackerFile = setupTrackerProperty(targetName);
+        buildRule.executeTarget(targetName);
+
+        Assert.assertTrue("ForkedTest#testSysProp was expected to succeed", verifySuccess(trackerFile,
+                ForkedTest.class.getName(), "testSysProp"));
+    }
+
     private Path setupTrackerProperty(final String targetName) {
         final String filename = targetName + "-tracker.txt";
         buildRule.getProject().setProperty(targetName + ".tracker", filename);