Skip to content

Commit

Permalink
Rename PARALLEL_BOTH to PARALLEL_CLASSES_AND_METHODS inside JUnit Runner
Browse files Browse the repository at this point in the history
Feedback in https://rbcommons.com/s/twitter/r/3925/ was to rename PARALLEL_BOTH to prevent confusion if we were to add more concurrency options in the future.

This just changes the junit-runner java code so that we can publish a new jar and make the corresponding updates in pants python code.

Testing Done:
CI green at https://travis-ci.org/pantsbuild/pants/builds/134416147

Bugs closed: 3536

Reviewed at https://rbcommons.com/s/twitter/r/3962/
  • Loading branch information
ericzundel committed Jun 1, 2016
1 parent ec8530e commit 230cf16
Show file tree
Hide file tree
Showing 12 changed files with 33 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,5 @@
@Retention(RetentionPolicy.RUNTIME)
@Inherited
@Target(ElementType.TYPE)
public @interface TestParallelBoth {
public @interface TestParallelClassesAndMethods {
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,9 @@

/**
* Annotate that methods in this test class can be run in parallel. Note that this will not run in
* parallel with other classes. If you want that behavior too, specify {@link TestParallelBoth}.
* See usage note in {@code org.pantsbuild.tools.junit.impl.ConsoleRunnerImpl}. The
* parallel with other classes. If you want that behavior too, specify
* {@link TestParallelClassesAndMethods}. See usage note in
* {@code org.pantsbuild.tools.junit.impl.ConsoleRunnerImpl}. The
* {@link TestSerial} and {@link TestParallel} annotation takes precedence over this annotation
* if a class has multiple annotations (including via inheritance).
* <p>
Expand Down
4 changes: 2 additions & 2 deletions src/java/org/pantsbuild/junit/annotations/TestSerial.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@
* {@code org.pantsbuild.tools.junit.impl.ConsoleRunnerImpl}. This annotation takes precedence
* over a {@link TestParallel} annotation if a class has both (including via inheritance).
* <P>
* Note that this annotation is not currently compatible with the PARALLEL_METHODS or PARALLEL_BOTH
* default concurrency setting. See
* Note that this annotation is not currently compatible with the PARALLEL_METHODS or
* PARALLEL_CLASSES_AND_METHODS default concurrency setting. See
* <a href="https://github.com/pantsbuild/pants/issues/3209">issue 3209</a>
*/
@Retention(RetentionPolicy.RUNTIME)
Expand Down
2 changes: 1 addition & 1 deletion src/java/org/pantsbuild/tools/junit/impl/Concurrency.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ public enum Concurrency {
SERIAL(false, false),
PARALLEL_CLASSES(true, false),
PARALLEL_METHODS(false, true),
PARALLEL_BOTH(true, true);
PARALLEL_CLASSES_AND_METHODS(true, true);

private final boolean parallelClasses;
private final boolean parallelMethods;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Charsets;
import com.google.common.base.Preconditions;
import com.google.common.base.Predicate;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import com.google.common.collect.Sets;
Expand All @@ -22,12 +20,9 @@
import java.io.IOException;
import java.io.OutputStream;
import java.io.PrintStream;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.Comparator;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -474,7 +469,7 @@ private int runExperimental(List<Spec> parsedTests, JUnitCore core)
// Run all of the parallel tests using the ConcurrentComputer
// NB(zundel): This runs these test of each concurrency setting together and waits for them
// to finish. This introduces a bottleneck after each class of test.
failures += runConcurrentTests(core, filter, Concurrency.PARALLEL_BOTH);
failures += runConcurrentTests(core, filter, Concurrency.PARALLEL_CLASSES_AND_METHODS);
failures += runConcurrentTests(core, filter, Concurrency.PARALLEL_CLASSES);
failures += runConcurrentTests(core, filter, Concurrency.PARALLEL_METHODS);
}
Expand Down Expand Up @@ -682,7 +677,7 @@ class Options {

@Option(name = "-default-concurrency",
usage = "Specify how to parallelize running tests.\n"
+ "Use -use-experimental-runner for PARALLEL_METHODS and PARALLEL_BOTH")
+ "Use -use-experimental-runner for PARALLEL_METHODS and PARALLEL_CLASSES_AND_METHODS")
private Concurrency defaultConcurrency;

private int parallelThreads = 0;
Expand Down
6 changes: 3 additions & 3 deletions src/java/org/pantsbuild/tools/junit/impl/Spec.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import java.util.LinkedHashSet;
import java.util.Set;
import org.pantsbuild.junit.annotations.TestParallel;
import org.pantsbuild.junit.annotations.TestParallelBoth;
import org.pantsbuild.junit.annotations.TestParallelClassesAndMethods;
import org.pantsbuild.junit.annotations.TestParallelMethods;
import org.pantsbuild.junit.annotations.TestSerial;

Expand Down Expand Up @@ -50,8 +50,8 @@ public Concurrency getConcurrency(Concurrency defaultConcurrency) {
return Concurrency.PARALLEL_CLASSES;
} else if (clazz.isAnnotationPresent(TestParallelMethods.class)) {
return Concurrency.PARALLEL_METHODS;
} else if (clazz.isAnnotationPresent(TestParallelBoth.class)) {
return Concurrency.PARALLEL_BOTH;
} else if (clazz.isAnnotationPresent(TestParallelClassesAndMethods.class)) {
return Concurrency.PARALLEL_CLASSES_AND_METHODS;
}
return defaultConcurrency;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -196,9 +196,9 @@ public void testConcurrencyParallelClasses() throws Exception {
}

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

Expand Down Expand Up @@ -274,8 +274,9 @@ public void testLegacyConcurrencyOptions() {
ConsoleRunnerImpl.computeConcurrencyOption(Concurrency.PARALLEL_CLASSES, false));
assertEquals(Concurrency.PARALLEL_METHODS,
ConsoleRunnerImpl.computeConcurrencyOption(Concurrency.PARALLEL_METHODS, false));
assertEquals(Concurrency.PARALLEL_BOTH,
ConsoleRunnerImpl.computeConcurrencyOption(Concurrency.PARALLEL_BOTH, false));
assertEquals(Concurrency.PARALLEL_CLASSES_AND_METHODS,
ConsoleRunnerImpl.computeConcurrencyOption(Concurrency.PARALLEL_CLASSES_AND_METHODS,
false));

assertEquals(Concurrency.SERIAL,
ConsoleRunnerImpl.computeConcurrencyOption(null, false));
Expand Down
9 changes: 5 additions & 4 deletions tests/java/org/pantsbuild/tools/junit/impl/SpecSetTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import java.util.Set;
import org.junit.Test;
import org.pantsbuild.tools.junit.lib.AnnotationOverrideClass;
import org.pantsbuild.tools.junit.lib.ParallelBothAnnotatedClass;
import org.pantsbuild.tools.junit.lib.ParallelClassesAndMethodsAnnotatedClass;
import org.pantsbuild.tools.junit.lib.UnannotatedTestClass;

import static org.junit.Assert.assertEquals;
Expand All @@ -17,16 +17,17 @@ public class SpecSetTest {
@Test public void testExtractClasses() {
Spec dummyClassSpec = new Spec(UnannotatedTestClass.class);
Spec annotationOverrideSpec = new Spec(AnnotationOverrideClass.class);
Spec parallelBothSpec = new Spec(ParallelBothAnnotatedClass.class);
Spec parallelBothSpec = new Spec(ParallelClassesAndMethodsAnnotatedClass.class);

SpecSet specSet = new SpecSet(
ImmutableList.of(dummyClassSpec, annotationOverrideSpec, parallelBothSpec),
Concurrency.PARALLEL_CLASSES);

assertEquals(3, specSet.specs().size());
Class<?>[] parallelBothClasses = specSet.extract(Concurrency.PARALLEL_BOTH).classes();
Class<?>[] parallelBothClasses =
specSet.extract(Concurrency.PARALLEL_CLASSES_AND_METHODS).classes();
assertEquals(1, parallelBothClasses.length);
assertEquals(ParallelBothAnnotatedClass.class, parallelBothClasses[0]);
assertEquals(ParallelClassesAndMethodsAnnotatedClass.class, parallelBothClasses[0]);
assertEquals(2, specSet.specs().size());

assertEquals(0, specSet.extract(Concurrency.PARALLEL_METHODS).specs().size());
Expand Down
8 changes: 5 additions & 3 deletions tests/java/org/pantsbuild/tools/junit/impl/SpecTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -30,20 +30,22 @@ public class SpecTest {
assertEquals(Concurrency.SERIAL, spec.getConcurrency(Concurrency.SERIAL));
assertEquals(Concurrency.PARALLEL_CLASSES, spec.getConcurrency(Concurrency.PARALLEL_CLASSES));
assertEquals(Concurrency.PARALLEL_METHODS, spec.getConcurrency(Concurrency.PARALLEL_METHODS));
assertEquals(Concurrency.PARALLEL_BOTH, spec.getConcurrency(Concurrency.PARALLEL_BOTH));
assertEquals(Concurrency.PARALLEL_CLASSES_AND_METHODS,
spec.getConcurrency(Concurrency.PARALLEL_CLASSES_AND_METHODS));
}

@Test public void testAnnotatedConcurrency() {
Spec spec = new Spec(ParallelAnnotatedClass.class);
assertEquals(Concurrency.PARALLEL_CLASSES, spec.getConcurrency(Concurrency.PARALLEL_BOTH));
assertEquals(Concurrency.PARALLEL_CLASSES,
spec.getConcurrency(Concurrency.PARALLEL_CLASSES_AND_METHODS));
assertEquals(Concurrency.PARALLEL_CLASSES, spec.getConcurrency(Concurrency.PARALLEL_CLASSES));
assertEquals(Concurrency.PARALLEL_CLASSES, spec.getConcurrency(Concurrency.PARALLEL_METHODS));
assertEquals(Concurrency.PARALLEL_CLASSES, spec.getConcurrency(Concurrency.SERIAL));
}

@Test public void testAnnotationPrecedence() {
Spec spec = new Spec(AnnotationOverrideClass.class);
assertEquals(Concurrency.SERIAL, spec.getConcurrency(Concurrency.PARALLEL_BOTH));
assertEquals(Concurrency.SERIAL, spec.getConcurrency(Concurrency.PARALLEL_CLASSES_AND_METHODS));
assertEquals(Concurrency.SERIAL, spec.getConcurrency(Concurrency.PARALLEL_CLASSES));
assertEquals(Concurrency.SERIAL, spec.getConcurrency(Concurrency.PARALLEL_METHODS));
assertEquals(Concurrency.SERIAL, spec.getConcurrency(Concurrency.SERIAL));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
package org.pantsbuild.tools.junit.lib;

import org.pantsbuild.junit.annotations.TestParallel;
import org.pantsbuild.junit.annotations.TestParallelBoth;
import org.pantsbuild.junit.annotations.TestParallelClassesAndMethods;
import org.pantsbuild.junit.annotations.TestParallelMethods;
import org.pantsbuild.junit.annotations.TestSerial;

Expand All @@ -15,6 +15,6 @@
@TestParallel
@TestSerial
@TestParallelMethods
@TestParallelBoth
@TestParallelClassesAndMethods
public class AnnotationOverrideClass {
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
import org.pantsbuild.junit.annotations.TestSerial;
import org.pantsbuild.tools.junit.impl.Concurrency;
import org.pantsbuild.tools.junit.impl.ConsoleRunnerImpl;
import org.pantsbuild.tools.junit.lib.TestRegistry;

@TestSerial
@RunWith(Parameterized.class)
Expand All @@ -37,7 +36,7 @@ protected enum TestParameters {
EXPERIMENTAL_SERIAL(true, Concurrency.SERIAL),
EXPERIMENTAL_PARALLEL_CLASSES(true, Concurrency.PARALLEL_CLASSES),
EXPERIMENTAL_PARALLEL_METHODS(true, Concurrency.PARALLEL_METHODS),
EXPERIMENTAL_PARALLEL_BOTH(true, Concurrency.PARALLEL_BOTH);
EXPERIMENTAL_PARALLEL_CLASSES_AND_METHODS(true, Concurrency.PARALLEL_CLASSES_AND_METHODS);

public final boolean useExperimentalRunner;
public final Concurrency defaultConcurrency;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@

package org.pantsbuild.tools.junit.lib;

import org.pantsbuild.junit.annotations.TestParallelBoth;
import org.pantsbuild.junit.annotations.TestParallelClassesAndMethods;

/**
* 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.
*/
@TestParallelBoth
public class ParallelBothAnnotatedClass {
@TestParallelClassesAndMethods
public class ParallelClassesAndMethodsAnnotatedClass {
}

0 comments on commit 230cf16

Please sign in to comment.