Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP validate actual rule type #1724

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

import java.lang.annotation.Annotation;

class ValidationError extends Exception {
public class ValidationError extends Exception {

private static final long serialVersionUID = 3176511008672645574L;

Expand Down
14 changes: 13 additions & 1 deletion src/main/java/org/junit/runners/BlockJUnit4ClassRunner.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -456,7 +457,8 @@ private long getTimeout(Test annotation) {
private static final ThreadLocal<RuleContainer> CURRENT_RULE_CONTAINER =
new ThreadLocal<RuleContainer>();

private static class RuleCollector<T> implements MemberValueConsumer<T> {
private static class RuleCollector<T> implements MemberValueConsumer<T>,
MemberValueConsumerExtension {
final List<T> result = new ArrayList<T>();

public void accept(FrameworkMember<?> member, T value) {
Expand All @@ -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);
}
}
}
}
22 changes: 21 additions & 1 deletion src/main/java/org/junit/runners/ParentRunner.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -561,16 +565,32 @@ public void setScheduler(RunnerScheduler scheduler) {
this.scheduler = scheduler;
}

private static class ClassRuleCollector implements MemberValueConsumer<TestRule> {
private static class ClassRuleCollector implements MemberValueConsumer<TestRule>,
MemberValueConsumerExtension {
final List<RuleContainer.RuleEntry> entries = new ArrayList<RuleContainer.RuleEntry>();
final List<Throwable> errors = new ArrayList<Throwable>();

public void accept(FrameworkMember<?> member, TestRule value) {
ClassRule rule = member.getAnnotation(ClassRule.class);
entries.add(new RuleContainer.RuleEntry(value, RuleContainer.RuleEntry.TYPE_TEST_RULE,
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<TestRule> getOrderedRules() {
if (!errors.isEmpty()) {
List<TestRule> result = new ArrayList<TestRule>(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<TestRule> result = new ArrayList<TestRule>(entries.size());
for (RuleContainer.RuleEntry entry : entries) {
Expand Down
22 changes: 22 additions & 0 deletions src/main/java/org/junit/runners/RuleContainer.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand All @@ -22,6 +28,8 @@ class RuleContainer {
private final IdentityHashMap<Object, Integer> orderValues = new IdentityHashMap<Object, Integer>();
private final List<TestRule> testRules = new ArrayList<TestRule>();
private final List<MethodRule> methodRules = new ArrayList<MethodRule>();
private final Map<FrameworkMember<?>, Object> mismatchedTypeRules =
new HashMap<FrameworkMember<?>, Object>();

/**
* Sets order value for the specified rule.
Expand All @@ -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<RuleEntry> ENTRY_COMPARATOR = new Comparator<RuleEntry>() {
public int compare(RuleEntry o1, RuleEntry o2) {
int result = compareInt(o1.order, o2.order);
Expand Down Expand Up @@ -70,6 +84,14 @@ private List<RuleEntry> getSortedEntries() {
*/
public Statement apply(FrameworkMethod method, Description description, Object target,
Statement statement) {
if (!mismatchedTypeRules.isEmpty()) {
List<Throwable> errors = new ArrayList<Throwable>();
for (Map.Entry<FrameworkMember<?>, 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;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package org.junit.runners.model;

/**
* @see MemberValueConsumer
* @since 4.14
*/
public interface MemberValueConsumerExtension {
void acceptMismatchedTypeValue(FrameworkMember<?> member, Object value);
}
2 changes: 2 additions & 0 deletions src/main/java/org/junit/runners/model/TestClass.java
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,8 @@ public <T> 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(
Expand Down
45 changes: 45 additions & 0 deletions src/test/java/org/junit/rules/ClassRulesTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> orderList = new LinkedList<String>();

private static class OrderTestRule implements TestRule {
Expand Down
4 changes: 4 additions & 0 deletions src/test/java/org/junit/rules/RuleMemberValidatorTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -76,6 +77,7 @@ static class NonPublicTestWithClassRule {
* <a href="https://github.com/junit-team/junit4/issues/1019">Issue #1019</a>
*/
@Test
@Ignore
public void rejectClassRuleThatIsImplementationOfMethodRule() {
TestClass target = new TestClass(TestWithClassRuleIsImplementationOfMethodRule.class);
CLASS_RULE_VALIDATOR.validate(target, errors);
Expand Down Expand Up @@ -126,6 +128,7 @@ public Statement apply(Statement base, FrameworkMethod method, Object target) {
* <a href="https://github.com/junit-team/junit4/issues/1019">Issue #1019</a>
*/
@Test
@Ignore
public void rejectClassRuleIsAnArbitraryObject() throws Exception {
TestClass target = new TestClass(TestWithClassRuleIsAnArbitraryObject.class);
CLASS_RULE_VALIDATOR.validate(target, errors);
Expand Down Expand Up @@ -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);
Expand Down
64 changes: 53 additions & 11 deletions src/test/java/org/junit/rules/TestRuleTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,33 +24,75 @@
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() {

}
}

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;
JUnitCore.runClasses(ExampleTest.class);
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;

Expand Down