diff --git a/CONTRIBUTORS b/CONTRIBUTORS index 47a79636a..dfe5f1792 100644 --- a/CONTRIBUTORS +++ b/CONTRIBUTORS @@ -183,6 +183,7 @@ Jim Allers Joerg Wassmer Joey Richey Johann Herunter +John Elion John Sisson Jon Dickinson Jon S. Stevens diff --git a/WHATSNEW b/WHATSNEW index 657c58818..21953a30d 100644 --- a/WHATSNEW +++ b/WHATSNEW @@ -144,6 +144,9 @@ Other changes: when enabled. GitHub Pull Request #1 +* 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 =================================== diff --git a/build.xml b/build.xml index f25683d13..45ce6f5a2 100644 --- a/build.xml +++ b/build.xml @@ -111,6 +111,12 @@ + + + + + + @@ -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}"> diff --git a/contributors.xml b/contributors.xml index 965ac0c05..ec10b9b75 100644 --- a/contributors.xml +++ b/contributors.xml @@ -755,6 +755,10 @@ Johann Herunter + + John + Elion + John Sisson diff --git a/manual/Tasks/junit.html b/manual/Tasks/junit.html index 2a9ebeb6c..2a328eb22 100644 --- a/manual/Tasks/junit.html +++ b/manual/Tasks/junit.html @@ -247,7 +247,16 @@ elements).

since Ant 1.8.2 - Ant 1.7.0 to 1.8.1 behave as if this attribute was true by default. No - + + + threads + a number of threads to run the tests in.
+ When this attribute is specified the tests will be split arbitrarily among the threads.
+ requires that the tests be forked with the perTest + option to be operative.
+ since Ant 1.9.4 + No +

By using the errorproperty and failureproperty diff --git a/src/main/org/apache/tools/ant/taskdefs/optional/junit/Constants.java b/src/main/org/apache/tools/ant/taskdefs/optional/junit/Constants.java index 5b8016a82..dc891b12a 100644 --- a/src/main/org/apache/tools/ant/taskdefs/optional/junit/Constants.java +++ b/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="; } diff --git a/src/main/org/apache/tools/ant/taskdefs/optional/junit/JUnitTask.java b/src/main/org/apache/tools/ant/taskdefs/optional/junit/JUnitTask.java index 6af8b99b0..998bef173 100644 --- a/src/main/org/apache/tools/ant/taskdefs/optional/junit/JUnitTask.java +++ b/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. + * + *

This attribute will be ignored if tests run in the same VM + * as Ant.

+ * + * @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 BuildException if the given test name is invalid. * Validity is defined as not null, 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); diff --git a/src/main/org/apache/tools/ant/taskdefs/optional/junit/JUnitTest.java b/src/main/org/apache/tools/ant/taskdefs/optional/junit/JUnitTest.java index be8f44984..8a298a777 100644 --- a/src/main/org/apache/tools/ant/taskdefs/optional/junit/JUnitTest.java +++ b/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 * diff --git a/src/main/org/apache/tools/ant/taskdefs/optional/junit/JUnitTestRunner.java b/src/main/org/apache/tools/ant/taskdefs/optional/junit/JUnitTestRunner.java index 81e71aa84..9628ac92b 100644 --- a/src/main/org/apache/tools/ant/taskdefs/optional/junit/JUnitTestRunner.java +++ b/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( diff --git a/src/main/org/apache/tools/ant/taskdefs/optional/junit/SummaryJUnitResultFormatter.java b/src/main/org/apache/tools/ant/taskdefs/optional/junit/SummaryJUnitResultFormatter.java index 89eef8cfe..5448c7b98 100644 --- a/src/main/org/apache/tools/ant/taskdefs/optional/junit/SummaryJUnitResultFormatter.java +++ b/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 {