Extract preliminary reusable Condition API from LifecycleMethodUtils

Author: marcphilipp

junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/LifecycleMethodUtils.java

@@ -12,26 +12,24 @@
 
 import static org.junit.platform.commons.support.AnnotationSupport.findAnnotatedMethods;
 import static org.junit.platform.commons.util.CollectionUtils.toUnmodifiableList;
+import static org.junit.platform.engine.support.discovery.DiscoveryIssueReporter.Condition.allOf;
 
 import java.lang.annotation.Annotation;
 import java.lang.reflect.Method;
 import java.util.List;
-import java.util.function.Consumer;
-import java.util.function.Function;
-import java.util.function.Predicate;
 
 import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.platform.commons.support.HierarchyTraversalMode;
 import org.junit.platform.commons.support.ModifierSupport;
-import org.junit.platform.commons.util.Preconditions;
 import org.junit.platform.commons.util.ReflectionUtils;
 import org.junit.platform.engine.DiscoveryIssue;
 import org.junit.platform.engine.DiscoveryIssue.Severity;
 import org.junit.platform.engine.support.descriptor.MethodSource;
 import org.junit.platform.engine.support.discovery.DiscoveryIssueReporter;
+import org.junit.platform.engine.support.discovery.DiscoveryIssueReporter.Condition;
 
 /**
  * Collection of utilities for working with test lifecycle methods.
@@ -70,7 +68,7 @@ private static List<Method> findMethodsAndCheckStatic(Class<?> testClass, boolea
 			Class<? extends Annotation> annotationType, HierarchyTraversalMode traversalMode,
 			DiscoveryIssueReporter issueReporter) {
 
-		Predicate<Method> additionalCondition = requireStatic ? isStatic(annotationType, issueReporter) : __ -> true;
+		Condition<Method> additionalCondition = requireStatic ? isStatic(annotationType, issueReporter) : __ -> true;
 		return findMethodsAndCheckVoidReturnType(testClass, annotationType, traversalMode, issueReporter,
 			additionalCondition);
 	}
@@ -85,46 +83,46 @@ private static List<Method> findMethodsAndCheckNonStatic(Class<?> testClass,
 
 	private static List<Method> findMethodsAndCheckVoidReturnType(Class<?> testClass,
 			Class<? extends Annotation> annotationType, HierarchyTraversalMode traversalMode,
-			DiscoveryIssueReporter issueReporter, Predicate<? super Method> additionalCondition) {
+			DiscoveryIssueReporter issueReporter, Condition<? super Method> additionalCondition) {
 
 		return findAnnotatedMethods(testClass, annotationType, traversalMode).stream() //
 				.peek(isNotPrivate(annotationType, issueReporter)) //
 				.filter(allOf(returnsPrimitiveVoid(annotationType, issueReporter), additionalCondition)) //
 				.collect(toUnmodifiableList());
 	}
 
-	private static Predicate<Method> isStatic(Class<? extends Annotation> annotationType,
+	private static Condition<Method> isStatic(Class<? extends Annotation> annotationType,
 			DiscoveryIssueReporter issueReporter) {
-		return DiscoveryIssueReportingPredicate.from(ModifierSupport::isStatic, issueReporter, method -> {
+		return issueReporter.createReportingCondition(ModifierSupport::isStatic, method -> {
 			String message = String.format(
 				"@%s method '%s' must be static unless the test class is annotated with @TestInstance(Lifecycle.PER_CLASS).",
 				annotationType.getSimpleName(), method.toGenericString());
 			return createIssue(Severity.ERROR, message, method);
 		});
 	}
 
-	private static Predicate<Method> isNotStatic(Class<? extends Annotation> annotationType,
+	private static Condition<Method> isNotStatic(Class<? extends Annotation> annotationType,
 			DiscoveryIssueReporter issueReporter) {
-		return DiscoveryIssueReportingPredicate.from(ModifierSupport::isNotStatic, issueReporter, method -> {
+		return issueReporter.createReportingCondition(ModifierSupport::isNotStatic, method -> {
 			String message = String.format("@%s method '%s' must not be static.", annotationType.getSimpleName(),
 				method.toGenericString());
 			return createIssue(Severity.ERROR, message, method);
 		});
 	}
 
-	private static Consumer<Method> isNotPrivate(Class<? extends Annotation> annotationType,
+	private static Condition<Method> isNotPrivate(Class<? extends Annotation> annotationType,
 			DiscoveryIssueReporter issueReporter) {
-		return DiscoveryIssueReportingPredicate.from(ModifierSupport::isNotPrivate, issueReporter, method -> {
+		return issueReporter.createReportingCondition(ModifierSupport::isNotPrivate, method -> {
 			String message = String.format(
 				"@%s method '%s' should not be private. This will be disallowed in a future release.",
 				annotationType.getSimpleName(), method.toGenericString());
 			return createIssue(Severity.DEPRECATION, message, method);
 		});
 	}
 
-	private static Predicate<Method> returnsPrimitiveVoid(Class<? extends Annotation> annotationType,
+	private static Condition<Method> returnsPrimitiveVoid(Class<? extends Annotation> annotationType,
 			DiscoveryIssueReporter issueReporter) {
-		return DiscoveryIssueReportingPredicate.from(ReflectionUtils::returnsPrimitiveVoid, issueReporter, method -> {
+		return issueReporter.createReportingCondition(ReflectionUtils::returnsPrimitiveVoid, method -> {
 			String message = String.format("@%s method '%s' must not return a value.", annotationType.getSimpleName(),
 				method.toGenericString());
 			return createIssue(Severity.ERROR, message, method);
@@ -135,61 +133,4 @@ private static DiscoveryIssue createIssue(Severity severity, String message, Met
 		return DiscoveryIssue.builder(severity, message).source(MethodSource.from(method)).build();
 	}
 
-	@SafeVarargs
-	@SuppressWarnings("varargs")
-	private static <T> Predicate<T> allOf(Predicate<? super T>... predicates) {
-		Preconditions.notNull(predicates, "predicates must not be null");
-		Preconditions.notEmpty(predicates, "predicates must not be empty");
-		Preconditions.containsNoNullElements(predicates, "predicates must not contain null elements");
-		return value -> {
-			boolean result = true;
-			for (Predicate<? super T> predicate : predicates) {
-				result &= predicate.test(value);
-			}
-			return result;
-		};
-	}
-
-	private abstract static class DiscoveryIssueReportingPredicate<T> implements Predicate<T>, Consumer<T> {
-
-		static <T> DiscoveryIssueReportingPredicate<T> from(Predicate<T> predicate, DiscoveryIssueReporter reporter,
-				Function<T, DiscoveryIssue> issueBuilder) {
-			return new DiscoveryIssueReportingPredicate<T>(reporter) {
-				@Override
-				protected boolean checkCondition(T value) {
-					return predicate.test(value);
-				}
-
-				@Override
-				protected DiscoveryIssue createIssue(T value) {
-					return issueBuilder.apply(value);
-				}
-			};
-		}
-
-		private final DiscoveryIssueReporter reporter;
-
-		protected DiscoveryIssueReportingPredicate(DiscoveryIssueReporter reporter) {
-			this.reporter = reporter;
-		}
-
-		@Override
-		public final boolean test(T value) {
-			if (checkCondition(value)) {
-				return true;
-			}
-			reporter.reportIssue(createIssue(value));
-			return false;
-		}
-
-		@Override
-		public void accept(T value) {
-			test(value);
-		}
-
-		protected abstract boolean checkCondition(T value);
-
-		protected abstract DiscoveryIssue createIssue(T value);
-	}
-
 }

junit-platform-engine/src/main/java/org/junit/platform/engine/support/discovery/DefaultDiscoveryIssueReporter.java

@@ -12,8 +12,15 @@
 
 import static org.apiguardian.api.API.Status.EXPERIMENTAL;
 
+import java.util.function.Consumer;
+import java.util.function.Function;
+import java.util.function.Predicate;
+
 import org.apiguardian.api.API;
+import org.junit.platform.commons.util.Preconditions;
 import org.junit.platform.engine.DiscoveryIssue;
+import org.junit.platform.engine.EngineDiscoveryListener;
+import org.junit.platform.engine.UniqueId;
 
 /**
  * {@code DiscoveryIssueReporter} defines the API for reporting
@@ -26,6 +33,20 @@
 @FunctionalInterface
 public interface DiscoveryIssueReporter {
 
+	/**
+	 * Create a new {@code DiscoveryIssueReporter} that reports issues to the
+	 * supplied {@link EngineDiscoveryListener} for the specified engine.
+	 *
+	 * @param engineDiscoveryListener the listener to report issues to; never
+	 * {@code null}
+	 * @param engineId the unique identifier of the engine; never {@code null}
+	 */
+	static DiscoveryIssueReporter create(EngineDiscoveryListener engineDiscoveryListener, UniqueId engineId) {
+		Preconditions.notNull(engineDiscoveryListener, "engineDiscoveryListener must not be null");
+		Preconditions.notNull(engineId, "engineId must not be null");
+		return issue -> engineDiscoveryListener.issueEncountered(engineId, issue);
+	}
+
 	/**
 	 * Build the supplied {@link DiscoveryIssue.Builder Builder} and report the
 	 * resulting {@link DiscoveryIssue}.
@@ -39,4 +60,77 @@ default void reportIssue(DiscoveryIssue.Builder builder) {
 	 */
 	void reportIssue(DiscoveryIssue issue);
 
+	/**
+	 * Create a {@link Condition} that reports a {@link DiscoveryIssue} when the
+	 * supplied {@link Predicate} is not met.
+	 *
+	 * @param predicate the predicate to test; never {@code null}
+	 * @param issueCreator the function to create the issue with; never {@code null}
+	 * @return a new {@code Condition}; never {@code null}
+	 */
+	default <T> Condition<T> createReportingCondition(Predicate<T> predicate,
+			Function<T, DiscoveryIssue> issueCreator) {
+		Preconditions.notNull(predicate, "predicate must not be null");
+		Preconditions.notNull(issueCreator, "issueCreator must not be null");
+		return value -> {
+			if (predicate.test(value)) {
+				return true;
+			}
+			else {
+				reportIssue(issueCreator.apply(value));
+				return false;
+			}
+		};
+	}
+
+	/**
+	 * A {@code Condition} is a union of {@link Predicate} and {@link Consumer}.
+	 *
+	 * <p>Instances of this type may be used as {@link Predicate Predicates} or
+	 * {@link Consumer Consumers}. For example, a {@code Condition} may be
+	 * passed to {@link java.util.stream.Stream#filter(Predicate)} if it is used
+	 * for filtering, or to {@link java.util.stream.Stream#peek(Consumer)} if it
+	 * is only used for reporting or other side effects.
+	 *
+	 * @see #createReportingCondition(Predicate, Function)
+	 */
+	@FunctionalInterface
+	interface Condition<T> extends Predicate<T>, Consumer<T> {
+
+		/**
+		 * Return a composed condition that represents a logical AND of the
+		 * supplied conditions without short-circuiting.
+		 *
+		 * <p><em>All</em> of the supplied conditions will be evaluated even if
+		 * one or more of them return {@code false} to ensure that all issues
+		 * are reported.
+		 *
+		 * @param conditions the conditions to compose; never {@code null}, not
+		 * empty, and must not contain any {@code null} elements
+		 * @return the composed condition; never {@code null}
+		 */
+		@SafeVarargs
+		@SuppressWarnings("varargs")
+		static <T> Condition<T> allOf(Condition<? super T>... conditions) {
+			Preconditions.notNull(conditions, "conditions must not be null");
+			Preconditions.notEmpty(conditions, "conditions must not be empty");
+			Preconditions.containsNoNullElements(conditions, "conditions must not contain null elements");
+			return value -> {
+				boolean result = true;
+				for (Condition<? super T> condition : conditions) {
+					result &= condition.test(value);
+				}
+				return result;
+			};
+		}
+
+		/**
+		 * Evaluate the {@code #test(Object)} method of this condition to
+		 * potentially report an issue.
+		 */
+		@Override
+		default void accept(T value) {
+			test(value);
+		}
+	}
 }

junit-platform-engine/src/main/java/org/junit/platform/engine/support/discovery/DiscoveryIssueReporter.java

@@ -110,7 +110,7 @@ private EngineDiscoveryRequestResolver(List<Function<InitializationContext<T>, S
 	public void resolve(EngineDiscoveryRequest request, T engineDescriptor) {
 		Preconditions.notNull(request, "request must not be null");
 		Preconditions.notNull(engineDescriptor, "engineDescriptor must not be null");
-		DiscoveryIssueReporter issueReporter = new DefaultDiscoveryIssueReporter(request.getDiscoveryListener(),
+		DiscoveryIssueReporter issueReporter = DiscoveryIssueReporter.create(request.getDiscoveryListener(),
 			engineDescriptor.getUniqueId());
 		InitializationContext<T> initializationContext = new DefaultInitializationContext<>(request, engineDescriptor,
 			issueReporter);