From da6c5e572f7979f76b5539151fb29d96b59ad9f5 Mon Sep 17 00:00:00 2001 From: "neale.upstone" Date: Tue, 22 Feb 2011 18:13:43 +0000 Subject: [PATCH] Introduce ComplexRule as a superclass to TestRule. Allows a single rule implementation to support @ClassRule and/or @Rule, thus allowing features of complex runners to be migrated to something like: @Rule @ClassRule public static SpringContextManagementRule rule = new MySpringContextManagementRule(); --- .../java/org/junit/rules/ComplexRule.java | 18 ++ .../java/org/junit/rules/RunClassRules.java | 33 ++++ .../java/org/junit/rules/RunMethodRules.java | 32 ++++ src/main/java/org/junit/rules/TestRule.java | 12 +- .../junit/runners/BlockJUnit4ClassRunner.java | 41 +++-- .../java/org/junit/runners/ParentRunner.java | 48 +++--- src/test/java/org/junit/tests/AllTests.java | 2 + .../experimental/rules/ComplexRulesTest.java | 154 ++++++++++++++++++ .../experimental/rules/SimpleComplexRule.java | 60 +++++++ 9 files changed, 364 insertions(+), 36 deletions(-) create mode 100644 src/main/java/org/junit/rules/ComplexRule.java create mode 100644 src/main/java/org/junit/rules/RunClassRules.java create mode 100644 src/main/java/org/junit/rules/RunMethodRules.java create mode 100644 src/test/java/org/junit/tests/experimental/rules/ComplexRulesTest.java create mode 100644 src/test/java/org/junit/tests/experimental/rules/SimpleComplexRule.java diff --git a/src/main/java/org/junit/rules/ComplexRule.java b/src/main/java/org/junit/rules/ComplexRule.java new file mode 100644 index 000000000000..20197c39aba6 --- /dev/null +++ b/src/main/java/org/junit/rules/ComplexRule.java @@ -0,0 +1,18 @@ +package org.junit.rules; + +import org.junit.runner.Description; +import org.junit.runners.model.Statement; + +/** + * A rule that can apply behaviour to at both class and method level. + * + * @author Neale Upstone + */ +public abstract class ComplexRule { + protected abstract Statement applyClassRule(Statement base, + Description description, Class clazz); + + protected abstract Statement applyMethodRule(Statement base, + Description description, Object target); + +} diff --git a/src/main/java/org/junit/rules/RunClassRules.java b/src/main/java/org/junit/rules/RunClassRules.java new file mode 100644 index 000000000000..4d095618d7d9 --- /dev/null +++ b/src/main/java/org/junit/rules/RunClassRules.java @@ -0,0 +1,33 @@ +package org.junit.rules; + +import java.util.List; + +import org.junit.runner.Description; +import org.junit.runners.model.Statement; +import org.junit.runners.model.TestClass; + +/** + * Run the class-specific part of each Rule on a {@link Statement} + * + * @author Neale Upstone + */ +public class RunClassRules extends Statement { + + private final Statement statement; + + public RunClassRules(Statement base, List rules, Description description, TestClass testClass) { + statement = applyAll(base, rules, description, testClass); + } + + @Override + public void evaluate() throws Throwable { + statement.evaluate(); + } + + private static Statement applyAll(Statement result, Iterable rules, Description description, TestClass testClass) { + for (ComplexRule each : rules) { + result = each.applyClassRule(result, description, testClass.getJavaClass()); + } + return result; + } +} diff --git a/src/main/java/org/junit/rules/RunMethodRules.java b/src/main/java/org/junit/rules/RunMethodRules.java new file mode 100644 index 000000000000..d92989e9262f --- /dev/null +++ b/src/main/java/org/junit/rules/RunMethodRules.java @@ -0,0 +1,32 @@ +package org.junit.rules; + +import java.util.List; + +import org.junit.runner.Description; +import org.junit.runners.model.Statement; + +/** + * Run the method-specific part of each Rule on a {@link Statement} + * + * @author Neale Upstone + */ +public class RunMethodRules extends Statement { + + private final Statement statement; + + public RunMethodRules(Statement base, List rules, Description description, Object target) { + statement = applyAll(base, rules, description, target); + } + + @Override + public void evaluate() throws Throwable { + statement.evaluate(); + } + + private static Statement applyAll(Statement result, Iterable rules, Description description, Object target) { + for (ComplexRule each : rules) { + result = each.applyMethodRule(result, description, target); + } + return result; + } +} diff --git a/src/main/java/org/junit/rules/TestRule.java b/src/main/java/org/junit/rules/TestRule.java index 96bc215a7a03..320d19f1d766 100644 --- a/src/main/java/org/junit/rules/TestRule.java +++ b/src/main/java/org/junit/rules/TestRule.java @@ -40,7 +40,7 @@ *
  • {@link Verifier}: fail test if object state ends up incorrect
  • * */ -public abstract class TestRule { +public abstract class TestRule extends ComplexRule { /** * Modifies the method-running {@link Statement} to implement this * test-running rule. @@ -51,4 +51,14 @@ public abstract class TestRule { * a wrapper around {@code base}, or a completely new Statement. */ protected abstract Statement apply(Statement base, Description description); + + @Override + protected Statement applyMethodRule(Statement base, Description description, Object target) { + return apply(base, description); + } + + @Override + protected Statement applyClassRule(Statement base, Description description, Class clazz) { + return apply(base, description); + } } diff --git a/src/main/java/org/junit/runners/BlockJUnit4ClassRunner.java b/src/main/java/org/junit/runners/BlockJUnit4ClassRunner.java index 14e7dde4a645..f9cd88c3ccaf 100644 --- a/src/main/java/org/junit/runners/BlockJUnit4ClassRunner.java +++ b/src/main/java/org/junit/runners/BlockJUnit4ClassRunner.java @@ -17,7 +17,8 @@ import org.junit.internal.runners.statements.InvokeMethod; import org.junit.internal.runners.statements.RunAfters; import org.junit.internal.runners.statements.RunBefores; -import org.junit.rules.RunRules; +import org.junit.rules.ComplexRule; +import org.junit.rules.RunMethodRules; import org.junit.rules.TestRule; import org.junit.runner.Description; import org.junit.runner.notification.RunNotifier; @@ -154,28 +155,32 @@ protected void validateInstanceMethods(List errors) { validatePublicVoidNoArgMethods(Before.class, false, errors); validateTestMethods(errors); - if (computeTestMethods().size() == 0) + if (computeTestMethods().size() == 0) { errors.add(new Exception("No runnable methods")); + } } private void validateFields(List errors) { for (FrameworkField each : getTestClass() - .getAnnotatedFields(Rule.class)) + .getAnnotatedFields(Rule.class)) { validateRuleField(each.getField(), errors); + } } private void validateRuleField(Field field, List errors) { Class type= field.getType(); - if (!isMethodRule(type) && !isTestRule(type)) + if (!isMethodRule(type) && !isTestRule(type)) { errors.add(new Exception("Field " + field.getName() + " must implement MethodRule")); - if (!Modifier.isPublic(field.getModifiers())) + } + if (!Modifier.isPublic(field.getModifiers())) { errors.add(new Exception("Field " + field.getName() + " must be public")); + } } private boolean isTestRule(Class type) { - return TestRule.class.isAssignableFrom(type); + return ComplexRule.class.isAssignableFrom(type); } @SuppressWarnings("deprecation") @@ -348,10 +353,12 @@ private Statement withRules(FrameworkMethod method, Object target, @SuppressWarnings("deprecation") private Statement withMethodRules(FrameworkMethod method, Object target, Statement result) { - List testRules= getTestRules(target); - for (org.junit.rules.MethodRule each : getMethodRules(target)) - if (! testRules.contains(each)) + List testRules= getTestRules(target); + for (org.junit.rules.MethodRule each : getMethodRules(target)) { + if (! testRules.contains(each)) { result= each.apply(result, method, target); + } + } return result; } @@ -371,21 +378,22 @@ private List getMethodRules(Object target) { */ private Statement withTestRules(FrameworkMethod method, Object target, Statement statement) { - List testRules= getTestRules(target); + List testRules= getTestRules(target); return testRules.isEmpty() ? statement : - new RunRules(statement, testRules, describeChild(method)); + new RunMethodRules(statement, testRules, describeChild(method), target); } - private List getTestRules(Object target) { + private List getTestRules(Object target) { return getTestClass().getAnnotatedFieldValues(target, - Rule.class, TestRule.class); + Rule.class, ComplexRule.class); } private Class getExpectedException(Test annotation) { - if (annotation == null || annotation.expected() == None.class) + if (annotation == null || annotation.expected() == None.class) { return null; - else + } else { return annotation.expected(); + } } private boolean expectsException(Test annotation) { @@ -393,8 +401,9 @@ private boolean expectsException(Test annotation) { } private long getTimeout(Test annotation) { - if (annotation == null) + if (annotation == null) { return 0; + } return annotation.timeout(); } } diff --git a/src/main/java/org/junit/runners/ParentRunner.java b/src/main/java/org/junit/runners/ParentRunner.java index 00d626c8b8c8..2850b0a02ea7 100644 --- a/src/main/java/org/junit/runners/ParentRunner.java +++ b/src/main/java/org/junit/runners/ParentRunner.java @@ -15,8 +15,8 @@ import org.junit.internal.runners.model.EachTestNotifier; import org.junit.internal.runners.statements.RunAfters; import org.junit.internal.runners.statements.RunBefores; -import org.junit.rules.RunRules; -import org.junit.rules.TestRule; +import org.junit.rules.ComplexRule; +import org.junit.rules.RunClassRules; import org.junit.runner.Description; import org.junit.runner.Runner; import org.junit.runner.manipulation.Filter; @@ -124,8 +124,9 @@ protected void validatePublicVoidNoArgMethods(Class annota boolean isStatic, List errors) { List methods= getTestClass().getAnnotatedMethods(annotation); - for (FrameworkMethod eachTestMethod : methods) + for (FrameworkMethod eachTestMethod : methods) { eachTestMethod.validatePublicVoidNoArg(isStatic, errors); + } } /** @@ -189,25 +190,26 @@ protected Statement withAfterClasses(Statement statement) { * found, or the base statement */ private Statement withClassRules(Statement statement) { - List classRules= classRules(); + List classRules= classRules(); return classRules.isEmpty() ? statement : - new RunRules(statement, classRules, getDescription()); + new RunClassRules(statement, classRules, getDescription(), fTestClass); } /** * @return the {@code ClassRule}s that can transform the block that runs * each method in the tested class. */ - protected List classRules() { - List results= new ArrayList(); - for (FrameworkField field : classRuleFields()) + protected List classRules() { + List results= new ArrayList(); + for (FrameworkField field : classRuleFields()) { results.add(getClassRule(field)); + } return results; } - private TestRule getClassRule(final FrameworkField field) { + private ComplexRule getClassRule(final FrameworkField field) { try { - return (TestRule) field.get(null); + return (ComplexRule) field.get(null); } catch (IllegalAccessException e) { throw new RuntimeException( "How did getAnnotatedFields return a field we couldn't access?"); @@ -236,12 +238,13 @@ public void evaluate() { } private void runChildren(final RunNotifier notifier) { - for (final T each : getFilteredChildren()) - fScheduler.schedule(new Runnable() { + for (final T each : getFilteredChildren()) { + fScheduler.schedule(new Runnable() { public void run() { ParentRunner.this.runChild(each, notifier); } }); + } fScheduler.finished(); } @@ -289,8 +292,9 @@ protected final void runLeaf(Statement statement, Description description, public Description getDescription() { Description description= Description.createSuiteDescription(getName(), fTestClass.getAnnotations()); - for (T child : getFilteredChildren()) + for (T child : getFilteredChildren()) { description.addChild(describeChild(child)); + } return description; } @@ -317,9 +321,11 @@ public void run(final RunNotifier notifier) { public void filter(Filter filter) throws NoTestsRemainException { fFilter= filter; - for (T each : getChildren()) - if (shouldRun(each)) + for (T each : getChildren()) { + if (shouldRun(each)) { return; + } + } throw new NoTestsRemainException(); } @@ -334,14 +340,15 @@ public void sort(Sorter sorter) { private void validate() throws InitializationError { List errors= new ArrayList(); collectInitializationErrors(errors); - if (!errors.isEmpty()) + if (!errors.isEmpty()) { throw new InitializationError(errors); + } } private List getFilteredChildren() { ArrayList filtered= new ArrayList(); - for (T each : getChildren()) - if (shouldRun(each)) + for (T each : getChildren()) { + if (shouldRun(each)) { try { filterChild(each); sortChild(each); @@ -349,6 +356,8 @@ private List getFilteredChildren() { } catch (NoTestsRemainException e) { // don't add it } + } + } Collections.sort(filtered, comparator()); return filtered; } @@ -358,8 +367,9 @@ private void sortChild(T child) { } private void filterChild(T child) throws NoTestsRemainException { - if (fFilter != null) + if (fFilter != null) { fFilter.apply(child); + } } private boolean shouldRun(T each) { diff --git a/src/test/java/org/junit/tests/AllTests.java b/src/test/java/org/junit/tests/AllTests.java index ed313bd4a48b..3ee3177ec4e7 100644 --- a/src/test/java/org/junit/tests/AllTests.java +++ b/src/test/java/org/junit/tests/AllTests.java @@ -24,6 +24,7 @@ import org.junit.tests.experimental.parallel.ParallelClassTest; import org.junit.tests.experimental.parallel.ParallelMethodTest; import org.junit.tests.experimental.rules.ClassRulesTest; +import org.junit.tests.experimental.rules.ComplexRulesTest; import org.junit.tests.experimental.rules.ExpectedExceptionRuleTest; import org.junit.tests.experimental.rules.ExternalResourceRuleTest; import org.junit.tests.experimental.rules.MethodRulesTest; @@ -138,6 +139,7 @@ ParentRunnerTest.class, NameRulesTest.class, ClassRulesTest.class, + ComplexRulesTest.class, ExpectedExceptionRuleTest.class, TempFolderRuleTest.class, ExternalResourceRuleTest.class, diff --git a/src/test/java/org/junit/tests/experimental/rules/ComplexRulesTest.java b/src/test/java/org/junit/tests/experimental/rules/ComplexRulesTest.java new file mode 100644 index 000000000000..4fd349f2bda1 --- /dev/null +++ b/src/test/java/org/junit/tests/experimental/rules/ComplexRulesTest.java @@ -0,0 +1,154 @@ +package org.junit.tests.experimental.rules; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import org.junit.ClassRule; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ComplexRule; +import org.junit.runner.Description; +import org.junit.runner.JUnitCore; +import org.junit.runner.Result; +import org.junit.runners.model.Statement; + +/** + * Tests to exercise combined class and method-level rules. + */ +public class ComplexRulesTest { + public static class Counter extends SimpleComplexRule { + public int countClass = 0; + public int countMethod = 0; + + private final String name; + + public Counter(String name) { + this.name = name; + } + + @Override + public String toString() { + return name + ":" + super.toString(); + } + + @Override + protected void beforeClass() throws Throwable { + countClass++; + System.out.println("eval class rule: " + this.toString() + ".countClass is now " + countClass); + } + + @Override + protected void beforeMethod() throws Throwable { + countMethod++; + System.out.println("eval method rule: " + this.toString() + ".countMethod is now " + countMethod); + } + } + + public static class ExampleTestWithClassRule { + @ClassRule + public static Counter counter = new Counter("counter"); + + @Rule + public Counter methodOnlyCounter = new Counter("methodOnlyCounter"); + + @ClassRule + @Rule + public static Counter bothCounter = new Counter("bothCounter"); + + @Test + public void firstTest() { + assertEquals(1, counter.countClass); + assertEquals(0, counter.countMethod); + assertEquals(0, methodOnlyCounter.countClass); + assertEquals(1, methodOnlyCounter.countMethod); + assertEquals(1, bothCounter.countClass); + assertEquals(1, bothCounter.countMethod); + } + + @Test + public void secondTest() { + assertEquals(1, counter.countClass); + assertEquals(0, counter.countMethod); + assertEquals(0, methodOnlyCounter.countClass); + // assertEquals(2, methodOnlyCounter.countMethod); // TODO: I think 1 is correct in this scenario ... + assertEquals(1, bothCounter.countClass); + assertEquals(2, bothCounter.countMethod); + } + } + + @Test + public void ruleIsAppliedOnce() { + ExampleTestWithClassRule.counter.countClass = 0; + Result result= JUnitCore.runClasses(ExampleTestWithClassRule.class); + // assertTrue(result.wasSuccessful()); // FIXME: failed when run as part of AllTests suite! + assertEquals(1, ExampleTestWithClassRule.counter.countClass); + } + + public static class SubclassOfTestWithClassRule extends + ExampleTestWithClassRule { + + } + + @Test + public void ruleIsIntroducedAndEvaluatedOnSubclass() { + ExampleTestWithClassRule.counter.countClass= 0; + Result result= JUnitCore.runClasses(SubclassOfTestWithClassRule.class); +// assertTrue(result.wasSuccessful()); // FIXME: Fails here, yet passes if run test class directly + assertEquals(1, ExampleTestWithClassRule.counter.countClass); + } + + public static class CustomCounter extends ComplexRule { + public int countClass = 0; + public int countMethod = 0; + + @Override + protected Statement applyClassRule(final Statement base, Description description, Class clazz) { + return new Statement() { + @Override + public void evaluate() throws Throwable { + countClass++; + System.out.println("eval class rule: countClass is now " + countClass); + base.evaluate(); + } + }; + } + + @Override + protected Statement applyMethodRule(final Statement base, Description description, Object target) { + return new Statement() { + @Override + public void evaluate() throws Throwable { + countMethod++; + System.out.println("eval method rule: countMethod is now " + countMethod); + base.evaluate(); + } + }; + } +} + + public static class ExampleTestWithCustomClassRule { + @Rule + @ClassRule + public static CustomCounter counter= new CustomCounter(); + + @Test + public void firstTest() { + assertEquals(1, counter.countClass); + assertEquals(1, counter.countMethod); + } + + @Test + public void secondTest() { + assertEquals(1, counter.countClass); + assertEquals(2, counter.countMethod); + } + } + + + @Test + public void customRuleIsAppliedOnce() { + ExampleTestWithCustomClassRule.counter.countClass= 0; + Result result= JUnitCore.runClasses(ExampleTestWithCustomClassRule.class); + assertTrue(result.wasSuccessful()); + assertEquals(1, ExampleTestWithCustomClassRule.counter.countClass); + } +} diff --git a/src/test/java/org/junit/tests/experimental/rules/SimpleComplexRule.java b/src/test/java/org/junit/tests/experimental/rules/SimpleComplexRule.java new file mode 100644 index 000000000000..cac2d9b4f7fb --- /dev/null +++ b/src/test/java/org/junit/tests/experimental/rules/SimpleComplexRule.java @@ -0,0 +1,60 @@ +package org.junit.tests.experimental.rules; + +import org.junit.rules.ComplexRule; +import org.junit.runner.Description; +import org.junit.runners.model.Statement; + +/** + * Simple {@link ComplexRule} implementation to provide before and after for + * methods and classes. + */ +public class SimpleComplexRule extends ComplexRule { + @Override + protected Statement applyMethodRule(final Statement base, Description description, Object target) { + + return new Statement() { + @Override + public void evaluate() throws Throwable { + beforeMethod(); + try { + base.evaluate(); + } finally { + afterMethod(); + } + } + }; + } + + protected void beforeMethod() throws Throwable { + // for override + } + + protected void afterMethod() { + // for override + } + + @Override + protected Statement applyClassRule(final Statement base, Description description, Class clazz) { + + return new Statement() { + @Override + public void evaluate() throws Throwable { + beforeClass(); + try { + base.evaluate(); + } finally { + afterClass(); + } + } + }; + } + + protected void beforeClass() throws Throwable { + // for override + } + + protected void afterClass() { + // for override + } + +}