diff --git a/src/main/java/org/junit/internal/runners/rules/RuleMemberValidator.java b/src/main/java/org/junit/internal/runners/rules/RuleMemberValidator.java index 36de4f106297..bd56273928cf 100644 --- a/src/main/java/org/junit/internal/runners/rules/RuleMemberValidator.java +++ b/src/main/java/org/junit/internal/runners/rules/RuleMemberValidator.java @@ -34,7 +34,6 @@ public class RuleMemberValidator { .withValidator(new DeclaringClassMustBePublic()) .withValidator(new MemberMustBeStatic()) .withValidator(new MemberMustBePublic()) - .withValidator(new FieldMustBeATestRule()) .build(); /** * Validates fields with a {@link Rule} annotation. @@ -43,7 +42,6 @@ public class RuleMemberValidator { testRuleValidatorBuilder() .withValidator(new MemberMustBeNonStaticOrAlsoClassRule()) .withValidator(new MemberMustBePublic()) - .withValidator(new FieldMustBeARule()) .build(); /** * Validates methods with a {@link ClassRule} annotation. diff --git a/src/main/java/org/junit/internal/runners/rules/ValidationError.java b/src/main/java/org/junit/internal/runners/rules/ValidationError.java index 31bd66019e09..a6eea4c0aafa 100644 --- a/src/main/java/org/junit/internal/runners/rules/ValidationError.java +++ b/src/main/java/org/junit/internal/runners/rules/ValidationError.java @@ -4,7 +4,7 @@ import java.lang.annotation.Annotation; -class ValidationError extends Exception { +public class ValidationError extends Exception { private static final long serialVersionUID = 3176511008672645574L; diff --git a/src/main/java/org/junit/runners/BlockJUnit4ClassRunner.java b/src/main/java/org/junit/runners/BlockJUnit4ClassRunner.java index 455341aa0cb9..e7ae916a3ca0 100644 --- a/src/main/java/org/junit/runners/BlockJUnit4ClassRunner.java +++ b/src/main/java/org/junit/runners/BlockJUnit4ClassRunner.java @@ -30,6 +30,7 @@ import org.junit.runners.model.FrameworkMethod; import org.junit.runners.model.InitializationError; import org.junit.runners.model.MemberValueConsumer; +import org.junit.runners.model.MemberValueConsumerExtension; import org.junit.runners.model.MultipleFailureException; import org.junit.runners.model.Statement; import org.junit.runners.model.TestClass; @@ -456,7 +457,8 @@ private long getTimeout(Test annotation) { private static final ThreadLocal CURRENT_RULE_CONTAINER = new ThreadLocal(); - private static class RuleCollector implements MemberValueConsumer { + private static class RuleCollector implements MemberValueConsumer, + MemberValueConsumerExtension { final List result = new ArrayList(); public void accept(FrameworkMember member, T value) { @@ -469,5 +471,15 @@ public void accept(FrameworkMember member, T value) { } result.add(value); } + + @Override + public void acceptMismatchedTypeValue(FrameworkMember member, Object value) { + RuleContainer container = CURRENT_RULE_CONTAINER.get(); + if (container != null + && !(value instanceof TestRule) + && !(value instanceof MethodRule)) { + container.addMismatchedTypeValue(member, value); + } + } } } diff --git a/src/main/java/org/junit/runners/ParentRunner.java b/src/main/java/org/junit/runners/ParentRunner.java index 0a0e7cb6a583..933548659a82 100644 --- a/src/main/java/org/junit/runners/ParentRunner.java +++ b/src/main/java/org/junit/runners/ParentRunner.java @@ -24,6 +24,8 @@ import org.junit.Rule; import org.junit.internal.AssumptionViolatedException; import org.junit.internal.runners.model.EachTestNotifier; +import org.junit.internal.runners.rules.ValidationError; +import org.junit.internal.runners.statements.Fail; import org.junit.internal.runners.statements.RunAfters; import org.junit.internal.runners.statements.RunBefores; import org.junit.rules.RunRules; @@ -44,6 +46,8 @@ import org.junit.runners.model.InitializationError; import org.junit.runners.model.InvalidTestClassError; import org.junit.runners.model.MemberValueConsumer; +import org.junit.runners.model.MemberValueConsumerExtension; +import org.junit.runners.model.MultipleFailureException; import org.junit.runners.model.RunnerScheduler; import org.junit.runners.model.Statement; import org.junit.runners.model.TestClass; @@ -561,8 +565,10 @@ public void setScheduler(RunnerScheduler scheduler) { this.scheduler = scheduler; } - private static class ClassRuleCollector implements MemberValueConsumer { + private static class ClassRuleCollector implements MemberValueConsumer, + MemberValueConsumerExtension { final List entries = new ArrayList(); + final List errors = new ArrayList(); public void accept(FrameworkMember member, TestRule value) { ClassRule rule = member.getAnnotation(ClassRule.class); @@ -570,7 +576,21 @@ public void accept(FrameworkMember member, TestRule value) { rule != null ? rule.order() : null)); } + @Override + public void acceptMismatchedTypeValue(FrameworkMember member, Object value) { + errors.add(new ValidationError(member, ClassRule.class, "must implement TestRule.")); + } + public List getOrderedRules() { + if (!errors.isEmpty()) { + List result = new ArrayList(1); + result.add(new TestRule() { + public Statement apply(Statement base, Description description) { + return new Fail(errors.size() == 1 ? errors.get(0) : new MultipleFailureException(errors)); + } + }); + return result; + } Collections.sort(entries, RuleContainer.ENTRY_COMPARATOR); List result = new ArrayList(entries.size()); for (RuleContainer.RuleEntry entry : entries) { diff --git a/src/main/java/org/junit/runners/RuleContainer.java b/src/main/java/org/junit/runners/RuleContainer.java index 30ddd8d38a03..53961f12ccf0 100644 --- a/src/main/java/org/junit/runners/RuleContainer.java +++ b/src/main/java/org/junit/runners/RuleContainer.java @@ -3,14 +3,20 @@ import java.util.ArrayList; import java.util.Collections; import java.util.Comparator; +import java.util.HashMap; import java.util.IdentityHashMap; import java.util.List; +import java.util.Map; import org.junit.Rule; +import org.junit.internal.runners.rules.ValidationError; +import org.junit.internal.runners.statements.Fail; import org.junit.rules.MethodRule; import org.junit.rules.TestRule; import org.junit.runner.Description; +import org.junit.runners.model.FrameworkMember; import org.junit.runners.model.FrameworkMethod; +import org.junit.runners.model.MultipleFailureException; import org.junit.runners.model.Statement; /** @@ -22,6 +28,8 @@ class RuleContainer { private final IdentityHashMap orderValues = new IdentityHashMap(); private final List testRules = new ArrayList(); private final List methodRules = new ArrayList(); + private final Map, Object> mismatchedTypeRules = + new HashMap, Object>(); /** * Sets order value for the specified rule. @@ -38,6 +46,12 @@ public void add(TestRule testRule) { testRules.add(testRule); } + public void addMismatchedTypeValue(FrameworkMember member, Object value) { + if (!mismatchedTypeRules.containsKey(member)) { + mismatchedTypeRules.put(member, value); + } + } + static final Comparator ENTRY_COMPARATOR = new Comparator() { public int compare(RuleEntry o1, RuleEntry o2) { int result = compareInt(o1.order, o2.order); @@ -70,6 +84,14 @@ private List getSortedEntries() { */ public Statement apply(FrameworkMethod method, Description description, Object target, Statement statement) { + if (!mismatchedTypeRules.isEmpty()) { + List errors = new ArrayList(); + for (Map.Entry, Object> entry : mismatchedTypeRules.entrySet()) { + errors.add(new ValidationError(entry.getKey(), Rule.class, + "must implement MethodRule or TestRule.")); + } + return new Fail(errors.size() == 1 ? errors.get(0) : new MultipleFailureException(errors)); + } if (methodRules.isEmpty() && testRules.isEmpty()) { return statement; } diff --git a/src/main/java/org/junit/runners/model/MemberValueConsumerExtension.java b/src/main/java/org/junit/runners/model/MemberValueConsumerExtension.java new file mode 100644 index 000000000000..e65b8ae425ed --- /dev/null +++ b/src/main/java/org/junit/runners/model/MemberValueConsumerExtension.java @@ -0,0 +1,9 @@ +package org.junit.runners.model; + +/** + * @see MemberValueConsumer + * @since 4.14 + */ +public interface MemberValueConsumerExtension { + void acceptMismatchedTypeValue(FrameworkMember member, Object value); +} diff --git a/src/main/java/org/junit/runners/model/TestClass.java b/src/main/java/org/junit/runners/model/TestClass.java index 5962c2b478dc..09bd4ae63a5b 100644 --- a/src/main/java/org/junit/runners/model/TestClass.java +++ b/src/main/java/org/junit/runners/model/TestClass.java @@ -249,6 +249,8 @@ public void collectAnnotatedFieldValues(Object test, Object fieldValue = each.get(test); if (valueClass.isInstance(fieldValue)) { consumer.accept(each, valueClass.cast(fieldValue)); + } else if (consumer instanceof MemberValueConsumerExtension) { + ((MemberValueConsumerExtension) consumer).acceptMismatchedTypeValue(each, fieldValue); } } catch (IllegalAccessException e) { throw new RuntimeException( diff --git a/src/test/java/org/junit/rules/ClassRulesTest.java b/src/test/java/org/junit/rules/ClassRulesTest.java index ddfb74494c97..12d87a99b256 100644 --- a/src/test/java/org/junit/rules/ClassRulesTest.java +++ b/src/test/java/org/junit/rules/ClassRulesTest.java @@ -109,6 +109,51 @@ public void customRuleIsAppliedOnce() { assertEquals(1, ExampleTestWithCustomClassRule.counter.count); } + public static class ExampleTestWithCustomClassRuleDeclaredAsObject { + @ClassRule + public static final Object counter = new CustomCounter(); + + @Test + public void firstTest() { + assertEquals(1, ((CustomCounter) counter).count); + } + + @Test + public void secondTest() { + assertEquals(1, ((CustomCounter) counter).count); + } + } + + @Test + public void classRuleFieldCanBeDeclaredWithOtherType() { + ((CustomCounter) ExampleTestWithCustomClassRuleDeclaredAsObject.counter).count = 0; + Result result = JUnitCore.runClasses(ExampleTestWithCustomClassRuleDeclaredAsObject.class); + assertTrue("wasSuccessful", result.wasSuccessful()); + assertEquals(1, + ((CustomCounter) ExampleTestWithCustomClassRuleDeclaredAsObject.counter).count); + } + + public static class ExampleTestWithMismatchedClassRuleType { + @ClassRule + public static final String example1 = "Example"; + @ClassRule + public static final String example2 = "Example"; + + @Test + public void firstTest() { + } + + @Test + public void secondTest() { + } + } + + @Test + public void classRuleFieldMismatchedType() { + Result result = JUnitCore.runClasses(ExampleTestWithMismatchedClassRuleType.class); + assertEquals(2, result.getFailureCount()); + } + private static final List orderList = new LinkedList(); private static class OrderTestRule implements TestRule { diff --git a/src/test/java/org/junit/rules/RuleMemberValidatorTest.java b/src/test/java/org/junit/rules/RuleMemberValidatorTest.java index 01465a661230..9193fecd9061 100644 --- a/src/test/java/org/junit/rules/RuleMemberValidatorTest.java +++ b/src/test/java/org/junit/rules/RuleMemberValidatorTest.java @@ -10,6 +10,7 @@ import java.util.List; import org.junit.ClassRule; +import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; import org.junit.runners.model.FrameworkMethod; @@ -76,6 +77,7 @@ static class NonPublicTestWithClassRule { * Issue #1019 */ @Test + @Ignore public void rejectClassRuleThatIsImplementationOfMethodRule() { TestClass target = new TestClass(TestWithClassRuleIsImplementationOfMethodRule.class); CLASS_RULE_VALIDATOR.validate(target, errors); @@ -126,6 +128,7 @@ public Statement apply(Statement base, FrameworkMethod method, Object target) { * Issue #1019 */ @Test + @Ignore public void rejectClassRuleIsAnArbitraryObject() throws Exception { TestClass target = new TestClass(TestWithClassRuleIsAnArbitraryObject.class); CLASS_RULE_VALIDATOR.validate(target, errors); @@ -212,6 +215,7 @@ public Statement apply(Statement base, FrameworkMethod method, } @Test + @Ignore public void rejectArbitraryObjectWithRuleAnnotation() throws Exception { TestClass target = new TestClass(TestWithArbitraryObjectWithRuleAnnotation.class); RULE_VALIDATOR.validate(target, errors); diff --git a/src/test/java/org/junit/rules/TestRuleTest.java b/src/test/java/org/junit/rules/TestRuleTest.java index cc6db7d9eb34..d56f776fa9f6 100644 --- a/src/test/java/org/junit/rules/TestRuleTest.java +++ b/src/test/java/org/junit/rules/TestRuleTest.java @@ -24,19 +24,21 @@ public class TestRuleTest { private static boolean wasRun; + private static class ExampleRule implements TestRule { + public Statement apply(final Statement base, Description description) { + return new Statement() { + @Override + public void evaluate() throws Throwable { + wasRun = true; + base.evaluate(); + } + }; + } + } + public static class ExampleTest { @Rule - public TestRule example = new TestRule() { - public Statement apply(final Statement base, Description description) { - return new Statement() { - @Override - public void evaluate() throws Throwable { - wasRun = true; - base.evaluate(); - } - }; - } - }; + public TestRule example = new ExampleRule(); @Test public void nothing() { @@ -44,6 +46,33 @@ public void nothing() { } } + public static class ExampleTestForTestingFieldValueType { + @Rule + public Object example = new ExampleRule(); + + @Test + public void nothing() { + + } + } + + public static class ExampleTestForTestingInvalidRuleType { + @Rule + public Object example1 = "Example"; + @Rule + public Object example2 = "Example"; + + @Test + public void first() { + + } + + @Test + public void second() { + + } + } + @Test public void ruleIsIntroducedAndEvaluated() { wasRun = false; @@ -51,6 +80,19 @@ public void ruleIsIntroducedAndEvaluated() { assertTrue(wasRun); } + @Test + public void ruleIsIntroducedAndEvaluatedUsingValueType() { + wasRun = false; + JUnitCore.runClasses(ExampleTestForTestingFieldValueType.class); + assertTrue(wasRun); + } + + @Test + public void ruleOfInvalidTypeIsReported() { + Result result = JUnitCore.runClasses(ExampleTestForTestingInvalidRuleType.class); + assertEquals(4, result.getFailureCount()); + } + public static class BothKindsOfRule implements TestRule, org.junit.rules.MethodRule { public int applications = 0;