junit task should support parallel/threads option

PR 55925

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

CONTRIBUTORS

@@ -183,6 +183,7 @@ Jim Allers
 Joerg Wassmer
 Joey Richey
 Johann Herunter
+John Elion
 John Sisson
 Jon Dickinson
 Jon S. Stevens

WHATSNEW

@@ -144,6 +144,9 @@ Other changes:
    when enabled.
    GitHub Pull Request #1
 
+* <junit> has now a threads attribute allowing to run the tests in several threads.
+  Bugzilla Report 55925
+
 Changes from Ant 1.9.2 TO Ant 1.9.3
 ===================================
 

build.xml

@@ -111,6 +111,12 @@
   <property name="test.haltonfailure" value="false"/>
   <property name="junit.fork" value="true"/>
   <property name="junit.forkmode" value="once"/>
+  <condition property="junit.threads" value="2" else="0">
+    <and>
+      <equals arg1="${junit.fork}" arg2="true"/>
+      <equals arg1="${junit.forkmode}" arg2="perTest"/>
+    </and>
+</condition>
   <property name="expandproperty.files"
             value="**/version.txt,**/defaultManifest.mf"/>
   <property name="junit.collector.dir" value="${build.dir}/failingTests"/>
@@ -1659,6 +1665,7 @@ ${antunit.reports}
                haltonfailure="${test.haltonfailure}"
                fork="${junit.fork}"
                forkmode="${junit.forkmode}"
+               threads="${junit.threads}"
                failureproperty="junit.failed"
                errorproperty="junit.failed"
                filtertrace="${junit.filtertrace}">

contributors.xml

@@ -755,6 +755,10 @@
     <first>Johann</first>
     <last>Herunter</last>
   </name>
+  <name>
+    <first>John</first>
+    <last>Elion</last>
+  </name>
   <name>
     <first>John</first>
     <last>Sisson</last>

manual/Tasks/junit.html

@@ -247,7 +247,16 @@ elements</a>).</p>
       <em>since Ant 1.8.2</em> - <strong>Ant 1.7.0 to 1.8.1 behave as
         if this attribute was true by default.</strong></td>
     <td align="center" valign="top">No</td>
-  </tr>    
+  </tr>
+  <tr>
+    <td valign="top">threads</td>
+    <td valign="top">a number of threads to run the tests in.<br/>
+      When this attribute is specified the tests will be split arbitrarily among the threads.<br/>
+      requires that the tests be forked with the <code>perTest</code>
+      option to be operative.<br/>
+      <em>since Ant 1.9.4</em></td>
+    <td align="center" valign="top">No</td>
+  </tr>
 </table>
 
 <p>By using the <code>errorproperty</code> and <code>failureproperty</code>

src/main/org/apache/tools/ant/taskdefs/optional/junit/Constants.java

@@ -38,4 +38,6 @@ public class Constants {
     static final String TERMINATED_SUCCESSFULLY = "terminated successfully";
     static final String LOG_FAILED_TESTS="logfailedtests=";
     static final String SKIP_NON_TESTS = "skipNonTests=";
+    /** @since Ant 1.9.4 */
+    static final String THREADID="threadid=";
 }

src/main/org/apache/tools/ant/taskdefs/optional/junit/JUnitTask.java

@@ -172,11 +172,15 @@ public class JUnitTask extends Task {
     /** A boolean on whether to get the forked path for ant classes */
     private boolean forkedPathChecked = false;
 
+    /* set when a test fails/errs with haltonfailure/haltonerror and >1 thread to stop other threads */
+    private volatile BuildException caughtBuildException = null;
+
     //   Attributes for basetest
     private boolean haltOnError = false;
     private boolean haltOnFail  = false;
     private boolean filterTrace = true;
     private boolean fork        = false;
+    private int     threads     = 1;
     private String  failureProperty;
     private String  errorProperty;
 
@@ -319,6 +323,22 @@ public class JUnitTask extends Task {
         this.forkMode = mode;
     }
 
+    /**
+     * Set the number of test threads to be used for parallel test
+     * execution.  The default is 1, which is the same behavior as
+     * before parallel test execution was possible.
+     *
+     * <p>This attribute will be ignored if tests run in the same VM
+     * as Ant.</p>
+     *
+     * @since Ant 1.9.4
+     */
+    public void setThreads(int threads) {
+        if (threads >= 0) {
+            this.threads = threads;
+        }
+    }
+
     /**
      * If true, print one-line statistics for each test, or "withOutAndErr"
      * to also show standard output and error.
@@ -798,6 +818,10 @@ public class JUnitTask extends Task {
         setupJUnitDelegate();
 
         List testLists = new ArrayList();
+        /* parallel test execution is only supported for multi-process execution */
+        int threads = ((!fork) || (forkMode.getValue().equals(ForkMode.ONCE))
+                       ? 1
+                       : this.threads);
 
         boolean forkPerTest = forkMode.getValue().equals(ForkMode.PER_TEST);
         if (forkPerTest || forkMode.getValue().equals(ForkMode.ONCE)) {
@@ -813,29 +837,172 @@ public class JUnitTask extends Task {
         }
 
         try {
-            Iterator iter = testLists.iterator();
-            while (iter.hasNext()) {
-                List l = (List) iter.next();
-                if (l.size() == 1) {
-                    execute((JUnitTest) l.get(0));
-                } else {
-                    execute(l);
-                }
-            }
+            /* prior to parallel the code in 'oneJunitThread' used to be here. */
+            runTestsInThreads(testLists, threads);
         } finally {
             cleanup();
         }
     }
 
+    /*
+     * When the list of tests is established, an array of threads is created to pick the
+     * tests off the list one at a time and execute them until the list is empty.  Tests are
+     * not assigned to threads until the thread is available.
+     *
+     * This class is the runnable thread subroutine that takes care of passing the shared
+     * list iterator and the handle back to the main class to the test execution subroutine
+     * code 'runTestsInThreads'.  One object is created for each thread and each one gets
+     * a unique thread id that can be useful for tracing test starts and stops.
+     *
+     * Because the threads are picking tests off the same list, it is the list *iterator*
+     * that must be shared, not the list itself - and the iterator must have a thread-safe
+     * ability to pop the list - hence the synchronized 'getNextTest'.
+     */
+    private class JunitTestThread implements Runnable {
+
+        JunitTestThread(JUnitTask master, Iterator iterator, int id) {
+            this.masterTask = master;
+            this.iterator = iterator;
+            this.id = id;
+        }
+
+        public void run() {
+            try {
+                masterTask.oneJunitThread(iterator, id);
+            } catch (BuildException b) {
+                /* saved to rethrow in main thread to be like single-threaded case */
+                caughtBuildException = b;
+            }
+        }
+
+        private JUnitTask masterTask;
+        private Iterator iterator;
+        private int id;               
+    }
+
+    /*
+     * Because the threads are picking tests off the same list, it is the list *iterator*
+     * that must be shared, not the list itself - and the iterator must have a thread-safe
+     * ability to pop the list - hence the synchronized 'getNextTest'.  We can't have two
+     * threads get the same test, or two threads simultaneously pop the list so that a test
+     * gets skipped!
+     */
+    private List getNextTest(Iterator iter) {
+        synchronized(iter) {
+            if (iter.hasNext()) {
+                return (List) iter.next();
+            }
+            return null;
+        }
+    }
+
+    /*
+     * This code loops keeps executing the next test or test bunch (depending on fork mode)
+     * on the list of test cases until none are left.  Basically this body of code used to
+     * be in the execute routine above; now, several copies (one for each test thread) execute
+     * simultaneously.  The while loop was modified to call the new thread-safe atomic list
+     * popping subroutine and the logging messages were added.
+     *
+     * If one thread aborts due to a BuildException (haltOnError, haltOnFailure, or any other
+     * fatal reason, no new tests/batches will be started but the running threads will be
+     * permitted to complete.  Additional tests may start in already-running batch-test threads.
+     */
+    private void oneJunitThread(Iterator iter, int threadId) {
+
+        List l;
+        log("Starting test thread " + threadId, Project.MSG_VERBOSE);
+        while ((caughtBuildException == null) && ((l = getNextTest(iter)) != null)) {
+            log("Running test " + l.get(0).toString() + "(" + l.size() + ") in thread " + threadId, Project.MSG_VERBOSE);
+            if (l.size() == 1) {
+                execute((JUnitTest) l.get(0), threadId);
+            } else {
+                execute(l, threadId);
+            }
+        }
+       log("Ending test thread " + threadId, Project.MSG_VERBOSE);
+    }
+
+
+    private void runTestsInThreads(List testList, int numThreads) {
+
+        Iterator iter = testList.iterator();
+
+        if (numThreads == 1) {
+            /* with just one thread just run the test - don't create any threads */
+            oneJunitThread(iter, 0);
+        }
+        else {
+            Thread threads[] = new Thread[numThreads];
+            int i;
+            boolean exceptionOccurred;
+
+            /* Need to split apart tests, which are still grouped in batches */
+            /* is there a simpler Java mechanism to do this? */
+            /* I assume we don't want to do this with "per batch" forking. */
+            List newlist = new ArrayList();
+            if (forkMode.getValue().equals(ForkMode.PER_TEST)) {
+                Iterator i1 = testList.iterator();
+                while (i1.hasNext()) {
+                    List l = (List) i1.next();
+                    if (l.size() == 1) {
+                         newlist.add(l);
+                    } else {
+                         Iterator i2 = l.iterator();
+                         while (i2.hasNext()) {
+                             List tmpSingleton = new ArrayList();
+                             tmpSingleton.add(i2.next());
+                             newlist.add(tmpSingleton);
+                         }
+                    }
+                }
+            } else {
+                 newlist = testList;
+            }
+            iter = newlist.iterator();
+
+            /* create 1 thread using the passthrough class, and let each thread start */
+            for (i = 0; i < numThreads; i++) {
+                threads[i] = new Thread(new JunitTestThread(this, iter, i+1));
+                threads[i].start();
+            }
+
+            /* wait for all of the threads to complete.  Not sure if the exception can actually occur in this use case. */
+            do {
+                exceptionOccurred = false;
+
+                try {
+                    for (i = 0; i < numThreads; i++) {
+                         threads[i].join();
+                    }
+                }
+                catch (InterruptedException e) {
+                    exceptionOccurred = true;
+                }
+            } while (exceptionOccurred);
+
+           /* an exception occurred in one of the threads - usually a haltOnError/Failure.
+              throw the exception again so it behaves like the single-thread case */
+           if (caughtBuildException != null) {
+               throw new BuildException(caughtBuildException);
+           }
+
+            /* all threads are completed - that's all there is to do. */
+            /* control will flow back to the test cleanup call and then execute is done. */
+        }
+    }
+
     /**
      * Run the tests.
      * @param arg one JUnitTest
+     * @param thread Identifies which thread is test running in (0 for single-threaded runs)
      * @throws BuildException in case of test failures or errors
      */
-    protected void execute(JUnitTest arg) throws BuildException {
+    protected void execute(JUnitTest arg, int thread) throws BuildException {
         validateTestName(arg.getName());
 
         JUnitTest test = (JUnitTest) arg.clone();
+        test.setThread(thread);
+
         // set the default values if not specified
         //@todo should be moved to the test class instead.
         if (test.getTodir() == null) {
@@ -858,6 +1025,15 @@ public class JUnitTask extends Task {
         actOnTestResult(result, test, "Test " + test.getName());
     }
 
+    /**
+     * Run the tests.
+     * @param arg one JUnitTest
+     * @throws BuildException in case of test failures or errors
+     */
+    protected void execute(JUnitTest arg) throws BuildException {
+        execute(arg, 0);
+    }
+
     /**
      * Throws a <code>BuildException</code> if the given test name is invalid.
      * Validity is defined as not <code>null</code>, not empty, and not the
@@ -875,9 +1051,10 @@ public class JUnitTask extends Task {
     /**
      * Execute a list of tests in a single forked Java VM.
      * @param testList the list of tests to execute.
+     * @param thread Identifies which thread is test running in (0 for single-threaded runs)
      * @throws BuildException on error.
      */
-    protected void execute(List testList) throws BuildException {
+    protected void execute(List testList, int thread) throws BuildException {
         JUnitTest test = null;
         // Create a temporary file to pass the test cases to run to
         // the runner (one test case per line)
@@ -894,6 +1071,7 @@ public class JUnitTask extends Task {
             Iterator iter = testList.iterator();
             while (iter.hasNext()) {
                 test = (JUnitTest) iter.next();
+                test.setThread(thread);
                 printDual(writer, logWriter, test.getName());
                 if (test.getMethods() != null) {
                     printDual(writer, logWriter, ":" + test.getMethodsString().replace(',', '+'));
@@ -935,6 +1113,15 @@ public class JUnitTask extends Task {
         }
     }
 
+    /**
+     * Execute a list of tests in a single forked Java VM.
+     * @param testList the list of tests to execute.
+     * @throws BuildException on error.
+     */
+    protected void execute(List testList) throws BuildException {
+        execute(testList, 0);
+    }
+
     /**
      * Execute a testcase by forking a new JVM. The command will block
      * until it finishes. To know if the process was destroyed or not
@@ -991,6 +1178,8 @@ public class JUnitTask extends Task {
                                       + String.valueOf(outputToFormatters));
         cmd.createArgument().setValue(Constants.LOG_FAILED_TESTS
                                       + String.valueOf(logFailedTests));
+        cmd.createArgument().setValue(Constants.THREADID
+                                      + String.valueOf(test.getThread()));
 
         // #31885
         cmd.createArgument().setValue(Constants.LOGTESTLISTENEREVENTS
@@ -1900,8 +2089,10 @@ public class JUnitTask extends Task {
         while (testList.hasMoreElements()) {
             JUnitTest test = (JUnitTest) testList.nextElement();
             if (test.shouldRun(getProject())) {
-                if (runIndividual || !test.getFork()) {
-                    execute(test);
+                /* with multi-threaded runs need to defer execution of even */
+                /* individual tests so the threads can pick tests off the queue. */
+                if ((runIndividual || !test.getFork()) && (threads == 1)) {
+                    execute(test, 0);
                 } else {
                     ForkedTestConfiguration c =
                         new ForkedTestConfiguration(test);

src/main/org/apache/tools/ant/taskdefs/optional/junit/JUnitTest.java

@@ -69,6 +69,8 @@ public class JUnitTest extends BaseTest implements Cloneable {
 
     private long runTime;
 
+    private int antThreadID;
+
     // Snapshot of the system properties
     private Properties props = null;
 
@@ -93,9 +95,9 @@ public class JUnitTest extends BaseTest implements Cloneable {
      */
     public JUnitTest(String name, boolean haltOnError, boolean haltOnFailure,
             boolean filtertrace) {
-        this(name, haltOnError, haltOnFailure, filtertrace, null);
-    }    
-    
+        this(name, haltOnError, haltOnFailure, filtertrace, null, 0);
+    }
+
     /**
      * Constructor with options.
      * @param name the name of the test.
@@ -107,12 +109,28 @@ public class JUnitTest extends BaseTest implements Cloneable {
      */
     public JUnitTest(String name, boolean haltOnError, boolean haltOnFailure,
                      boolean filtertrace, String[] methods) {
+        this(name, haltOnError, haltOnFailure, filtertrace, methods, 0);    
+    }
+
+    /**
+     * Constructor with options.
+     * @param name the name of the test.
+     * @param haltOnError if true halt the tests if there is an error.
+     * @param haltOnFailure if true halt the tests if there is a failure.
+     * @param filtertrace if true filter stack traces.
+     * @param methods if non-null run only these test methods
+     * @param thread Ant thread ID in which test is currently running
+     * @since 1.9.4
+     */
+    public JUnitTest(String name, boolean haltOnError, boolean haltOnFailure,
+                     boolean filtertrace, String[] methods, int thread) {
         this.name  = name;
         this.haltOnError = haltOnError;
         this.haltOnFail = haltOnFailure;
         this.filtertrace = filtertrace;
         this.methodsSpecified = methods != null;
         this.methods = methodsSpecified ? (String[]) methods.clone() : null;
+        this.antThreadID = thread;
     }
 
     /**
@@ -148,6 +166,17 @@ public class JUnitTest extends BaseTest implements Cloneable {
         name = value;
     }
 
+    /**
+     * Set the thread id
+     * @param thread the Ant id of the thread running this test
+	 * (this is not the system process or thread id)
+     * (this will be 0 in single-threaded mode).
+     * @since Ant 1.9.4
+     */
+    public void setThread(int thread) {
+        this.antThreadID = thread;
+    }
+
     /**
      * Set the name of the output file.
      * @param value the name of the output file to use.
@@ -347,6 +376,14 @@ public class JUnitTest extends BaseTest implements Cloneable {
         return name;
     }
 
+    /**
+     * Get the Ant id of the thread running the test.
+     * @return the thread id
+     */
+    public int getThread() {
+        return antThreadID;
+    }
+
     /**
      * Get the name of the output file
      *

src/main/org/apache/tools/ant/taskdefs/optional/junit/JUnitTestRunner.java

@@ -899,7 +899,7 @@ public class JUnitTestRunner implements TestListener, JUnitTaskMirror.JUnitTestR
         boolean logFailedTests = true;
         boolean logTestListenerEvents = false;
         boolean skipNonTests = false;
-
+        int antThreadID = 0; /* Ant id of thread running this unit test, 0 in single-threaded mode */
 
         if (args.length == 0) {
             System.err.println("required argument TestClassName missing");
@@ -955,6 +955,8 @@ public class JUnitTestRunner implements TestListener, JUnitTaskMirror.JUnitTestR
             } else if (args[i].startsWith(Constants.SKIP_NON_TESTS)) {
                 skipNonTests = Project.toBoolean(
                     args[i].substring(Constants.SKIP_NON_TESTS.length()));
+            } else if (args[i].startsWith(Constants.THREADID)) {
+                antThreadID = Integer.parseInt( args[i].substring(Constants.THREADID.length()) );
             }
         }
 
@@ -995,6 +997,7 @@ public class JUnitTestRunner implements TestListener, JUnitTaskMirror.JUnitTestR
                     t.setOutfile(st.nextToken());
                     t.setProperties(props);
                     t.setSkipNonTests(skipNonTests);
+                    t.setThread(antThreadID);
                     code = launch(t, testMethodNames, haltError, stackfilter, haltFail,
                                   showOut, outputToFormat,
                                   logTestListenerEvents);
@@ -1021,6 +1024,7 @@ public class JUnitTestRunner implements TestListener, JUnitTaskMirror.JUnitTestR
             }
         } else {
             JUnitTest t = new JUnitTest(args[0]);
+            t.setThread(antThreadID);
             t.setProperties(props);
             t.setSkipNonTests(skipNonTests);
             returnCode = launch(

src/main/org/apache/tools/ant/taskdefs/optional/junit/SummaryJUnitResultFormatter.java

@@ -53,6 +53,27 @@ public class SummaryJUnitResultFormatter
      */
     public SummaryJUnitResultFormatter() {
     }
+
+    /**
+     *  Insures that a line of log output is written and flushed as a single
+     *  operation, to prevent lines from being spliced into other lines.
+     *  (Hopefully this solves the issue of run on lines -
+     *  [junit] Tests Run: 2 Failures: 2 [junit] Tests run: 5...
+     *  synchronized doesn't seem to be to harsh a penalty since it only
+     *  occurs twice per test - at the beginning and end.  Note that message
+     *  construction occurs outside the locked block.
+     *
+     *  @param b data to be written as an unbroken block
+     */
+    private synchronized void writeOutputLine(byte[] b) {
+        try {
+            out.write(b);
+            out.flush();
+        } catch (IOException ioex) {
+            throw new BuildException("Unable to write summary output", ioex);
+        }
+    }
+
     /**
      * The testsuite started.
      * @param suite the testsuite.
@@ -60,15 +81,16 @@ public class SummaryJUnitResultFormatter
     public void startTestSuite(JUnitTest suite) {
         String newLine = System.getProperty("line.separator");
         StringBuffer sb = new StringBuffer("Running ");
-        sb.append(suite.getName());
-        sb.append(newLine);
+        int antThreadID = suite.getThread();
 
-        try {
-            out.write(sb.toString().getBytes());
-            out.flush();
-        } catch (IOException ioex) {
-            throw new BuildException("Unable to write summary output", ioex);
+        sb.append(suite.getName());
+        /* only write thread id in multi-thread mode so default old way doesn't change output */
+        if (antThreadID > 0) {
+            sb.append(" in thread ");
+            sb.append(antThreadID);
         }
+        sb.append(newLine);
+        writeOutputLine(sb.toString().getBytes());
     }
     /**
      * Empty
@@ -149,6 +171,17 @@ public class SummaryJUnitResultFormatter
         sb.append(", Time elapsed: ");
         sb.append(nf.format(suite.getRunTime() / ONE_SECOND));
         sb.append(" sec");
+
+        /* class name needed with multi-threaded execution because
+           results line may not appear immediately below start line.
+           only write thread id, class name in multi-thread mode so
+           the line still looks as much like the old line as possible. */
+        if (suite.getThread() > 0) {
+            sb.append(", Thread: ");
+            sb.append(suite.getThread());
+            sb.append(", Class: ");
+            sb.append(suite.getName());
+        }
         sb.append(newLine);
 
         if (withOutAndErr) {
@@ -164,10 +197,7 @@ public class SummaryJUnitResultFormatter
         }
 
         try {
-            out.write(sb.toString().getBytes());
-            out.flush();
-        } catch (IOException ioex) {
-            throw new BuildException("Unable to write summary output", ioex);
+            writeOutputLine(sb.toString().getBytes());
         } finally {
             if (out != System.out && out != System.err) {
                 try {