Skip to content

Commit

Permalink
New -default-concurrency parameter to junit-runner
Browse files Browse the repository at this point in the history
Followon to https://rbcommons.com/s/twitter/r/3707/  this creates a new command line parameter for the JUnit runner that will allow us to more precisely specify the type of concurrency desired.

Testing Done:
Updated ConsoleRunnerTest and added two new test cases to show serial behavior for classes not annotated with @TestSerial

CI green at https://travis-ci.org/pantsbuild/pants/builds/125445433

Bugs closed: 3191, 3265

Reviewed at https://rbcommons.com/s/twitter/r/3753/
  • Loading branch information
ericzundel committed Apr 26, 2016
1 parent d5c18bd commit 94388b3
Show file tree
Hide file tree
Showing 7 changed files with 195 additions and 30 deletions.
24 changes: 24 additions & 0 deletions src/java/org/pantsbuild/tools/junit/impl/Concurrency.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package org.pantsbuild.tools.junit.impl;

public enum Concurrency {
SERIAL(false, false),
PARALLEL_CLASSES(true, false),
PARALLEL_METHODS(false, true),
PARALLEL_BOTH(true, true);

private final boolean parallelClasses;
private final boolean parallelMethods;

Concurrency(boolean parallelClasses, boolean parallelMethods) {
this.parallelClasses = parallelClasses;
this.parallelMethods = parallelMethods;
}

public boolean shouldRunClassesParallel() {
return parallelClasses;
}

public boolean shouldRunMethodsParallel() {
return parallelMethods;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,11 @@ public class ConcurrentCompositeRequest extends CompositeRequest {

private final ConcurrentRunnerScheduler runnerScheduler;

public ConcurrentCompositeRequest(List<Request> requests, boolean defaultParallel,
boolean parallelMethods, int numThreads)
public ConcurrentCompositeRequest(List<Request> requests, Concurrency defaultConcurrency,
int numThreads)
throws InitializationError {
super(requests);
this.runnerScheduler = new ConcurrentRunnerScheduler(defaultParallel, parallelMethods,
numThreads);
this.runnerScheduler = new ConcurrentRunnerScheduler(defaultConcurrency, numThreads);
setScheduler(runnerScheduler);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

package org.pantsbuild.tools.junit.impl;

import com.google.common.base.Preconditions;
import com.google.common.base.Throwables;
import com.google.common.util.concurrent.ThreadFactoryBuilder;
import java.util.LinkedList;
Expand All @@ -21,8 +22,7 @@ public class ConcurrentRunnerScheduler implements RunnerScheduler {
private final CompletionService<Void> completionService;
private final Queue<Future<Void>> concurrentTasks;
private final Queue<Runnable> serialTasks;
private final boolean defaultParallel;
private final boolean parallelMethods;
private final Concurrency defaultConcurrency;

/**
* A concurrent scheduler to run junit tests in parallel if possible, followed by tests that can
Expand All @@ -34,14 +34,13 @@ public class ConcurrentRunnerScheduler implements RunnerScheduler {
*
* Call {@link org.junit.runners.ParentRunner#setScheduler} to use this scheduler.
*
* @param defaultParallel whether to unannotated classes in parallel
* @param parallelMethods run individual test methods in parallel
* @param numThreads number of parallel threads to use, must be positive.
* @param defaultConcurrency Describes how to parallelize unannotated classes.
* @param numThreads Number of parallel threads to use, must be positive.
*/
public ConcurrentRunnerScheduler(boolean defaultParallel, boolean parallelMethods,
public ConcurrentRunnerScheduler(Concurrency defaultConcurrency,
int numThreads) {
this.defaultParallel = defaultParallel;
this.parallelMethods = parallelMethods;
Preconditions.checkNotNull(defaultConcurrency);
this.defaultConcurrency = defaultConcurrency;
ThreadFactory threadFactory = new ThreadFactoryBuilder()
.setDaemon(true)
.setNameFormat("concurrent-junit-runner-%d")
Expand Down Expand Up @@ -75,12 +74,13 @@ public void schedule(Runnable childStatement, Class<?> clazz) {

private boolean shouldMethodsRunParallel() {
// TODO(zundel): Add support for an annotation like TestParallelMethods.
return this.parallelMethods;
return defaultConcurrency.shouldRunMethodsParallel();
}

private boolean shouldClassRunParallel(Class<?> clazz) {
return !clazz.isAnnotationPresent(TestSerial.class)
&& (clazz.isAnnotationPresent(TestParallel.class) || this.defaultParallel);
&& (clazz.isAnnotationPresent(TestParallel.class) ||
defaultConcurrency.shouldRunClassesParallel());
}

@Override
Expand Down
55 changes: 44 additions & 11 deletions src/java/org/pantsbuild/tools/junit/impl/ConsoleRunnerImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

package org.pantsbuild.tools.junit.impl;

import com.google.common.annotations.VisibleForTesting;
import java.io.File;
import java.io.FileNotFoundException;
import java.io.FileOutputStream;
Expand Down Expand Up @@ -334,8 +335,7 @@ enum OutputMode {
private final boolean xmlReport;
private final File outdir;
private final boolean perTestTimer;
private final boolean defaultParallel;
private final boolean parallelMethods;
private final Concurrency defaultConcurrency;
private final int parallelThreads;
private final int testShard;
private final int numTestShards;
Expand All @@ -349,22 +349,26 @@ enum OutputMode {
boolean xmlReport,
boolean perTestTimer,
File outdir,
boolean defaultParallel,
boolean parallelMethods,
Concurrency defaultConcurrency,
int parallelThreads,
int testShard,
int numTestShards,
int numRetries,
PrintStream out,
PrintStream err) {

Preconditions.checkNotNull(outputMode);
Preconditions.checkNotNull(defaultConcurrency);
Preconditions.checkNotNull(out);
Preconditions.checkNotNull(err);

this.failFast = failFast;
this.outputMode = outputMode;
this.xmlReport = xmlReport;
this.perTestTimer = perTestTimer;
this.outdir = outdir;
this.defaultParallel = defaultParallel;
this.defaultConcurrency = defaultConcurrency;
this.parallelThreads = parallelThreads;
this.parallelMethods = parallelMethods;
this.testShard = testShard;
this.numTestShards = numTestShards;
this.numRetries = numRetries;
Expand Down Expand Up @@ -424,7 +428,7 @@ void run(Iterable<String> tests) {
try {
if (this.parallelThreads > 1) {
ConcurrentCompositeRequest request = new ConcurrentCompositeRequest(
requests, this.defaultParallel, this.parallelMethods, this.parallelThreads);
requests, this.defaultConcurrency, this.parallelThreads);
failures = core.run(request).getFailureCount();
} else {
for (Request request : requests) {
Expand Down Expand Up @@ -550,7 +554,7 @@ class TestMethod {

private boolean shouldRunParallelMethods(Class<?> clazz) {
// TODO(zundel): Add support for an annotation like TestParallelMethods.
return this.defaultParallel && this.parallelMethods;
return this.defaultConcurrency.shouldRunMethodsParallel();
}

// Loads classes without initializing them. We just need the type, annotations and method
Expand Down Expand Up @@ -665,9 +669,13 @@ class Options {
private boolean defaultParallel;

@Option(name = "-parallel-methods",
usage = "Run methods within a class in parallel.")
usage = "EXPERIMENTAL: Run methods within a class in parallel.")
private boolean parallelMethods;

@Option(name = "-default-concurrency",
usage = "Specify how to parallelize running tests.")
private Concurrency defaultConcurrency;

private int parallelThreads = 0;

@Option(name = "-parallel-threads",
Expand Down Expand Up @@ -744,14 +752,16 @@ public void setNumRetries(int numRetries) {
exit(1);
}

options.defaultConcurrency = computeConcurrencyOption(options.defaultConcurrency,
options.defaultParallel, options.parallelMethods);

ConsoleRunnerImpl runner =
new ConsoleRunnerImpl(options.failFast,
options.outputMode,
options.xmlReport,
options.perTestTimer,
options.outdir,
options.defaultParallel,
options.parallelMethods,
options.defaultConcurrency,
options.parallelThreads,
options.testShard,
options.numTestShards,
Expand All @@ -777,6 +787,29 @@ public void setNumRetries(int numRetries) {
runner.run(tests);
}

/**
* Used to convert the legacy -default-parallel and -parallel-methods options to the new
* style -default-concurrency values
*/
@VisibleForTesting
static Concurrency computeConcurrencyOption(Concurrency defaultConcurrency,
boolean defaultParallel, boolean parallelMethods) {

if (defaultConcurrency != null) {
// -default-concurrency option present - use it.
return defaultConcurrency;
}

// Fall Back to using -default-parallel and -parallel-methods
if (!defaultParallel) {
return Concurrency.SERIAL;
}
if (parallelMethods) {
return Concurrency.PARALLEL_BOTH;
}
return Concurrency.PARALLEL_CLASSES;
}

public static final Predicate<Constructor<?>> IS_PUBLIC_CONSTRUCTOR =
new Predicate<Constructor<?>>() {
@Override public boolean apply(Constructor<?> constructor) {
Expand Down
59 changes: 54 additions & 5 deletions tests/java/org/pantsbuild/tools/junit/impl/ConsoleRunnerTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import org.hamcrest.CoreMatchers;
import org.junit.Assert;
import org.junit.Test;
import org.pantsbuild.tools.junit.ConsoleRunner;
import org.pantsbuild.tools.junit.lib.FailingTestRunner;
import org.pantsbuild.tools.junit.lib.FlakyTest;
import org.pantsbuild.tools.junit.lib.MockTest4;
Expand Down Expand Up @@ -69,15 +70,17 @@ public void testShardedTesting23() {
@Test
public void testShardedTesting12WithParallelThreads() {
ConsoleRunnerImpl.main(asArgsArray(
"MockTest1 MockTest2 MockTest3 -test-shard 1/2 -parallel-threads 4 -default-parallel"));
"MockTest1 MockTest2 MockTest3 "
+ "-test-shard 1/2 -parallel-threads 4 -default-concurrency PARALLEL_CLASSES"));
assertEquals("test12 test21 test31", TestRegistry.getCalledTests());
}

@Test
public void testShardedTesting23WithParallelThreads() {
// This tests a corner case when no tests from MockTest2 are going to run.
ConsoleRunnerImpl.main(asArgsArray(
"MockTest1 MockTest2 MockTest3 -test-shard 2/3 -parallel-threads 3 -default-parallel"));
"MockTest1 MockTest2 MockTest3 "
+ "-test-shard 2/3 -parallel-threads 3 -default-concurrency PARALLEL_CLASSES"));
assertEquals("test13 test31", TestRegistry.getCalledTests());
}

Expand Down Expand Up @@ -138,7 +141,8 @@ public void testConsoleOutput() {
public void testOutputDir() throws Exception {
String outdir = temporary.newFolder("testOutputDir").getAbsolutePath();
ConsoleRunnerImpl.main(asArgsArray(
"MockTest4 -parallel-threads 1 -default-parallel -xmlreport -outdir " + outdir));
"MockTest4 -parallel-threads 1 -default-concurrency PARALLEL_CLASSES -xmlreport -outdir "
+ outdir));
Assert.assertEquals("test41 test42", TestRegistry.getCalledTests());

String testClassName = MockTest4.class.getCanonicalName();
Expand All @@ -158,25 +162,52 @@ public void testParallelAnnotation() throws Exception {
@Test
public void testSerialAnnotation() throws Exception {
ConsoleRunnerImpl.main(asArgsArray(
"AnnotatedSerialTest1 AnnotatedSerialTest2 -default-parallel -parallel-threads 2"));
"AnnotatedSerialTest1 AnnotatedSerialTest2 "
+ "-default-concurrency PARALLEL_CLASSES -parallel-threads 2"));
assertEquals("astest1 astest2", TestRegistry.getCalledTests());
}

/* LEGACY, remove after -default-parallel argument is removed */
@Test
public void testParallelDefaultParallel() throws Exception {
ConsoleRunnerImpl.main(asArgsArray(
"ParallelTest1 ParallelTest2 -parallel-threads 2 -default-parallel"));
assertEquals("ptest1 ptest2", TestRegistry.getCalledTests());
}

/* LEGACY, remove after -parallel-methods argument is removed */
@Test
public void testParallelMethodsDefaultParallel() throws Exception {
ConsoleRunnerImpl.main(asArgsArray(
"ParallelMethodsDefaultParallelTest1 ParallelMethodsDefaultParallelTest2"
+ " -parallel-methods -parallel-threads 4 -default-parallel"));
+ " -parallel-methods -parallel-threads 4 -default-parallel"));
assertEquals("pmdptest11 pmdptest12 pmdptest21 pmdptest22", TestRegistry.getCalledTests());
}

@Test
public void testConcurrencyParallelClasses() throws Exception {
ConsoleRunnerImpl.main(asArgsArray(
"ParallelTest1 ParallelTest2 -parallel-threads 2 -default-concurrency PARALLEL_CLASSES"));
assertEquals("ptest1 ptest2", TestRegistry.getCalledTests());
}

@Test
public void testConcurrencyParallelBoth() throws Exception {
ConsoleRunnerImpl.main(asArgsArray(
"ParallelMethodsDefaultParallelTest1 ParallelMethodsDefaultParallelTest2"
+ " -default-concurrency PARALLEL_BOTH -parallel-threads 4"));
assertEquals("pmdptest11 pmdptest12 pmdptest21 pmdptest22", TestRegistry.getCalledTests());
}

@Test
public void testConcurrencySerial() throws Exception {
ConsoleRunnerImpl.main(asArgsArray(
"SerialTest1 SerialTest2"
+ " -default-concurrency SERIAL -parallel-threads 4"));
assertEquals("stest1 stest2", TestRegistry.getCalledTests());

}

@Test
public void testXmlReportAll() throws Exception {
String testClassName = XmlReportTest.class.getCanonicalName();
Expand Down Expand Up @@ -400,4 +431,22 @@ public void testXmlErrorInTestRunnerInitialization() throws Exception {
assertThat(testCase.getError().getStacktrace(),
containsString(FailingTestRunner.class.getCanonicalName() + ".getTestRules("));
}

@Test
public void testLegacyConcurrencyOptions() {
// New style option overrides old
assertEquals(Concurrency.SERIAL,
ConsoleRunnerImpl.computeConcurrencyOption(Concurrency.SERIAL, true, true));
assertEquals(Concurrency.PARALLEL_CLASSES,
ConsoleRunnerImpl.computeConcurrencyOption(Concurrency.PARALLEL_CLASSES, false, false));

assertEquals(Concurrency.SERIAL,
ConsoleRunnerImpl.computeConcurrencyOption(null, false, false));
assertEquals(Concurrency.SERIAL,
ConsoleRunnerImpl.computeConcurrencyOption(null, false, true));
assertEquals(Concurrency.PARALLEL_BOTH,
ConsoleRunnerImpl.computeConcurrencyOption(null, true, true));
assertEquals(Concurrency.PARALLEL_CLASSES,
ConsoleRunnerImpl.computeConcurrencyOption(null, true, false));
}
}
43 changes: 43 additions & 0 deletions tests/java/org/pantsbuild/tools/junit/lib/SerialTest1.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// Copyright 2016 Pants project contributors (see CONTRIBUTORS.md).
// Licensed under the Apache License, Version 2.0 (see LICENSE).
package org.pantsbuild.tools.junit.lib;

import java.util.concurrent.atomic.AtomicBoolean;
import org.junit.Test;
import org.pantsbuild.junit.annotations.TestSerial;

import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;

/**
* This test is intentionally under a java_library() BUILD target so it will not be run
* on its own. It is run by the ConsoleRunnerTest suite to test ConsoleRunnerImpl.
* <p>
* This test is just like {@link AnnotatedSerialTest1} but without the annotation.
* Exercises the -default-concurrency SERIAL option
* <p>
* These tests are intended to show that the two classes will be run serially, even if
* parallel test running is on.
* To properly exercise this function, both test classes must be running at the same time with
* the option -default-concurrency SERIAL option when running with just these two classes as specs.
* <p>
* Uses a timeout, so its not completely deterministic, but it gives 3 seconds to allow any
* concurrency to take place.
* </p>
*/
public class SerialTest1 {
private static final int WAIT_TIMEOUT_MS = 3000;
private static AtomicBoolean waiting = new AtomicBoolean(false);

@Test
public void stest1() throws Exception {
awaitLatch("stest1");
}

static void awaitLatch(String methodName) throws Exception {
TestRegistry.registerTestCall(methodName);
assertFalse(waiting.getAndSet(true));
Thread.sleep(WAIT_TIMEOUT_MS);
assertTrue(waiting.getAndSet(false));
}
}
Loading

0 comments on commit 94388b3

Please sign in to comment.