Skip to content

Commit

Permalink
Process all annotated members in a single pass
Browse files Browse the repository at this point in the history 
This fixes the case where either fields or methods are used to define
instances of ExpectedException, i.e. in particular the most common case
of a single (field) instance.

Issue: #1069
  • Loading branch information
@marcphilipp @sbrannen
marcphilipp authored and sbrannen committed Sep 19, 2017
1 parent e996bce commit 25e0126
Show file tree
Hide file tree
Showing 7 changed files with 57 additions and 140 deletions.
17 changes: 3 additions & 14 deletions ...port/src/main/java/org/junit/jupiter/migrationsupport/rules/ExpectedExceptionSupport.java
View file
Expand Up @@ -14,17 +14,13 @@
import static java.lang.Boolean.TRUE; import static java.lang.Boolean.TRUE;
import static org.apiguardian.api.API.Status.EXPERIMENTAL; import static org.apiguardian.api.API.Status.EXPERIMENTAL;


import java.util.function.Function;

import org.apiguardian.api.API; import org.apiguardian.api.API;
import org.junit.jupiter.api.extension.AfterEachCallback; import org.junit.jupiter.api.extension.AfterEachCallback;
import org.junit.jupiter.api.extension.ExtensionContext; import org.junit.jupiter.api.extension.ExtensionContext;
import org.junit.jupiter.api.extension.ExtensionContext.Namespace; import org.junit.jupiter.api.extension.ExtensionContext.Namespace;
import org.junit.jupiter.api.extension.ExtensionContext.Store; import org.junit.jupiter.api.extension.ExtensionContext.Store;
import org.junit.jupiter.api.extension.TestExecutionExceptionHandler; import org.junit.jupiter.api.extension.TestExecutionExceptionHandler;
import org.junit.jupiter.migrationsupport.rules.adapter.AbstractTestRuleAdapter;
import org.junit.jupiter.migrationsupport.rules.adapter.ExpectedExceptionAdapter; import org.junit.jupiter.migrationsupport.rules.adapter.ExpectedExceptionAdapter;
import org.junit.jupiter.migrationsupport.rules.member.TestRuleAnnotatedMember;
import org.junit.rules.ExpectedException; import org.junit.rules.ExpectedException;


/** /**
Expand All @@ -48,27 +44,20 @@ public class ExpectedExceptionSupport implements AfterEachCallback, TestExecutio


private static final String EXCEPTION_WAS_HANDLED = "exceptionWasHandled"; private static final String EXCEPTION_WAS_HANDLED = "exceptionWasHandled";


private final Function<TestRuleAnnotatedMember, AbstractTestRuleAdapter> adapterGenerator = ExpectedExceptionAdapter::new; private final TestRuleSupport support = new TestRuleSupport(ExpectedExceptionAdapter::new, ExpectedException.class);

private final TestRuleFieldSupport fieldSupport = new TestRuleFieldSupport(this.adapterGenerator,
ExpectedException.class);
private final TestRuleMethodSupport methodSupport = new TestRuleMethodSupport(this.adapterGenerator,
ExpectedException.class);


@Override @Override
public void handleTestExecutionException(ExtensionContext context, Throwable throwable) throws Throwable { public void handleTestExecutionException(ExtensionContext context, Throwable throwable) throws Throwable {
getStore(context).put(EXCEPTION_WAS_HANDLED, TRUE); getStore(context).put(EXCEPTION_WAS_HANDLED, TRUE);
this.methodSupport.handleTestExecutionException(context, throwable); this.support.handleTestExecutionException(context, throwable);
this.fieldSupport.handleTestExecutionException(context, throwable);
} }


@Override @Override
public void afterEach(ExtensionContext context) throws Exception { public void afterEach(ExtensionContext context) throws Exception {
boolean exceptionWasHandled = getStore(context).getOrComputeIfAbsent(EXCEPTION_WAS_HANDLED, key -> FALSE, boolean exceptionWasHandled = getStore(context).getOrComputeIfAbsent(EXCEPTION_WAS_HANDLED, key -> FALSE,
Boolean.class); Boolean.class);
if (!exceptionWasHandled) { if (!exceptionWasHandled) {
this.methodSupport.afterEach(context); this.support.afterEach(context);
this.fieldSupport.afterEach(context);
} }
} }


Expand Down
17 changes: 3 additions & 14 deletions ...pport/src/main/java/org/junit/jupiter/migrationsupport/rules/ExternalResourceSupport.java
View file
Expand Up @@ -12,15 +12,11 @@


import static org.apiguardian.api.API.Status.EXPERIMENTAL; import static org.apiguardian.api.API.Status.EXPERIMENTAL;


import java.util.function.Function;

import org.apiguardian.api.API; import org.apiguardian.api.API;
import org.junit.jupiter.api.extension.AfterEachCallback; import org.junit.jupiter.api.extension.AfterEachCallback;
import org.junit.jupiter.api.extension.BeforeEachCallback; import org.junit.jupiter.api.extension.BeforeEachCallback;
import org.junit.jupiter.api.extension.ExtensionContext; import org.junit.jupiter.api.extension.ExtensionContext;
import org.junit.jupiter.migrationsupport.rules.adapter.AbstractTestRuleAdapter;
import org.junit.jupiter.migrationsupport.rules.adapter.ExternalResourceAdapter; import org.junit.jupiter.migrationsupport.rules.adapter.ExternalResourceAdapter;
import org.junit.jupiter.migrationsupport.rules.member.TestRuleAnnotatedMember;
import org.junit.rules.ExternalResource; import org.junit.rules.ExternalResource;


/** /**
Expand All @@ -45,23 +41,16 @@
@API(status = EXPERIMENTAL, since = "5.0") @API(status = EXPERIMENTAL, since = "5.0")
public class ExternalResourceSupport implements BeforeEachCallback, AfterEachCallback { public class ExternalResourceSupport implements BeforeEachCallback, AfterEachCallback {


private final Function<TestRuleAnnotatedMember, AbstractTestRuleAdapter> adapterGenerator = ExternalResourceAdapter::new; private final TestRuleSupport support = new TestRuleSupport(ExternalResourceAdapter::new, ExternalResource.class);

private final TestRuleFieldSupport fieldSupport = new TestRuleFieldSupport(this.adapterGenerator,
ExternalResource.class);
private final TestRuleMethodSupport methodSupport = new TestRuleMethodSupport(this.adapterGenerator,
ExternalResource.class);


@Override @Override
public void beforeEach(ExtensionContext context) throws Exception { public void beforeEach(ExtensionContext context) throws Exception {
this.fieldSupport.beforeEach(context); this.support.beforeEach(context);
this.methodSupport.beforeEach(context);
} }


@Override @Override
public void afterEach(ExtensionContext context) throws Exception { public void afterEach(ExtensionContext context) throws Exception {
this.methodSupport.afterEach(context); this.support.afterEach(context);
this.fieldSupport.afterEach(context);
} }


} }
39 changes: 0 additions & 39 deletions ...nsupport/src/main/java/org/junit/jupiter/migrationsupport/rules/TestRuleFieldSupport.java
View file

This file was deleted.

44 changes: 0 additions & 44 deletions ...support/src/main/java/org/junit/jupiter/migrationsupport/rules/TestRuleMethodSupport.java
View file

This file was deleted.

65 changes: 48 additions & 17 deletions ...upport/rules/AbstractTestRuleSupport.java → ...grationsupport/rules/TestRuleSupport.java
View file
Expand Up @@ -10,11 +10,21 @@


package org.junit.jupiter.migrationsupport.rules; package org.junit.jupiter.migrationsupport.rules;


import static org.junit.platform.commons.util.AnnotationUtils.findPublicAnnotatedFields;
import static org.junit.platform.commons.util.AnnotationUtils.isAnnotated;
import static org.junit.platform.commons.util.ReflectionUtils.findMethods;

import java.lang.reflect.Field;
import java.lang.reflect.Member; import java.lang.reflect.Member;
import java.lang.reflect.Method;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List; import java.util.List;
import java.util.function.Consumer; import java.util.function.Consumer;
import java.util.function.Function; import java.util.function.Function;
import java.util.function.Predicate;


import org.junit.Rule;
import org.junit.jupiter.api.extension.AfterEachCallback; import org.junit.jupiter.api.extension.AfterEachCallback;
import org.junit.jupiter.api.extension.BeforeEachCallback; import org.junit.jupiter.api.extension.BeforeEachCallback;
import org.junit.jupiter.api.extension.ExtensionContext; import org.junit.jupiter.api.extension.ExtensionContext;
Expand All @@ -29,39 +39,58 @@
/** /**
* @since 5.0 * @since 5.0
*/ */
abstract class AbstractTestRuleSupport<T extends Member> class TestRuleSupport implements BeforeEachCallback, TestExecutionExceptionHandler, AfterEachCallback {
implements BeforeEachCallback, TestExecutionExceptionHandler, AfterEachCallback {
private static final Consumer<List<Member>> NO_OP = members -> {
};


private final Class<? extends TestRule> ruleType; private final Class<? extends TestRule> ruleType;
private final Function<TestRuleAnnotatedMember, AbstractTestRuleAdapter> adapterGenerator; private final Function<TestRuleAnnotatedMember, AbstractTestRuleAdapter> adapterGenerator;


AbstractTestRuleSupport(Function<TestRuleAnnotatedMember, AbstractTestRuleAdapter> adapterGenerator, TestRuleSupport(Function<TestRuleAnnotatedMember, AbstractTestRuleAdapter> adapterGenerator,
Class<? extends TestRule> ruleType) { Class<? extends TestRule> ruleType) {
this.adapterGenerator = adapterGenerator; this.adapterGenerator = adapterGenerator;
this.ruleType = ruleType; this.ruleType = ruleType;
} }


protected abstract List<T> findRuleAnnotatedMembers(Object testInstance); private List<Member> findRuleAnnotatedMembers(Object testInstance) {
List<Member> members = new ArrayList<>();
members.addAll(findAnnotatedFields(testInstance));
members.addAll(findAnnotatedMethods(testInstance));
return members;
}

private List<Method> findAnnotatedMethods(Object testInstance) {
Predicate<Method> isRuleMethod = method -> isAnnotated(method, Rule.class);
Predicate<Method> hasCorrectReturnType = method -> method.getReturnType().isAssignableFrom(getRuleType());

return findMethods(testInstance.getClass(), isRuleMethod.and(hasCorrectReturnType));
}

private List<Field> findAnnotatedFields(Object testInstance) {
return findPublicAnnotatedFields(testInstance.getClass(), getRuleType(), Rule.class);
}


protected Class<? extends TestRule> getRuleType() { private Class<? extends TestRule> getRuleType() {
return this.ruleType; return this.ruleType;
} }


@Override @Override
public void beforeEach(ExtensionContext context) throws Exception { public void beforeEach(ExtensionContext context) throws Exception {
invokeAppropriateMethodOnRuleAnnotatedMembers(context, GenericBeforeAndAfterAdvice::before); invokeAppropriateMethodOnRuleAnnotatedMembers(context, NO_OP, GenericBeforeAndAfterAdvice::before);
} }


@Override @Override
public void handleTestExecutionException(ExtensionContext context, Throwable throwable) throws Throwable { public void handleTestExecutionException(ExtensionContext context, Throwable throwable) throws Throwable {
int numRuleAnnotatedMembers = invokeAppropriateMethodOnRuleAnnotatedMembers(context, advice -> { int numRuleAnnotatedMembers = invokeAppropriateMethodOnRuleAnnotatedMembers(context, Collections::reverse,
try { advice -> {
advice.handleTestExecutionException(throwable); try {
} advice.handleTestExecutionException(throwable);
catch (Throwable t) { }
throw ExceptionUtils.throwAsUncheckedException(t); catch (Throwable t) {
} throw ExceptionUtils.throwAsUncheckedException(t);
}); }
});


// If no appropriate @Rule annotated members were discovered, we then // If no appropriate @Rule annotated members were discovered, we then
// have to rethrow the exception in order not to silently swallow it. // have to rethrow the exception in order not to silently swallow it.
Expand All @@ -73,17 +102,19 @@ public void handleTestExecutionException(ExtensionContext context, Throwable thr


@Override @Override
public void afterEach(ExtensionContext context) throws Exception { public void afterEach(ExtensionContext context) throws Exception {
invokeAppropriateMethodOnRuleAnnotatedMembers(context, GenericBeforeAndAfterAdvice::after); invokeAppropriateMethodOnRuleAnnotatedMembers(context, Collections::reverse,
GenericBeforeAndAfterAdvice::after);
} }


/** /**
* @return the number of appropriate rule-annotated members that were discovered * @return the number of appropriate rule-annotated members that were discovered
*/ */
private int invokeAppropriateMethodOnRuleAnnotatedMembers(ExtensionContext context, private int invokeAppropriateMethodOnRuleAnnotatedMembers(ExtensionContext context, Consumer<List<Member>> ordering,
Consumer<GenericBeforeAndAfterAdvice> methodCaller) { Consumer<GenericBeforeAndAfterAdvice> methodCaller) {


Object testInstance = context.getRequiredTestInstance(); Object testInstance = context.getRequiredTestInstance();
List<T> members = findRuleAnnotatedMembers(testInstance); List<Member> members = findRuleAnnotatedMembers(testInstance);
ordering.accept(members);


// @formatter:off // @formatter:off
members.stream() members.stream()
Expand Down
13 changes: 2 additions & 11 deletions ...rationsupport/src/main/java/org/junit/jupiter/migrationsupport/rules/VerifierSupport.java
View file
Expand Up @@ -12,14 +12,10 @@


import static org.apiguardian.api.API.Status.EXPERIMENTAL; import static org.apiguardian.api.API.Status.EXPERIMENTAL;


import java.util.function.Function;

import org.apiguardian.api.API; import org.apiguardian.api.API;
import org.junit.jupiter.api.extension.AfterEachCallback; import org.junit.jupiter.api.extension.AfterEachCallback;
import org.junit.jupiter.api.extension.ExtensionContext; import org.junit.jupiter.api.extension.ExtensionContext;
import org.junit.jupiter.migrationsupport.rules.adapter.AbstractTestRuleAdapter;
import org.junit.jupiter.migrationsupport.rules.adapter.VerifierAdapter; import org.junit.jupiter.migrationsupport.rules.adapter.VerifierAdapter;
import org.junit.jupiter.migrationsupport.rules.member.TestRuleAnnotatedMember;
import org.junit.rules.Verifier; import org.junit.rules.Verifier;


/** /**
Expand All @@ -44,16 +40,11 @@
@API(status = EXPERIMENTAL, since = "5.0") @API(status = EXPERIMENTAL, since = "5.0")
public class VerifierSupport implements AfterEachCallback { public class VerifierSupport implements AfterEachCallback {


private final Function<TestRuleAnnotatedMember, AbstractTestRuleAdapter> adapterGenerator = VerifierAdapter::new; private final TestRuleSupport support = new TestRuleSupport(VerifierAdapter::new, Verifier.class);

private final TestRuleFieldSupport fieldSupport = new TestRuleFieldSupport(this.adapterGenerator, Verifier.class);
private final TestRuleMethodSupport methodSupport = new TestRuleMethodSupport(this.adapterGenerator,
Verifier.class);


@Override @Override
public void afterEach(ExtensionContext context) throws Exception { public void afterEach(ExtensionContext context) throws Exception {
this.fieldSupport.afterEach(context); this.support.afterEach(context);
this.methodSupport.afterEach(context);
} }


} }
2 changes: 1 addition & 1 deletion .../java/org/junit/jupiter/migrationsupport/rules/member/TestRuleAnnotatedMemberFactory.java
View file
Expand Up @@ -35,7 +35,7 @@ public static TestRuleAnnotatedMember from(Object testInstance, Member member) {
if (member instanceof Method) { if (member instanceof Method) {
return new TestRuleAnnotatedMethod(testInstance, (Method) member); return new TestRuleAnnotatedMethod(testInstance, (Method) member);
} }
else if (member instanceof Field) { if (member instanceof Field) {
return new TestRuleAnnotatedField(testInstance, (Field) member); return new TestRuleAnnotatedField(testInstance, (Field) member);
} }
throw new PreconditionViolationException( throw new PreconditionViolationException(
Expand Down

0 comments on commit 25e0126

Please sign in to comment.