From a63079d676e50887fc49bd1239bcf27ecfb50b29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ce=CC=81sar=20Puerta?= Date: Mon, 27 Feb 2017 11:15:44 -0800 Subject: [PATCH] Support the selection of a default Runner in ClassRequest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch updates `Request` and `ClassRequest` to enable the configuration of a default JUnit4 Runner other than `BlockJUnit4ClassRunner`. The only visible change is a new static method in Request, `classWithDefaultRunner`, which configures the runner class to use if not configured in the test. The context of this change is the following: when managing a large test suite that gets contributions from a large development team, it's typical to enforce certain logic across all tests for global state initialization and cleanup. In the particular case of Twitter for Android, all our tests set up global application graphs before test execution and reset static state afterwards. It is critical for correctness that every single test follows this pattern. This seems to be a particular example of the general need for a mechanism that configures the global state of the test environment. This is typically achieved by forcing all tests to use a single shared runner or to extend a base class. However, these approaches are error prone, since they require dilligence for setting up the test correctly, or an external validation (eg. a checkstyle detector). A default test runner is a low-tech solution to this problem: a test runner extending `BlockJUnit4ClassRunner` implements state management logic and is configured to be used with all tests that don't explicitly say otherwise. The change is incomplete and doesn't implement tests, and it's meant a small proof of concept for feedback. --- .../AllDefaultPossibilitiesBuilder.java | 5 +++++ .../internal/builders/AnnotatedBuilder.java | 21 +------------------ .../internal/builders/DefaultBuilder.java | 18 ++++++++++++++++ .../junit/internal/requests/ClassRequest.java | 18 +++++++++++++++- src/main/java/org/junit/runner/Request.java | 13 ++++++++++++ .../junit/runners/model/RunnerBuilder.java | 18 ++++++++++++++++ 6 files changed, 72 insertions(+), 21 deletions(-) create mode 100644 src/main/java/org/junit/internal/builders/DefaultBuilder.java diff --git a/src/main/java/org/junit/internal/builders/AllDefaultPossibilitiesBuilder.java b/src/main/java/org/junit/internal/builders/AllDefaultPossibilitiesBuilder.java index 8704a546b5f2..7d2c0848022c 100644 --- a/src/main/java/org/junit/internal/builders/AllDefaultPossibilitiesBuilder.java +++ b/src/main/java/org/junit/internal/builders/AllDefaultPossibilitiesBuilder.java @@ -31,6 +31,7 @@ public Runner runnerForClass(Class testClass) throws Throwable { annotatedBuilder(), suiteMethodBuilder(), junit3Builder(), + defaultBuilder(), junit4Builder()); for (RunnerBuilder each : builders) { @@ -58,6 +59,10 @@ protected IgnoredBuilder ignoredBuilder() { return new IgnoredBuilder(); } + protected RunnerBuilder defaultBuilder() { + return new NullBuilder(); + } + protected RunnerBuilder suiteMethodBuilder() { if (canUseSuiteMethod) { return new SuiteMethodBuilder(); diff --git a/src/main/java/org/junit/internal/builders/AnnotatedBuilder.java b/src/main/java/org/junit/internal/builders/AnnotatedBuilder.java index 04d7a683652f..3c163c7535a5 100644 --- a/src/main/java/org/junit/internal/builders/AnnotatedBuilder.java +++ b/src/main/java/org/junit/internal/builders/AnnotatedBuilder.java @@ -2,7 +2,6 @@ import org.junit.runner.RunWith; import org.junit.runner.Runner; -import org.junit.runners.model.InitializationError; import org.junit.runners.model.RunnerBuilder; import java.lang.reflect.Modifier; @@ -69,8 +68,6 @@ * @since 4.0 */ public class AnnotatedBuilder extends RunnerBuilder { - private static final String CONSTRUCTOR_ERROR_FORMAT = "Custom runner class %s should have a public constructor with signature %s(Class testClass)"; - private final RunnerBuilder suiteBuilder; public AnnotatedBuilder(RunnerBuilder suiteBuilder) { @@ -83,7 +80,7 @@ public Runner runnerForClass(Class testClass) throws Exception { currentTestClass = getEnclosingClassForNonStaticMemberClass(currentTestClass)) { RunWith annotation = currentTestClass.getAnnotation(RunWith.class); if (annotation != null) { - return buildRunner(annotation.value(), testClass); + return buildRunner(annotation.value(), testClass, suiteBuilder); } } @@ -97,20 +94,4 @@ private Class getEnclosingClassForNonStaticMemberClass(Class currentTestCl return null; } } - - public Runner buildRunner(Class runnerClass, - Class testClass) throws Exception { - try { - return runnerClass.getConstructor(Class.class).newInstance(testClass); - } catch (NoSuchMethodException e) { - try { - return runnerClass.getConstructor(Class.class, - RunnerBuilder.class).newInstance(testClass, suiteBuilder); - } catch (NoSuchMethodException e2) { - String simpleName = runnerClass.getSimpleName(); - throw new InitializationError(String.format( - CONSTRUCTOR_ERROR_FORMAT, simpleName, simpleName)); - } - } - } } \ No newline at end of file diff --git a/src/main/java/org/junit/internal/builders/DefaultBuilder.java b/src/main/java/org/junit/internal/builders/DefaultBuilder.java new file mode 100644 index 000000000000..2bab5e7f8d17 --- /dev/null +++ b/src/main/java/org/junit/internal/builders/DefaultBuilder.java @@ -0,0 +1,18 @@ +package org.junit.internal.builders; + +import org.junit.runner.Runner; +import org.junit.runners.model.RunnerBuilder; + +public class DefaultBuilder extends RunnerBuilder { + private Class defaultRunnerClass; + private RunnerBuilder suiteBuilder; + + public DefaultBuilder(Class defaultRunnerClass, RunnerBuilder suiteBuilder) { + this.defaultRunnerClass = defaultRunnerClass; + this.suiteBuilder = suiteBuilder; + } + + public Runner runnerForClass(Class testClass) throws Throwable { + return buildRunner(defaultRunnerClass, testClass, suiteBuilder); + } +} diff --git a/src/main/java/org/junit/internal/requests/ClassRequest.java b/src/main/java/org/junit/internal/requests/ClassRequest.java index 9000e2423b9f..d7c668599612 100644 --- a/src/main/java/org/junit/internal/requests/ClassRequest.java +++ b/src/main/java/org/junit/internal/requests/ClassRequest.java @@ -1,6 +1,7 @@ package org.junit.internal.requests; import org.junit.internal.builders.AllDefaultPossibilitiesBuilder; +import org.junit.internal.builders.DefaultBuilder; import org.junit.internal.builders.SuiteMethodBuilder; import org.junit.runner.Request; import org.junit.runner.Runner; @@ -17,10 +18,17 @@ public class ClassRequest extends Request { private final Class fTestClass; private final boolean canUseSuiteMethod; private volatile Runner runner; + private Class defaultRunnerClass; - public ClassRequest(Class testClass, boolean canUseSuiteMethod) { + public ClassRequest(Class testClass, boolean canUseSuiteMethod, + Class defaultRunnerClass) { this.fTestClass = testClass; this.canUseSuiteMethod = canUseSuiteMethod; + this.defaultRunnerClass = defaultRunnerClass; + } + + public ClassRequest(Class testClass, boolean canUseSuiteMethod) { + this(testClass, canUseSuiteMethod, null); } public ClassRequest(Class testClass) { @@ -45,6 +53,14 @@ private class CustomAllDefaultPossibilitiesBuilder extends AllDefaultPossibiliti protected RunnerBuilder suiteMethodBuilder() { return new CustomSuiteMethodBuilder(); } + + @Override + protected RunnerBuilder defaultBuilder() { + if (defaultRunnerClass != null) { + return new DefaultBuilder(defaultRunnerClass, this); + } + return super.defaultBuilder(); + } } /* diff --git a/src/main/java/org/junit/runner/Request.java b/src/main/java/org/junit/runner/Request.java index 264489217f74..b15dd29fdf88 100644 --- a/src/main/java/org/junit/runner/Request.java +++ b/src/main/java/org/junit/runner/Request.java @@ -61,6 +61,19 @@ public static Request classWithoutSuiteMethod(Class clazz) { return new ClassRequest(clazz, false); } + /** + * Create a Request that, when processed, will run all the tests + * in a class. If the class does not set the runner to use, a runner of the given + * class will be used by default. + * + * @param clazz the class containing the tests + * @param defaultRunnerClass the class of the runner to use by default + * @return a Request that will cause all tests in the class to be run + */ + public static Request classWithDefaultRunner(Class clazz, Class defaultRunnerClass) { + return new ClassRequest(clazz, true, defaultRunnerClass); + } + /** * Create a Request that, when processed, will run all the tests * in a set of classes. diff --git a/src/main/java/org/junit/runners/model/RunnerBuilder.java b/src/main/java/org/junit/runners/model/RunnerBuilder.java index bc6f85f04813..6396a6b85adf 100644 --- a/src/main/java/org/junit/runners/model/RunnerBuilder.java +++ b/src/main/java/org/junit/runners/model/RunnerBuilder.java @@ -37,6 +37,8 @@ * @since 4.5 */ public abstract class RunnerBuilder { + private static final String CONSTRUCTOR_ERROR_FORMAT = "Custom runner class %s should have a public constructor with signature %s(Class testClass)"; + private final Set> parents = new HashSet>(); /** @@ -80,6 +82,22 @@ void removeParent(Class klass) { parents.remove(klass); } + public Runner buildRunner(Class runnerClass, + Class testClass, RunnerBuilder suiteBuilder) throws Exception { + try { + return runnerClass.getConstructor(Class.class).newInstance(testClass); + } catch (NoSuchMethodException e) { + try { + return runnerClass.getConstructor(Class.class, + RunnerBuilder.class).newInstance(testClass, suiteBuilder); + } catch (NoSuchMethodException e2) { + String simpleName = runnerClass.getSimpleName(); + throw new InitializationError(String.format( + CONSTRUCTOR_ERROR_FORMAT, simpleName, simpleName)); + } + } + } + /** * Constructs and returns a list of Runners, one for each child class in * {@code children}. Care is taken to avoid infinite recursion: