From 3e0a5447f7177dcbe0bac4ec747e4aa50d0dc039 Mon Sep 17 00:00:00 2001
From: Marc Philipp <mail@marcphilipp.de>
Date: Mon, 17 Mar 2025 17:57:11 +0100
Subject: [PATCH 1/4] Pass `DiscoveryIssueReporter` to `LifecycleMethodUtils`
---
.../descriptor/ClassBasedTestDescriptor.java | 18 ++-
.../descriptor/LifecycleMethodUtils.java | 138 ++++++++++++------
.../discovery/DiscoveryIssueReporter.java | 1 +
.../descriptor/LifecycleMethodUtilsTests.java | 109 +++++++++-----
4 files changed, 181 insertions(+), 85 deletions(-)
diff --git a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/ClassBasedTestDescriptor.java b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/ClassBasedTestDescriptor.java
index 2e97cc2d385f..dd7c41f40f52 100644
--- a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/ClassBasedTestDescriptor.java
+++ b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/ClassBasedTestDescriptor.java
@@ -166,8 +166,14 @@ public final JupiterEngineExecutionContext prepare(JupiterEngineExecutionContext
registerExtensionsFromConstructorParameters(registry, getTestClass());
}
- this.beforeAllMethods = findBeforeAllMethods(getTestClass(), this.classInfo.lifecycle == Lifecycle.PER_METHOD);
- this.afterAllMethods = findAfterAllMethods(getTestClass(), this.classInfo.lifecycle == Lifecycle.PER_METHOD);
+ this.beforeAllMethods = findBeforeAllMethods(getTestClass(), this.classInfo.lifecycle == Lifecycle.PER_METHOD,
+ issue -> {
+ throw new JUnitException(issue.message());
+ });
+ this.afterAllMethods = findAfterAllMethods(getTestClass(), this.classInfo.lifecycle == Lifecycle.PER_METHOD,
+ issue -> {
+ throw new JUnitException(issue.message());
+ });
this.beforeAllMethods.forEach(method -> registerExtensionsFromExecutableParameters(registry, method));
// Since registerBeforeEachMethodAdapters() and registerAfterEachMethodAdapters() also
@@ -466,13 +472,17 @@ private boolean isPerClassLifecycle(JupiterEngineExecutionContext context) {
}
private void registerBeforeEachMethodAdapters(ExtensionRegistrar registrar) {
- List<Method> beforeEachMethods = findBeforeEachMethods(getTestClass());
+ List<Method> beforeEachMethods = findBeforeEachMethods(getTestClass(), issue -> {
+ throw new JUnitException(issue.message());
+ });
registerMethodsAsExtensions(beforeEachMethods, registrar, this::synthesizeBeforeEachMethodAdapter);
}
private void registerAfterEachMethodAdapters(ExtensionRegistrar registrar) {
// Make a local copy since findAfterEachMethods() returns an immutable list.
- List<Method> afterEachMethods = new ArrayList<>(findAfterEachMethods(getTestClass()));
+ List<Method> afterEachMethods = new ArrayList<>(findAfterEachMethods(getTestClass(), issue -> {
+ throw new JUnitException(issue.message());
+ }));
// Since the bottom-up ordering of afterEachMethods will later be reversed when the
// synthesized AfterEachMethodAdapters are executed within TestMethodTestDescriptor,
diff --git a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/LifecycleMethodUtils.java b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/LifecycleMethodUtils.java
index 83f60a06ef9a..507660732a68 100644
--- a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/LifecycleMethodUtils.java
+++ b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/LifecycleMethodUtils.java
@@ -11,19 +11,26 @@
package org.junit.jupiter.engine.descriptor;
import static org.junit.platform.commons.support.AnnotationSupport.findAnnotatedMethods;
-import static org.junit.platform.commons.util.ReflectionUtils.returnsPrimitiveVoid;
+import static org.junit.platform.commons.util.CollectionUtils.toUnmodifiableList;
import java.lang.annotation.Annotation;
import java.lang.reflect.Method;
import java.util.List;
+import java.util.function.Function;
+import java.util.function.Predicate;
+import java.util.stream.Stream;
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.JUnitException;
import org.junit.platform.commons.support.HierarchyTraversalMode;
import org.junit.platform.commons.support.ModifierSupport;
+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;
/**
* Collection of utilities for working with test lifecycle methods.
@@ -36,68 +43,117 @@ private LifecycleMethodUtils() {
/* no-op */
}
- static List<Method> findBeforeAllMethods(Class<?> testClass, boolean requireStatic) {
- return findMethodsAndAssertStatic(testClass, requireStatic, BeforeAll.class, HierarchyTraversalMode.TOP_DOWN);
+ static List<Method> findBeforeAllMethods(Class<?> testClass, boolean requireStatic,
+ DiscoveryIssueReporter issueReporter) {
+ return findMethodsAndCheckStatic(testClass, requireStatic, BeforeAll.class, HierarchyTraversalMode.TOP_DOWN,
+ issueReporter);
}
- static List<Method> findAfterAllMethods(Class<?> testClass, boolean requireStatic) {
- return findMethodsAndAssertStatic(testClass, requireStatic, AfterAll.class, HierarchyTraversalMode.BOTTOM_UP);
+ static List<Method> findAfterAllMethods(Class<?> testClass, boolean requireStatic,
+ DiscoveryIssueReporter issueReporter) {
+ return findMethodsAndCheckStatic(testClass, requireStatic, AfterAll.class, HierarchyTraversalMode.BOTTOM_UP,
+ issueReporter);
}
- static List<Method> findBeforeEachMethods(Class<?> testClass) {
- return findMethodsAndAssertNonStatic(testClass, BeforeEach.class, HierarchyTraversalMode.TOP_DOWN);
+ static List<Method> findBeforeEachMethods(Class<?> testClass, DiscoveryIssueReporter issueReporter) {
+ return findMethodsAndCheckNonStatic(testClass, BeforeEach.class, HierarchyTraversalMode.TOP_DOWN,
+ issueReporter);
}
- static List<Method> findAfterEachMethods(Class<?> testClass) {
- return findMethodsAndAssertNonStatic(testClass, AfterEach.class, HierarchyTraversalMode.BOTTOM_UP);
+ static List<Method> findAfterEachMethods(Class<?> testClass, DiscoveryIssueReporter issueReporter) {
+ return findMethodsAndCheckNonStatic(testClass, AfterEach.class, HierarchyTraversalMode.BOTTOM_UP,
+ issueReporter);
}
- private static List<Method> findMethodsAndAssertStatic(Class<?> testClass, boolean requireStatic,
- Class<? extends Annotation> annotationType, HierarchyTraversalMode traversalMode) {
+ private static List<Method> findMethodsAndCheckStatic(Class<?> testClass, boolean requireStatic,
+ Class<? extends Annotation> annotationType, HierarchyTraversalMode traversalMode,
+ DiscoveryIssueReporter issueReporter) {
- List<Method> methods = findMethodsAndCheckVoidReturnType(testClass, annotationType, traversalMode);
- if (requireStatic) {
- methods.forEach(method -> assertStatic(annotationType, method));
- }
- return methods;
+ return findMethodsAndCheckVoidReturnType(testClass, annotationType, traversalMode, issueReporter) //
+ .filter(requireStatic ? isStatic(annotationType, issueReporter) : __ -> true) //
+ .collect(toUnmodifiableList());
}
- private static List<Method> findMethodsAndAssertNonStatic(Class<?> testClass,
- Class<? extends Annotation> annotationType, HierarchyTraversalMode traversalMode) {
+ private static List<Method> findMethodsAndCheckNonStatic(Class<?> testClass,
+ Class<? extends Annotation> annotationType, HierarchyTraversalMode traversalMode,
+ DiscoveryIssueReporter issueReporter) {
- List<Method> methods = findMethodsAndCheckVoidReturnType(testClass, annotationType, traversalMode);
- methods.forEach(method -> assertNonStatic(annotationType, method));
- return methods;
+ return findMethodsAndCheckVoidReturnType(testClass, annotationType, traversalMode, issueReporter) //
+ .filter(isNotStatic(annotationType, issueReporter)) //
+ .collect(toUnmodifiableList());
}
- private static List<Method> findMethodsAndCheckVoidReturnType(Class<?> testClass,
- Class<? extends Annotation> annotationType, HierarchyTraversalMode traversalMode) {
+ private static Stream<Method> findMethodsAndCheckVoidReturnType(Class<?> testClass,
+ Class<? extends Annotation> annotationType, HierarchyTraversalMode traversalMode,
+ DiscoveryIssueReporter issueReporter) {
- List<Method> methods = findAnnotatedMethods(testClass, annotationType, traversalMode);
- methods.forEach(method -> assertVoid(annotationType, method));
- return methods;
+ return findAnnotatedMethods(testClass, annotationType, traversalMode).stream() //
+ .filter(returnsPrimitiveVoid(annotationType, issueReporter));
}
- private static void assertStatic(Class<? extends Annotation> annotationType, Method method) {
- if (ModifierSupport.isNotStatic(method)) {
- throw new JUnitException(String.format(
+ private static Predicate<Method> isStatic(Class<? extends Annotation> annotationType,
+ DiscoveryIssueReporter issueReporter) {
+ return DiscoveryIssueReportingPredicate.from(issueReporter, 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()));
- }
+ annotationType.getSimpleName(), method.toGenericString());
+ return DiscoveryIssue.builder(Severity.ERROR, message).source(MethodSource.from(method)).build();
+ });
}
- private static void assertNonStatic(Class<? extends Annotation> annotationType, Method method) {
- if (ModifierSupport.isStatic(method)) {
- throw new JUnitException(String.format("@%s method '%s' must not be static.",
- annotationType.getSimpleName(), method.toGenericString()));
- }
+ private static Predicate<Method> isNotStatic(Class<? extends Annotation> annotationType,
+ DiscoveryIssueReporter issueReporter) {
+ return DiscoveryIssueReportingPredicate.from(issueReporter, ModifierSupport::isNotStatic, method -> {
+ String message = String.format("@%s method '%s' must not be static.", annotationType.getSimpleName(),
+ method.toGenericString());
+ return DiscoveryIssue.builder(Severity.ERROR, message).source(MethodSource.from(method)).build();
+ });
}
- private static void assertVoid(Class<? extends Annotation> annotationType, Method method) {
- if (!returnsPrimitiveVoid(method)) {
- throw new JUnitException(String.format("@%s method '%s' must not return a value.",
- annotationType.getSimpleName(), method.toGenericString()));
+ private static Predicate<Method> returnsPrimitiveVoid(Class<? extends Annotation> annotationType,
+ DiscoveryIssueReporter issueReporter) {
+ return DiscoveryIssueReportingPredicate.from(issueReporter, ReflectionUtils::returnsPrimitiveVoid, method -> {
+ String message = String.format("@%s method '%s' must not return a value.", annotationType.getSimpleName(),
+ method.toGenericString());
+ return DiscoveryIssue.builder(Severity.ERROR, message).source(MethodSource.from(method)).build();
+ });
+ }
+
+ abstract static class DiscoveryIssueReportingPredicate<T> implements Predicate<T> {
+
+ static <T> DiscoveryIssueReportingPredicate<T> from(DiscoveryIssueReporter reporter, Predicate<T> predicate,
+ Function<T, DiscoveryIssue> issueBuilder) {
+ return new DiscoveryIssueReportingPredicate<T>(reporter) {
+ @Override
+ protected boolean checkCondition(T t) {
+ return predicate.test(t);
+ }
+
+ @Override
+ protected DiscoveryIssue createIssue(T t) {
+ return issueBuilder.apply(t);
+ }
+ };
+ }
+
+ private final DiscoveryIssueReporter reporter;
+
+ protected DiscoveryIssueReportingPredicate(DiscoveryIssueReporter reporter) {
+ this.reporter = reporter;
+ }
+
+ @Override
+ public final boolean test(T t) {
+ if (checkCondition(t)) {
+ return true;
+ }
+ reporter.reportIssue(createIssue(t));
+ return false;
}
+
+ protected abstract boolean checkCondition(T t);
+
+ protected abstract DiscoveryIssue createIssue(T t);
}
}
diff --git a/junit-platform-engine/src/main/java/org/junit/platform/engine/support/discovery/DiscoveryIssueReporter.java b/junit-platform-engine/src/main/java/org/junit/platform/engine/support/discovery/DiscoveryIssueReporter.java
index 39a931891c09..a087b0502e35 100644
--- a/junit-platform-engine/src/main/java/org/junit/platform/engine/support/discovery/DiscoveryIssueReporter.java
+++ b/junit-platform-engine/src/main/java/org/junit/platform/engine/support/discovery/DiscoveryIssueReporter.java
@@ -23,6 +23,7 @@
* @see SelectorResolver.Context
*/
@API(status = EXPERIMENTAL, since = "1.13")
+@FunctionalInterface
public interface DiscoveryIssueReporter {
/**
diff --git a/jupiter-tests/src/test/java/org/junit/jupiter/engine/descriptor/LifecycleMethodUtilsTests.java b/jupiter-tests/src/test/java/org/junit/jupiter/engine/descriptor/LifecycleMethodUtilsTests.java
index 719f335679bf..55d754fa52e6 100644
--- a/jupiter-tests/src/test/java/org/junit/jupiter/engine/descriptor/LifecycleMethodUtilsTests.java
+++ b/jupiter-tests/src/test/java/org/junit/jupiter/engine/descriptor/LifecycleMethodUtilsTests.java
@@ -10,15 +10,16 @@
package org.junit.jupiter.engine.descriptor;
+import static java.util.function.Predicate.isEqual;
import static org.assertj.core.api.Assertions.assertThat;
-import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.engine.descriptor.LifecycleMethodUtils.findAfterAllMethods;
import static org.junit.jupiter.engine.descriptor.LifecycleMethodUtils.findAfterEachMethods;
import static org.junit.jupiter.engine.descriptor.LifecycleMethodUtils.findBeforeAllMethods;
import static org.junit.jupiter.engine.descriptor.LifecycleMethodUtils.findBeforeEachMethods;
+import static org.junit.platform.commons.util.FunctionUtils.where;
import java.lang.reflect.Method;
+import java.util.ArrayList;
import java.util.List;
import org.junit.jupiter.api.AfterAll;
@@ -28,7 +29,9 @@
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestInstance;
import org.junit.jupiter.api.TestInstance.Lifecycle;
-import org.junit.platform.commons.JUnitException;
+import org.junit.platform.engine.DiscoveryIssue;
+import org.junit.platform.engine.DiscoveryIssue.Severity;
+import org.junit.platform.engine.support.descriptor.MethodSource;
/**
* Unit tests for {@link LifecycleMethodUtils}.
@@ -37,94 +40,120 @@
*/
class LifecycleMethodUtilsTests {
+ List<DiscoveryIssue> discoveryIssues = new ArrayList<>();
+
@Test
- void findNonVoidBeforeAllMethodsWithStandardLifecycle() {
- JUnitException exception = assertThrows(JUnitException.class,
- () -> findBeforeAllMethods(TestCaseWithNonVoidLifecyleMethods.class, true));
- assertEquals(
- "@BeforeAll method 'java.lang.Double org.junit.jupiter.engine.descriptor.TestCaseWithNonVoidLifecyleMethods.cc()' must not return a value.",
- exception.getMessage());
+ void findNonVoidBeforeAllMethodsWithStandardLifecycle() throws Exception {
+ var methods = findBeforeAllMethods(TestCaseWithNonVoidLifecyleMethods.class, true, discoveryIssues::add);
+ assertThat(methods).isEmpty();
+
+ var expectedIssue = DiscoveryIssue.builder(Severity.ERROR,
+ "@BeforeAll method 'java.lang.Double org.junit.jupiter.engine.descriptor.TestCaseWithNonVoidLifecyleMethods.cc()' must not return a value.") //
+ .source(MethodSource.from(TestCaseWithNonVoidLifecyleMethods.class.getDeclaredMethod("cc"))) //
+ .build();
+ assertThat(discoveryIssues).containsExactly(expectedIssue);
}
@Test
- void findNonVoidAfterAllMethodsWithStandardLifecycle() {
- JUnitException exception = assertThrows(JUnitException.class,
- () -> findAfterAllMethods(TestCaseWithNonVoidLifecyleMethods.class, true));
- assertEquals(
- "@AfterAll method 'java.lang.String org.junit.jupiter.engine.descriptor.TestCaseWithNonVoidLifecyleMethods.dd()' must not return a value.",
- exception.getMessage());
+ void findNonVoidAfterAllMethodsWithStandardLifecycle() throws Exception {
+ var methods = findAfterAllMethods(TestCaseWithNonVoidLifecyleMethods.class, true, discoveryIssues::add);
+ assertThat(methods).isEmpty();
+
+ var expectedIssue = DiscoveryIssue.builder(Severity.ERROR,
+ "@AfterAll method 'java.lang.String org.junit.jupiter.engine.descriptor.TestCaseWithNonVoidLifecyleMethods.dd()' must not return a value.") //
+ .source(MethodSource.from(TestCaseWithNonVoidLifecyleMethods.class.getDeclaredMethod("dd"))) //
+ .build();
+ assertThat(discoveryIssues).containsExactly(expectedIssue);
}
@Test
- void findNonVoidBeforeEachMethodsWithStandardLifecycle() {
- JUnitException exception = assertThrows(JUnitException.class,
- () -> findBeforeEachMethods(TestCaseWithNonVoidLifecyleMethods.class));
- assertEquals(
- "@BeforeEach method 'java.lang.String org.junit.jupiter.engine.descriptor.TestCaseWithNonVoidLifecyleMethods.aa()' must not return a value.",
- exception.getMessage());
+ void findNonVoidBeforeEachMethodsWithStandardLifecycle() throws Exception {
+ var methods = findBeforeEachMethods(TestCaseWithNonVoidLifecyleMethods.class, discoveryIssues::add);
+ assertThat(methods).isEmpty();
+
+ var expectedIssue = DiscoveryIssue.builder(Severity.ERROR,
+ "@BeforeEach method 'java.lang.String org.junit.jupiter.engine.descriptor.TestCaseWithNonVoidLifecyleMethods.aa()' must not return a value.") //
+ .source(MethodSource.from(TestCaseWithNonVoidLifecyleMethods.class.getDeclaredMethod("aa"))) //
+ .build();
+ assertThat(discoveryIssues).containsExactly(expectedIssue);
}
@Test
- void findNonVoidAfterEachMethodsWithStandardLifecycle() {
- JUnitException exception = assertThrows(JUnitException.class,
- () -> findAfterEachMethods(TestCaseWithNonVoidLifecyleMethods.class));
- assertEquals(
- "@AfterEach method 'int org.junit.jupiter.engine.descriptor.TestCaseWithNonVoidLifecyleMethods.bb()' must not return a value.",
- exception.getMessage());
+ void findNonVoidAfterEachMethodsWithStandardLifecycle() throws Exception {
+ var methods = findAfterEachMethods(TestCaseWithNonVoidLifecyleMethods.class, discoveryIssues::add);
+ assertThat(methods).isEmpty();
+
+ var expectedIssue = DiscoveryIssue.builder(Severity.ERROR,
+ "@AfterEach method 'int org.junit.jupiter.engine.descriptor.TestCaseWithNonVoidLifecyleMethods.bb()' must not return a value.") //
+ .source(MethodSource.from(TestCaseWithNonVoidLifecyleMethods.class.getDeclaredMethod("bb"))) //
+ .build();
+ assertThat(discoveryIssues).containsExactly(expectedIssue);
}
@Test
void findBeforeEachMethodsWithStandardLifecycle() {
- List<Method> methods = findBeforeEachMethods(TestCaseWithStandardLifecycle.class);
+ List<Method> methods = findBeforeEachMethods(TestCaseWithStandardLifecycle.class, discoveryIssues::add);
assertThat(namesOf(methods)).containsExactlyInAnyOrder("nine", "ten");
+ assertThat(discoveryIssues).isEmpty();
}
@Test
void findAfterEachMethodsWithStandardLifecycle() {
- List<Method> methods = findAfterEachMethods(TestCaseWithStandardLifecycle.class);
+ List<Method> methods = findAfterEachMethods(TestCaseWithStandardLifecycle.class, discoveryIssues::add);
assertThat(namesOf(methods)).containsExactlyInAnyOrder("eleven", "twelve");
}
@Test
void findBeforeAllMethodsWithStandardLifecycleAndWithoutRequiringStatic() {
- List<Method> methods = findBeforeAllMethods(TestCaseWithStandardLifecycle.class, false);
+ List<Method> methods = findBeforeAllMethods(TestCaseWithStandardLifecycle.class, false, discoveryIssues::add);
assertThat(namesOf(methods)).containsExactly("one");
+ assertThat(discoveryIssues).isEmpty();
}
@Test
- void findBeforeAllMethodsWithStandardLifecycleAndRequiringStatic() {
- JUnitException exception = assertThrows(JUnitException.class,
- () -> findBeforeAllMethods(TestCaseWithStandardLifecycle.class, true));
- assertEquals(
- "@BeforeAll method 'void org.junit.jupiter.engine.descriptor.TestCaseWithStandardLifecycle.one()' must be static unless the test class is annotated with @TestInstance(Lifecycle.PER_CLASS).",
- exception.getMessage());
+ void findBeforeAllMethodsWithStandardLifecycleAndRequiringStatic() throws Exception {
+ var methods = findBeforeAllMethods(TestCaseWithStandardLifecycle.class, true, discoveryIssues::add);
+ assertThat(methods).isEmpty();
+
+ var expectedIssue = DiscoveryIssue.builder(Severity.ERROR,
+ "@BeforeAll method 'void org.junit.jupiter.engine.descriptor.TestCaseWithStandardLifecycle.one()' must be static unless the test class is annotated with @TestInstance(Lifecycle.PER_CLASS).") //
+ .source(MethodSource.from(TestCaseWithStandardLifecycle.class.getDeclaredMethod("one"))) //
+ .build();
+ assertThat(discoveryIssues).containsExactly(expectedIssue);
}
@Test
void findBeforeAllMethodsWithLifeCyclePerClassAndRequiringStatic() {
- List<Method> methods = findBeforeAllMethods(TestCaseWithLifecyclePerClass.class, false);
+ List<Method> methods = findBeforeAllMethods(TestCaseWithLifecyclePerClass.class, false, discoveryIssues::add);
assertThat(namesOf(methods)).containsExactlyInAnyOrder("three", "four");
+ assertThat(discoveryIssues).isEmpty();
}
@Test
void findAfterAllMethodsWithStandardLifecycleAndWithoutRequiringStatic() {
- List<Method> methods = findAfterAllMethods(TestCaseWithStandardLifecycle.class, false);
+ List<Method> methods = findAfterAllMethods(TestCaseWithStandardLifecycle.class, false, discoveryIssues::add);
assertThat(namesOf(methods)).containsExactlyInAnyOrder("five", "six");
+ assertThat(discoveryIssues).isEmpty();
}
@Test
void findAfterAllMethodsWithStandardLifecycleAndRequiringStatic() {
- assertThrows(JUnitException.class, () -> findAfterAllMethods(TestCaseWithStandardLifecycle.class, true));
+ var methods = findAfterAllMethods(TestCaseWithStandardLifecycle.class, true, discoveryIssues::add);
+ assertThat(methods).isEmpty();
+
+ assertThat(discoveryIssues) //
+ .filteredOn(where(DiscoveryIssue::severity, isEqual(Severity.ERROR))) //
+ .isNotEmpty();
}
@Test
void findAfterAllMethodsWithLifeCyclePerClassAndRequiringStatic() {
- List<Method> methods = findAfterAllMethods(TestCaseWithLifecyclePerClass.class, false);
+ List<Method> methods = findAfterAllMethods(TestCaseWithLifecyclePerClass.class, false, discoveryIssues::add);
assertThat(namesOf(methods)).containsExactlyInAnyOrder("seven", "eight");
}
From 050c13249e6c2b059c5ddbf8711df5d447655725 Mon Sep 17 00:00:00 2001
From: Marc Philipp <mail@marcphilipp.de>
Date: Mon, 17 Mar 2025 17:57:25 +0100
Subject: [PATCH 2/4] Report invalid lifecycle methods as discovery issues
Issue: #242
---
.../descriptor/ClassBasedTestDescriptor.java | 72 +++++++++++++------
.../descriptor/LifecycleMethodUtils.java | 45 ++++++++----
.../engine/descriptor/Validatable.java | 32 +++++++++
.../discovery/DiscoverySelectorResolver.java | 14 ++--
.../src/nativeImage/initialize-at-build-time | 1 +
...alidLifecycleMethodConfigurationTests.java | 62 ++++++----------
...stInstanceLifecycleConfigurationTests.java | 6 +-
.../descriptor/LifecycleMethodUtilsTests.java | 16 +++--
8 files changed, 158 insertions(+), 90 deletions(-)
create mode 100644 junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/Validatable.java
diff --git a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/ClassBasedTestDescriptor.java b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/ClassBasedTestDescriptor.java
index dd7c41f40f52..7186b34f3226 100644
--- a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/ClassBasedTestDescriptor.java
+++ b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/ClassBasedTestDescriptor.java
@@ -71,10 +71,12 @@
import org.junit.platform.commons.util.ReflectionUtils;
import org.junit.platform.commons.util.StringUtils;
import org.junit.platform.commons.util.UnrecoverableExceptions;
+import org.junit.platform.engine.DiscoveryIssue;
import org.junit.platform.engine.TestDescriptor;
import org.junit.platform.engine.TestTag;
import org.junit.platform.engine.UniqueId;
import org.junit.platform.engine.support.descriptor.ClassSource;
+import org.junit.platform.engine.support.discovery.DiscoveryIssueReporter;
import org.junit.platform.engine.support.hierarchical.ThrowableCollector;
/**
@@ -84,21 +86,21 @@
*/
@API(status = INTERNAL, since = "5.5")
public abstract class ClassBasedTestDescriptor extends JupiterTestDescriptor
- implements ResourceLockAware, TestClassAware {
+ implements ResourceLockAware, TestClassAware, Validatable {
private static final InterceptingExecutableInvoker executableInvoker = new InterceptingExecutableInvoker();
protected final ClassInfo classInfo;
+ private LifecycleMethods lifecycleMethods;
private TestInstanceFactory testInstanceFactory;
- private List<Method> beforeAllMethods;
- private List<Method> afterAllMethods;
ClassBasedTestDescriptor(UniqueId uniqueId, Class<?> testClass, Supplier<String> displayNameSupplier,
JupiterConfiguration configuration) {
super(uniqueId, testClass, displayNameSupplier, ClassSource.from(testClass), configuration);
this.classInfo = new ClassInfo(testClass, configuration);
+ this.lifecycleMethods = new LifecycleMethods(this.classInfo);
}
ClassBasedTestDescriptor(UniqueId uniqueId, Class<?> testClass, String displayName,
@@ -106,6 +108,7 @@ public abstract class ClassBasedTestDescriptor extends JupiterTestDescriptor
super(uniqueId, displayName, ClassSource.from(testClass), configuration);
this.classInfo = new ClassInfo(testClass, configuration);
+ this.lifecycleMethods = new LifecycleMethods(this.classInfo);
}
// --- TestClassAware ------------------------------------------------------
@@ -127,6 +130,15 @@ public final String getLegacyReportingName() {
return getTestClass().getName();
}
+ // --- Validatable ---------------------------------------------------------
+
+ @Override
+ public void validate(DiscoveryIssueReporter reporter) {
+ List<DiscoveryIssue> discoveryIssues = this.lifecycleMethods.discoveryIssues;
+ discoveryIssues.forEach(reporter::reportIssue);
+ discoveryIssues.clear();
+ }
+
// --- Node ----------------------------------------------------------------
@Override
@@ -166,16 +178,7 @@ public final JupiterEngineExecutionContext prepare(JupiterEngineExecutionContext
registerExtensionsFromConstructorParameters(registry, getTestClass());
}
- this.beforeAllMethods = findBeforeAllMethods(getTestClass(), this.classInfo.lifecycle == Lifecycle.PER_METHOD,
- issue -> {
- throw new JUnitException(issue.message());
- });
- this.afterAllMethods = findAfterAllMethods(getTestClass(), this.classInfo.lifecycle == Lifecycle.PER_METHOD,
- issue -> {
- throw new JUnitException(issue.message());
- });
-
- this.beforeAllMethods.forEach(method -> registerExtensionsFromExecutableParameters(registry, method));
+ this.lifecycleMethods.beforeAll.forEach(method -> registerExtensionsFromExecutableParameters(registry, method));
// Since registerBeforeEachMethodAdapters() and registerAfterEachMethodAdapters() also
// invoke registerExtensionsFromExecutableParameters(), we invoke those methods before
// invoking registerExtensionsFromExecutableParameters() for @AfterAll methods,
@@ -183,7 +186,7 @@ public final JupiterEngineExecutionContext prepare(JupiterEngineExecutionContext
// on parameters in lifecycle methods.
registerBeforeEachMethodAdapters(registry);
registerAfterEachMethodAdapters(registry);
- this.afterAllMethods.forEach(method -> registerExtensionsFromExecutableParameters(registry, method));
+ this.lifecycleMethods.afterAll.forEach(method -> registerExtensionsFromExecutableParameters(registry, method));
registerExtensionsFromInstanceFields(registry, getTestClass());
ThrowableCollector throwableCollector = createThrowableCollector();
@@ -258,6 +261,13 @@ public final void after(JupiterEngineExecutionContext context) {
}
}
+ @Override
+ public void cleanUp(JupiterEngineExecutionContext context) throws Exception {
+ super.cleanUp(context);
+ this.lifecycleMethods = null;
+ this.testInstanceFactory = null;
+ }
+
private TestInstanceFactory resolveTestInstanceFactory(ExtensionRegistry registry) {
List<TestInstanceFactory> factories = registry.getExtensions(TestInstanceFactory.class);
@@ -410,7 +420,7 @@ private void invokeBeforeAllMethods(JupiterEngineExecutionContext context) {
ThrowableCollector throwableCollector = context.getThrowableCollector();
Object testInstance = extensionContext.getTestInstance().orElse(null);
- for (Method method : this.beforeAllMethods) {
+ for (Method method : this.lifecycleMethods.beforeAll) {
throwableCollector.execute(() -> {
try {
executableInvoker.invoke(method, testInstance, extensionContext, registry,
@@ -439,7 +449,7 @@ private void invokeAfterAllMethods(JupiterEngineExecutionContext context) {
ThrowableCollector throwableCollector = context.getThrowableCollector();
Object testInstance = extensionContext.getTestInstance().orElse(null);
- this.afterAllMethods.forEach(method -> throwableCollector.execute(() -> {
+ this.lifecycleMethods.afterAll.forEach(method -> throwableCollector.execute(() -> {
try {
executableInvoker.invoke(method, testInstance, extensionContext, registry,
ReflectiveInterceptorCall.ofVoidMethod(InvocationInterceptor::interceptAfterAllMethod));
@@ -472,17 +482,13 @@ private boolean isPerClassLifecycle(JupiterEngineExecutionContext context) {
}
private void registerBeforeEachMethodAdapters(ExtensionRegistrar registrar) {
- List<Method> beforeEachMethods = findBeforeEachMethods(getTestClass(), issue -> {
- throw new JUnitException(issue.message());
- });
- registerMethodsAsExtensions(beforeEachMethods, registrar, this::synthesizeBeforeEachMethodAdapter);
+ registerMethodsAsExtensions(this.lifecycleMethods.beforeEach, registrar,
+ this::synthesizeBeforeEachMethodAdapter);
}
private void registerAfterEachMethodAdapters(ExtensionRegistrar registrar) {
// Make a local copy since findAfterEachMethods() returns an immutable list.
- List<Method> afterEachMethods = new ArrayList<>(findAfterEachMethods(getTestClass(), issue -> {
- throw new JUnitException(issue.message());
- }));
+ List<Method> afterEachMethods = new ArrayList<>(this.lifecycleMethods.afterEach);
// Since the bottom-up ordering of afterEachMethods will later be reversed when the
// synthesized AfterEachMethodAdapters are executed within TestMethodTestDescriptor,
@@ -523,6 +529,7 @@ private void invokeMethodInExtensionContext(Method method, ExtensionContext cont
}
protected static class ClassInfo {
+
final Class<?> testClass;
final Set<TestTag> tags;
final Lifecycle lifecycle;
@@ -538,4 +545,23 @@ protected static class ClassInfo {
}
}
+ private static class LifecycleMethods {
+
+ private final List<DiscoveryIssue> discoveryIssues = new ArrayList<>();
+
+ private final List<Method> beforeAll;
+ private final List<Method> afterAll;
+ private final List<Method> beforeEach;
+ private final List<Method> afterEach;
+
+ LifecycleMethods(ClassInfo classInfo) {
+ Class<?> testClass = classInfo.testClass;
+ boolean requireStatic = classInfo.lifecycle == Lifecycle.PER_METHOD;
+ this.beforeAll = findBeforeAllMethods(testClass, requireStatic, discoveryIssues::add);
+ this.afterAll = findAfterAllMethods(testClass, requireStatic, discoveryIssues::add);
+ this.beforeEach = findBeforeEachMethods(testClass, discoveryIssues::add);
+ this.afterEach = findAfterEachMethods(testClass, discoveryIssues::add);
+ }
+ }
+
}
diff --git a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/LifecycleMethodUtils.java b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/LifecycleMethodUtils.java
index 507660732a68..f277ab66a46b 100644
--- a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/LifecycleMethodUtils.java
+++ b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/LifecycleMethodUtils.java
@@ -18,7 +18,6 @@
import java.util.List;
import java.util.function.Function;
import java.util.function.Predicate;
-import java.util.stream.Stream;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.AfterEach;
@@ -26,6 +25,7 @@
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;
@@ -69,31 +69,31 @@ private static List<Method> findMethodsAndCheckStatic(Class<?> testClass, boolea
Class<? extends Annotation> annotationType, HierarchyTraversalMode traversalMode,
DiscoveryIssueReporter issueReporter) {
- return findMethodsAndCheckVoidReturnType(testClass, annotationType, traversalMode, issueReporter) //
- .filter(requireStatic ? isStatic(annotationType, issueReporter) : __ -> true) //
- .collect(toUnmodifiableList());
+ Predicate<Method> additionalCondition = requireStatic ? isStatic(annotationType, issueReporter) : __ -> true;
+ return findMethodsAndCheckVoidReturnType(testClass, annotationType, traversalMode, issueReporter,
+ additionalCondition);
}
private static List<Method> findMethodsAndCheckNonStatic(Class<?> testClass,
Class<? extends Annotation> annotationType, HierarchyTraversalMode traversalMode,
DiscoveryIssueReporter issueReporter) {
- return findMethodsAndCheckVoidReturnType(testClass, annotationType, traversalMode, issueReporter) //
- .filter(isNotStatic(annotationType, issueReporter)) //
- .collect(toUnmodifiableList());
+ return findMethodsAndCheckVoidReturnType(testClass, annotationType, traversalMode, issueReporter,
+ isNotStatic(annotationType, issueReporter));
}
- private static Stream<Method> findMethodsAndCheckVoidReturnType(Class<?> testClass,
+ private static List<Method> findMethodsAndCheckVoidReturnType(Class<?> testClass,
Class<? extends Annotation> annotationType, HierarchyTraversalMode traversalMode,
- DiscoveryIssueReporter issueReporter) {
+ DiscoveryIssueReporter issueReporter, Predicate<? super Method> additionalCondition) {
return findAnnotatedMethods(testClass, annotationType, traversalMode).stream() //
- .filter(returnsPrimitiveVoid(annotationType, issueReporter));
+ .filter(allOf(returnsPrimitiveVoid(annotationType, issueReporter), additionalCondition)) //
+ .collect(toUnmodifiableList());
}
private static Predicate<Method> isStatic(Class<? extends Annotation> annotationType,
DiscoveryIssueReporter issueReporter) {
- return DiscoveryIssueReportingPredicate.from(issueReporter, ModifierSupport::isStatic, method -> {
+ return DiscoveryIssueReportingPredicate.from(ModifierSupport::isStatic, issueReporter, 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());
@@ -103,7 +103,7 @@ private static Predicate<Method> isStatic(Class<? extends Annotation> annotation
private static Predicate<Method> isNotStatic(Class<? extends Annotation> annotationType,
DiscoveryIssueReporter issueReporter) {
- return DiscoveryIssueReportingPredicate.from(issueReporter, ModifierSupport::isNotStatic, method -> {
+ return DiscoveryIssueReportingPredicate.from(ModifierSupport::isNotStatic, issueReporter, method -> {
String message = String.format("@%s method '%s' must not be static.", annotationType.getSimpleName(),
method.toGenericString());
return DiscoveryIssue.builder(Severity.ERROR, message).source(MethodSource.from(method)).build();
@@ -112,16 +112,31 @@ private static Predicate<Method> isNotStatic(Class<? extends Annotation> annotat
private static Predicate<Method> returnsPrimitiveVoid(Class<? extends Annotation> annotationType,
DiscoveryIssueReporter issueReporter) {
- return DiscoveryIssueReportingPredicate.from(issueReporter, ReflectionUtils::returnsPrimitiveVoid, method -> {
+ return DiscoveryIssueReportingPredicate.from(ReflectionUtils::returnsPrimitiveVoid, issueReporter, method -> {
String message = String.format("@%s method '%s' must not return a value.", annotationType.getSimpleName(),
method.toGenericString());
return DiscoveryIssue.builder(Severity.ERROR, message).source(MethodSource.from(method)).build();
});
}
- abstract static class DiscoveryIssueReportingPredicate<T> implements Predicate<T> {
+ @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> {
- static <T> DiscoveryIssueReportingPredicate<T> from(DiscoveryIssueReporter reporter, Predicate<T> predicate,
+ static <T> DiscoveryIssueReportingPredicate<T> from(Predicate<T> predicate, DiscoveryIssueReporter reporter,
Function<T, DiscoveryIssue> issueBuilder) {
return new DiscoveryIssueReportingPredicate<T>(reporter) {
@Override
diff --git a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/Validatable.java b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/Validatable.java
new file mode 100644
index 000000000000..b4c83e1944ad
--- /dev/null
+++ b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/Validatable.java
@@ -0,0 +1,32 @@
+/*
+ * Copyright 2015-2025 the original author or authors.
+ *
+ * All rights reserved. This program and the accompanying materials are
+ * made available under the terms of the Eclipse Public License v2.0 which
+ * accompanies this distribution and is available at
+ *
+ * https://www.eclipse.org/legal/epl-v20.html
+ */
+
+package org.junit.jupiter.engine.descriptor;
+
+import static org.apiguardian.api.API.Status.INTERNAL;
+
+import org.apiguardian.api.API;
+import org.junit.platform.engine.support.discovery.DiscoveryIssueReporter;
+
+/**
+ * Interface for descriptors that can be validated during discovery.
+ *
+ * @since 5.13
+ */
+@API(status = INTERNAL, since = "5.13")
+public interface Validatable {
+
+ /**
+ * Validate the state of this descriptor and report any issues found to the
+ * supplied {@link DiscoveryIssueReporter}.
+ */
+ void validate(DiscoveryIssueReporter reporter);
+
+}
diff --git a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/DiscoverySelectorResolver.java b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/DiscoverySelectorResolver.java
index 89a79c679dec..1198d6da5a6d 100644
--- a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/DiscoverySelectorResolver.java
+++ b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/DiscoverySelectorResolver.java
@@ -16,6 +16,7 @@
import org.junit.jupiter.engine.config.JupiterConfiguration;
import org.junit.jupiter.engine.descriptor.JupiterEngineDescriptor;
import org.junit.jupiter.engine.descriptor.JupiterTestDescriptor;
+import org.junit.jupiter.engine.descriptor.Validatable;
import org.junit.jupiter.engine.discovery.predicates.IsTestClassWithTests;
import org.junit.platform.engine.EngineDiscoveryRequest;
import org.junit.platform.engine.TestDescriptor;
@@ -44,12 +45,15 @@ public class DiscoverySelectorResolver {
new ClassOrderingVisitor(getConfiguration(ctx)), //
new MethodOrderingVisitor(getConfiguration(ctx)), //
descriptor -> {
- if (descriptor instanceof JupiterTestDescriptor) {
- ((JupiterTestDescriptor) descriptor).prunePriorToFiltering();
+ if (descriptor instanceof Validatable) {
+ ((Validatable) descriptor).validate(ctx.getIssueReporter());
}
- } //
- )) //
- .build();
+ })) //
+ .addTestDescriptorVisitor(ctx -> descriptor -> {
+ if (descriptor instanceof JupiterTestDescriptor) {
+ ((JupiterTestDescriptor) descriptor).prunePriorToFiltering();
+ }
+ }).build();
private static JupiterConfiguration getConfiguration(InitializationContext<JupiterEngineDescriptor> context) {
return context.getEngineDescriptor().getConfiguration();
diff --git a/junit-jupiter-engine/src/nativeImage/initialize-at-build-time b/junit-jupiter-engine/src/nativeImage/initialize-at-build-time
index df2adf10a9ba..4e405ef9656f 100644
--- a/junit-jupiter-engine/src/nativeImage/initialize-at-build-time
+++ b/junit-jupiter-engine/src/nativeImage/initialize-at-build-time
@@ -6,6 +6,7 @@ org.junit.jupiter.engine.config.InstantiatingConfigurationParameterConverter
org.junit.jupiter.engine.descriptor.ClassTestDescriptor
org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor
org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor$ClassInfo
+org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor$LifecycleMethods
org.junit.jupiter.engine.descriptor.DynamicDescendantFilter
org.junit.jupiter.engine.descriptor.ExclusiveResourceCollector$1
org.junit.jupiter.engine.descriptor.JupiterEngineDescriptor
diff --git a/jupiter-tests/src/test/java/org/junit/jupiter/engine/InvalidLifecycleMethodConfigurationTests.java b/jupiter-tests/src/test/java/org/junit/jupiter/engine/InvalidLifecycleMethodConfigurationTests.java
index e1d7a47cfce6..a17b1fb8d4a6 100644
--- a/jupiter-tests/src/test/java/org/junit/jupiter/engine/InvalidLifecycleMethodConfigurationTests.java
+++ b/jupiter-tests/src/test/java/org/junit/jupiter/engine/InvalidLifecycleMethodConfigurationTests.java
@@ -10,82 +10,64 @@
package org.junit.jupiter.engine;
-import static org.junit.jupiter.api.Assertions.assertAll;
-import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.platform.engine.discovery.DiscoverySelectors.selectClass;
+import static java.util.function.Predicate.isEqual;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.platform.commons.util.FunctionUtils.where;
+
+import java.lang.annotation.Annotation;
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.jupiter.api.Test;
-import org.junit.platform.testkit.engine.EngineExecutionResults;
-import org.junit.platform.testkit.engine.Events;
+import org.junit.platform.engine.DiscoveryIssue;
+import org.junit.platform.engine.DiscoveryIssue.Severity;
/**
* Integration tests that verify proper handling of invalid configuration for
* lifecycle methods in conjunction with the {@link JupiterTestEngine}.
*
- * <p>In general, configuration errors should not be thrown until the
- * execution phase, thereby giving all containers a chance to execute.
- *
* @since 5.0
*/
class InvalidLifecycleMethodConfigurationTests extends AbstractJupiterTestEngineTests {
@Test
void executeValidTestCaseAlongsideTestCaseWithInvalidNonStaticBeforeAllDeclaration() {
- assertContainerFailed(TestCaseWithInvalidNonStaticBeforeAllMethod.class);
+ assertReportsError(TestCaseWithInvalidNonStaticBeforeAllMethod.class, BeforeAll.class);
}
@Test
void executeValidTestCaseAlongsideTestCaseWithInvalidNonStaticAfterAllDeclaration() {
- assertContainerFailed(TestCaseWithInvalidNonStaticAfterAllMethod.class);
+ assertReportsError(TestCaseWithInvalidNonStaticAfterAllMethod.class, AfterAll.class);
}
@Test
void executeValidTestCaseAlongsideTestCaseWithInvalidStaticBeforeEachDeclaration() {
- assertContainerFailed(TestCaseWithInvalidStaticBeforeEachMethod.class);
+ assertReportsError(TestCaseWithInvalidStaticBeforeEachMethod.class, BeforeEach.class);
}
@Test
void executeValidTestCaseAlongsideTestCaseWithInvalidStaticAfterEachDeclaration() {
- assertContainerFailed(TestCaseWithInvalidStaticAfterEachMethod.class);
+ assertReportsError(TestCaseWithInvalidStaticAfterEachMethod.class, AfterEach.class);
}
- private void assertContainerFailed(Class<?> invalidTestClass) {
- EngineExecutionResults executionResults = executeTests(selectClass(TestCase.class),
- selectClass(invalidTestClass));
- Events containers = executionResults.containerEvents();
- Events tests = executionResults.testEvents();
-
- // @formatter:off
- assertAll(
- () -> assertEquals(3, containers.started().count(), "# containers started"),
- () -> assertEquals(1, tests.started().count(), "# tests started"),
- () -> assertEquals(1, tests.succeeded().count(), "# tests succeeded"),
- () -> assertEquals(0, tests.failed().count(), "# tests failed"),
- () -> assertEquals(3, containers.finished().count(), "# containers finished"),
- () -> assertEquals(1, containers.failed().count(), "# containers failed")
- );
- // @formatter:on
+ private void assertReportsError(Class<?> invalidTestClass, Class<? extends Annotation> annotationType) {
+ var results = discoverTestsForClass(invalidTestClass);
+
+ assertThat(results.getDiscoveryIssues()) //
+ .filteredOn(where(DiscoveryIssue::severity, isEqual(Severity.ERROR))) //
+ .extracting(DiscoveryIssue::message) //
+ .asString().contains("@%s method".formatted(annotationType.getSimpleName()));
}
// -------------------------------------------------------------------------
- @SuppressWarnings("JUnitMalformedDeclaration")
- static class TestCase {
-
- @Test
- void test() {
- }
- }
-
@SuppressWarnings("JUnitMalformedDeclaration")
static class TestCaseWithInvalidNonStaticBeforeAllMethod {
// must be static
- @SuppressWarnings("JUnitMalformedDeclaration")
+ @SuppressWarnings("unused")
@BeforeAll
void beforeAll() {
}
@@ -99,7 +81,7 @@ void test() {
static class TestCaseWithInvalidNonStaticAfterAllMethod {
// must be static
- @SuppressWarnings("JUnitMalformedDeclaration")
+ @SuppressWarnings("unused")
@AfterAll
void afterAll() {
}
@@ -113,7 +95,7 @@ void test() {
static class TestCaseWithInvalidStaticBeforeEachMethod {
// must NOT be static
- @SuppressWarnings("JUnitMalformedDeclaration")
+ @SuppressWarnings("unused")
@BeforeEach
static void beforeEach() {
}
@@ -127,7 +109,7 @@ void test() {
static class TestCaseWithInvalidStaticAfterEachMethod {
// must NOT be static
- @SuppressWarnings("JUnitMalformedDeclaration")
+ @SuppressWarnings("unused")
@AfterEach
static void afterEach() {
}
diff --git a/jupiter-tests/src/test/java/org/junit/jupiter/engine/TestInstanceLifecycleConfigurationTests.java b/jupiter-tests/src/test/java/org/junit/jupiter/engine/TestInstanceLifecycleConfigurationTests.java
index fdb7e512ce75..8090690f32f6 100644
--- a/jupiter-tests/src/test/java/org/junit/jupiter/engine/TestInstanceLifecycleConfigurationTests.java
+++ b/jupiter-tests/src/test/java/org/junit/jupiter/engine/TestInstanceLifecycleConfigurationTests.java
@@ -73,7 +73,7 @@ void instancePerClassConfiguredViaSystemProperty() {
Class<?> testClass = AssumedInstancePerClassTestCase.class;
// Should fail by default...
- performAssertions(testClass, 2, 1, 0);
+ performAssertions(testClass, 1, 1, 0);
// Should pass with the system property set
System.setProperty(KEY, PER_CLASS.name());
@@ -85,7 +85,7 @@ void instancePerClassConfiguredViaConfigParam() {
Class<?> testClass = AssumedInstancePerClassTestCase.class;
// Should fail by default...
- performAssertions(testClass, 2, 1, 0);
+ performAssertions(testClass, 1, 1, 0);
// Should pass with the config param
performAssertions(testClass, singletonMap(KEY, PER_CLASS.name()), 2, 0, 1, "beforeAll", "test", "afterAll");
@@ -97,7 +97,7 @@ void instancePerClassConfiguredViaConfigParamThatOverridesSystemProperty() {
// Should fail with system property
System.setProperty(KEY, PER_METHOD.name());
- performAssertions(testClass, 2, 1, 0);
+ performAssertions(testClass, 1, 1, 0);
// Should pass with the config param
performAssertions(testClass, singletonMap(KEY, PER_CLASS.name()), 2, 0, 1, "beforeAll", "test", "afterAll");
diff --git a/jupiter-tests/src/test/java/org/junit/jupiter/engine/descriptor/LifecycleMethodUtilsTests.java b/jupiter-tests/src/test/java/org/junit/jupiter/engine/descriptor/LifecycleMethodUtilsTests.java
index 55d754fa52e6..7e122432ad56 100644
--- a/jupiter-tests/src/test/java/org/junit/jupiter/engine/descriptor/LifecycleMethodUtilsTests.java
+++ b/jupiter-tests/src/test/java/org/junit/jupiter/engine/descriptor/LifecycleMethodUtilsTests.java
@@ -47,11 +47,15 @@ void findNonVoidBeforeAllMethodsWithStandardLifecycle() throws Exception {
var methods = findBeforeAllMethods(TestCaseWithNonVoidLifecyleMethods.class, true, discoveryIssues::add);
assertThat(methods).isEmpty();
- var expectedIssue = DiscoveryIssue.builder(Severity.ERROR,
+ var notVoidIssue = DiscoveryIssue.builder(Severity.ERROR,
"@BeforeAll method 'java.lang.Double org.junit.jupiter.engine.descriptor.TestCaseWithNonVoidLifecyleMethods.cc()' must not return a value.") //
.source(MethodSource.from(TestCaseWithNonVoidLifecyleMethods.class.getDeclaredMethod("cc"))) //
.build();
- assertThat(discoveryIssues).containsExactly(expectedIssue);
+ var notStaticIssue = DiscoveryIssue.builder(Severity.ERROR,
+ "@BeforeAll method 'java.lang.Double org.junit.jupiter.engine.descriptor.TestCaseWithNonVoidLifecyleMethods.cc()' must be static unless the test class is annotated with @TestInstance(Lifecycle.PER_CLASS).") //
+ .source(MethodSource.from(TestCaseWithNonVoidLifecyleMethods.class.getDeclaredMethod("cc"))) //
+ .build();
+ assertThat(discoveryIssues).containsExactlyInAnyOrder(notVoidIssue, notStaticIssue);
}
@Test
@@ -59,11 +63,15 @@ void findNonVoidAfterAllMethodsWithStandardLifecycle() throws Exception {
var methods = findAfterAllMethods(TestCaseWithNonVoidLifecyleMethods.class, true, discoveryIssues::add);
assertThat(methods).isEmpty();
- var expectedIssue = DiscoveryIssue.builder(Severity.ERROR,
+ var notVoidIssue = DiscoveryIssue.builder(Severity.ERROR,
"@AfterAll method 'java.lang.String org.junit.jupiter.engine.descriptor.TestCaseWithNonVoidLifecyleMethods.dd()' must not return a value.") //
.source(MethodSource.from(TestCaseWithNonVoidLifecyleMethods.class.getDeclaredMethod("dd"))) //
.build();
- assertThat(discoveryIssues).containsExactly(expectedIssue);
+ var notStaticIssue = DiscoveryIssue.builder(Severity.ERROR,
+ "@AfterAll method 'java.lang.String org.junit.jupiter.engine.descriptor.TestCaseWithNonVoidLifecyleMethods.dd()' must be static unless the test class is annotated with @TestInstance(Lifecycle.PER_CLASS).") //
+ .source(MethodSource.from(TestCaseWithNonVoidLifecyleMethods.class.getDeclaredMethod("dd"))) //
+ .build();
+ assertThat(discoveryIssues).containsExactlyInAnyOrder(notVoidIssue, notStaticIssue);
}
@Test
From 49acd3fc1bfa43fa2495b197ca4ed5f47abb11b7 Mon Sep 17 00:00:00 2001
From: Marc Philipp <mail@marcphilipp.de>
Date: Tue, 18 Mar 2025 14:44:51 +0100
Subject: [PATCH 3/4] Generate deprecation issue for private lifecycle methods
Issue: #242
---
.../descriptor/LifecycleMethodUtils.java | 47 ++++++++---
.../descriptor/LifecycleMethodUtilsTests.java | 79 +++++++++++--------
2 files changed, 82 insertions(+), 44 deletions(-)
diff --git a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/LifecycleMethodUtils.java b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/LifecycleMethodUtils.java
index f277ab66a46b..99693164d075 100644
--- a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/LifecycleMethodUtils.java
+++ b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/LifecycleMethodUtils.java
@@ -16,6 +16,7 @@
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;
@@ -87,6 +88,7 @@ private static List<Method> findMethodsAndCheckVoidReturnType(Class<?> testClass
DiscoveryIssueReporter issueReporter, Predicate<? super Method> additionalCondition) {
return findAnnotatedMethods(testClass, annotationType, traversalMode).stream() //
+ .peek(isNotPrivate(annotationType, issueReporter)) //
.filter(allOf(returnsPrimitiveVoid(annotationType, issueReporter), additionalCondition)) //
.collect(toUnmodifiableList());
}
@@ -97,7 +99,7 @@ private static Predicate<Method> isStatic(Class<? extends Annotation> annotation
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 DiscoveryIssue.builder(Severity.ERROR, message).source(MethodSource.from(method)).build();
+ return createIssue(Severity.ERROR, message, method);
});
}
@@ -106,7 +108,17 @@ private static Predicate<Method> isNotStatic(Class<? extends Annotation> annotat
return DiscoveryIssueReportingPredicate.from(ModifierSupport::isNotStatic, issueReporter, method -> {
String message = String.format("@%s method '%s' must not be static.", annotationType.getSimpleName(),
method.toGenericString());
- return DiscoveryIssue.builder(Severity.ERROR, message).source(MethodSource.from(method)).build();
+ return createIssue(Severity.ERROR, message, method);
+ });
+ }
+
+ private static Consumer<Method> isNotPrivate(Class<? extends Annotation> annotationType,
+ DiscoveryIssueReporter issueReporter) {
+ return DiscoveryIssueReportingPredicate.from(ModifierSupport::isNotPrivate, issueReporter, 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);
});
}
@@ -115,10 +127,14 @@ private static Predicate<Method> returnsPrimitiveVoid(Class<? extends Annotation
return DiscoveryIssueReportingPredicate.from(ReflectionUtils::returnsPrimitiveVoid, issueReporter, method -> {
String message = String.format("@%s method '%s' must not return a value.", annotationType.getSimpleName(),
method.toGenericString());
- return DiscoveryIssue.builder(Severity.ERROR, message).source(MethodSource.from(method)).build();
+ return createIssue(Severity.ERROR, message, method);
});
}
+ private static DiscoveryIssue createIssue(Severity severity, String message, Method method) {
+ return DiscoveryIssue.builder(severity, message).source(MethodSource.from(method)).build();
+ }
+
@SafeVarargs
@SuppressWarnings("varargs")
private static <T> Predicate<T> allOf(Predicate<? super T>... predicates) {
@@ -134,19 +150,19 @@ private static <T> Predicate<T> allOf(Predicate<? super T>... predicates) {
};
}
- private abstract static class DiscoveryIssueReportingPredicate<T> implements Predicate<T> {
+ 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 t) {
- return predicate.test(t);
+ protected boolean checkCondition(T value) {
+ return predicate.test(value);
}
@Override
- protected DiscoveryIssue createIssue(T t) {
- return issueBuilder.apply(t);
+ protected DiscoveryIssue createIssue(T value) {
+ return issueBuilder.apply(value);
}
};
}
@@ -158,17 +174,22 @@ protected DiscoveryIssueReportingPredicate(DiscoveryIssueReporter reporter) {
}
@Override
- public final boolean test(T t) {
- if (checkCondition(t)) {
+ public final boolean test(T value) {
+ if (checkCondition(value)) {
return true;
}
- reporter.reportIssue(createIssue(t));
+ reporter.reportIssue(createIssue(value));
return false;
}
- protected abstract boolean checkCondition(T t);
+ @Override
+ public void accept(T value) {
+ test(value);
+ }
+
+ protected abstract boolean checkCondition(T value);
- protected abstract DiscoveryIssue createIssue(T t);
+ protected abstract DiscoveryIssue createIssue(T value);
}
}
diff --git a/jupiter-tests/src/test/java/org/junit/jupiter/engine/descriptor/LifecycleMethodUtilsTests.java b/jupiter-tests/src/test/java/org/junit/jupiter/engine/descriptor/LifecycleMethodUtilsTests.java
index 7e122432ad56..2da2aa759d2e 100644
--- a/jupiter-tests/src/test/java/org/junit/jupiter/engine/descriptor/LifecycleMethodUtilsTests.java
+++ b/jupiter-tests/src/test/java/org/junit/jupiter/engine/descriptor/LifecycleMethodUtilsTests.java
@@ -44,58 +44,78 @@ class LifecycleMethodUtilsTests {
@Test
void findNonVoidBeforeAllMethodsWithStandardLifecycle() throws Exception {
- var methods = findBeforeAllMethods(TestCaseWithNonVoidLifecyleMethods.class, true, discoveryIssues::add);
+ var methods = findBeforeAllMethods(TestCaseWithInvalidLifecycleMethods.class, true, discoveryIssues::add);
assertThat(methods).isEmpty();
+ var methodSource = MethodSource.from(TestCaseWithInvalidLifecycleMethods.class.getDeclaredMethod("cc"));
var notVoidIssue = DiscoveryIssue.builder(Severity.ERROR,
- "@BeforeAll method 'java.lang.Double org.junit.jupiter.engine.descriptor.TestCaseWithNonVoidLifecyleMethods.cc()' must not return a value.") //
- .source(MethodSource.from(TestCaseWithNonVoidLifecyleMethods.class.getDeclaredMethod("cc"))) //
+ "@BeforeAll method 'private java.lang.Double org.junit.jupiter.engine.descriptor.TestCaseWithInvalidLifecycleMethods.cc()' must not return a value.") //
+ .source(methodSource) //
.build();
var notStaticIssue = DiscoveryIssue.builder(Severity.ERROR,
- "@BeforeAll method 'java.lang.Double org.junit.jupiter.engine.descriptor.TestCaseWithNonVoidLifecyleMethods.cc()' must be static unless the test class is annotated with @TestInstance(Lifecycle.PER_CLASS).") //
- .source(MethodSource.from(TestCaseWithNonVoidLifecyleMethods.class.getDeclaredMethod("cc"))) //
+ "@BeforeAll method 'private java.lang.Double org.junit.jupiter.engine.descriptor.TestCaseWithInvalidLifecycleMethods.cc()' must be static unless the test class is annotated with @TestInstance(Lifecycle.PER_CLASS).") //
+ .source(methodSource) //
.build();
- assertThat(discoveryIssues).containsExactlyInAnyOrder(notVoidIssue, notStaticIssue);
+ var privateIssue = DiscoveryIssue.builder(Severity.DEPRECATION,
+ "@BeforeAll method 'private java.lang.Double org.junit.jupiter.engine.descriptor.TestCaseWithInvalidLifecycleMethods.cc()' should not be private. This will be disallowed in a future release.") //
+ .source(methodSource) //
+ .build();
+ assertThat(discoveryIssues).containsExactlyInAnyOrder(notVoidIssue, notStaticIssue, privateIssue);
}
@Test
void findNonVoidAfterAllMethodsWithStandardLifecycle() throws Exception {
- var methods = findAfterAllMethods(TestCaseWithNonVoidLifecyleMethods.class, true, discoveryIssues::add);
+ var methods = findAfterAllMethods(TestCaseWithInvalidLifecycleMethods.class, true, discoveryIssues::add);
assertThat(methods).isEmpty();
+ var methodSource = MethodSource.from(TestCaseWithInvalidLifecycleMethods.class.getDeclaredMethod("dd"));
var notVoidIssue = DiscoveryIssue.builder(Severity.ERROR,
- "@AfterAll method 'java.lang.String org.junit.jupiter.engine.descriptor.TestCaseWithNonVoidLifecyleMethods.dd()' must not return a value.") //
- .source(MethodSource.from(TestCaseWithNonVoidLifecyleMethods.class.getDeclaredMethod("dd"))) //
+ "@AfterAll method 'private java.lang.String org.junit.jupiter.engine.descriptor.TestCaseWithInvalidLifecycleMethods.dd()' must not return a value.") //
+ .source(methodSource) //
.build();
var notStaticIssue = DiscoveryIssue.builder(Severity.ERROR,
- "@AfterAll method 'java.lang.String org.junit.jupiter.engine.descriptor.TestCaseWithNonVoidLifecyleMethods.dd()' must be static unless the test class is annotated with @TestInstance(Lifecycle.PER_CLASS).") //
- .source(MethodSource.from(TestCaseWithNonVoidLifecyleMethods.class.getDeclaredMethod("dd"))) //
+ "@AfterAll method 'private java.lang.String org.junit.jupiter.engine.descriptor.TestCaseWithInvalidLifecycleMethods.dd()' must be static unless the test class is annotated with @TestInstance(Lifecycle.PER_CLASS).") //
+ .source(methodSource) //
+ .build();
+ var privateIssue = DiscoveryIssue.builder(Severity.DEPRECATION,
+ "@AfterAll method 'private java.lang.String org.junit.jupiter.engine.descriptor.TestCaseWithInvalidLifecycleMethods.dd()' should not be private. This will be disallowed in a future release.") //
+ .source(methodSource) //
.build();
- assertThat(discoveryIssues).containsExactlyInAnyOrder(notVoidIssue, notStaticIssue);
+ assertThat(discoveryIssues).containsExactlyInAnyOrder(notVoidIssue, notStaticIssue, privateIssue);
}
@Test
void findNonVoidBeforeEachMethodsWithStandardLifecycle() throws Exception {
- var methods = findBeforeEachMethods(TestCaseWithNonVoidLifecyleMethods.class, discoveryIssues::add);
+ var methods = findBeforeEachMethods(TestCaseWithInvalidLifecycleMethods.class, discoveryIssues::add);
assertThat(methods).isEmpty();
- var expectedIssue = DiscoveryIssue.builder(Severity.ERROR,
- "@BeforeEach method 'java.lang.String org.junit.jupiter.engine.descriptor.TestCaseWithNonVoidLifecyleMethods.aa()' must not return a value.") //
- .source(MethodSource.from(TestCaseWithNonVoidLifecyleMethods.class.getDeclaredMethod("aa"))) //
+ var methodSource = MethodSource.from(TestCaseWithInvalidLifecycleMethods.class.getDeclaredMethod("aa"));
+ var notVoidIssue = DiscoveryIssue.builder(Severity.ERROR,
+ "@BeforeEach method 'private java.lang.String org.junit.jupiter.engine.descriptor.TestCaseWithInvalidLifecycleMethods.aa()' must not return a value.") //
+ .source(methodSource) //
.build();
- assertThat(discoveryIssues).containsExactly(expectedIssue);
+ var privateIssue = DiscoveryIssue.builder(Severity.DEPRECATION,
+ "@BeforeEach method 'private java.lang.String org.junit.jupiter.engine.descriptor.TestCaseWithInvalidLifecycleMethods.aa()' should not be private. This will be disallowed in a future release.") //
+ .source(methodSource) //
+ .build();
+ assertThat(discoveryIssues).containsExactlyInAnyOrder(notVoidIssue, privateIssue);
}
@Test
void findNonVoidAfterEachMethodsWithStandardLifecycle() throws Exception {
- var methods = findAfterEachMethods(TestCaseWithNonVoidLifecyleMethods.class, discoveryIssues::add);
+ var methods = findAfterEachMethods(TestCaseWithInvalidLifecycleMethods.class, discoveryIssues::add);
assertThat(methods).isEmpty();
- var expectedIssue = DiscoveryIssue.builder(Severity.ERROR,
- "@AfterEach method 'int org.junit.jupiter.engine.descriptor.TestCaseWithNonVoidLifecyleMethods.bb()' must not return a value.") //
- .source(MethodSource.from(TestCaseWithNonVoidLifecyleMethods.class.getDeclaredMethod("bb"))) //
+ var methodSource = MethodSource.from(TestCaseWithInvalidLifecycleMethods.class.getDeclaredMethod("bb"));
+ var notVoidIssue = DiscoveryIssue.builder(Severity.ERROR,
+ "@AfterEach method 'private int org.junit.jupiter.engine.descriptor.TestCaseWithInvalidLifecycleMethods.bb()' must not return a value.") //
+ .source(methodSource) //
.build();
- assertThat(discoveryIssues).containsExactly(expectedIssue);
+ var privateIssue = DiscoveryIssue.builder(Severity.DEPRECATION,
+ "@AfterEach method 'private int org.junit.jupiter.engine.descriptor.TestCaseWithInvalidLifecycleMethods.bb()' should not be private. This will be disallowed in a future release.") //
+ .source(methodSource) //
+ .build();
+ assertThat(discoveryIssues).containsExactlyInAnyOrder(notVoidIssue, privateIssue);
}
@Test
@@ -228,29 +248,26 @@ void eight() {
}
-class TestCaseWithNonVoidLifecyleMethods {
+@SuppressWarnings("JUnitMalformedDeclaration")
+class TestCaseWithInvalidLifecycleMethods {
- @SuppressWarnings("JUnitMalformedDeclaration")
@BeforeEach
- String aa() {
+ private String aa() {
return null;
}
- @SuppressWarnings("JUnitMalformedDeclaration")
@AfterEach
- int bb() {
+ private int bb() {
return 1;
}
- @SuppressWarnings("JUnitMalformedDeclaration")
@BeforeAll
- Double cc() {
+ private Double cc() {
return null;
}
- @SuppressWarnings("JUnitMalformedDeclaration")
@AfterAll
- String dd() {
+ private String dd() {
return "";
}
From c68dd38b3499ac3fe60fe33691bdc56d2c4b72c3 Mon Sep 17 00:00:00 2001
From: Marc Philipp <mail@marcphilipp.de>
Date: Tue, 18 Mar 2025 15:27:21 +0100
Subject: [PATCH 4/4] Extract preliminary reusable `Condition` API from
`LifecycleMethodUtils`
---
.../descriptor/LifecycleMethodUtils.java | 83 +++-------------
.../DefaultDiscoveryIssueReporter.java | 35 -------
.../discovery/DiscoveryIssueReporter.java | 94 +++++++++++++++++++
.../EngineDiscoveryRequestResolver.java | 2 +-
4 files changed, 107 insertions(+), 107 deletions(-)
delete mode 100644 junit-platform-engine/src/main/java/org/junit/platform/engine/support/discovery/DefaultDiscoveryIssueReporter.java
diff --git a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/LifecycleMethodUtils.java b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/LifecycleMethodUtils.java
index 99693164d075..a73d9fc2f491 100644
--- a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/LifecycleMethodUtils.java
+++ b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/LifecycleMethodUtils.java
@@ -12,13 +12,11 @@
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;
@@ -26,12 +24,12 @@
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,7 +83,7 @@ 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)) //
@@ -93,9 +91,9 @@ private static List<Method> findMethodsAndCheckVoidReturnType(Class<?> testClass
.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());
@@ -103,18 +101,18 @@ private static Predicate<Method> isStatic(Class<? extends Annotation> annotation
});
}
- 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());
@@ -122,9 +120,9 @@ private static Consumer<Method> isNotPrivate(Class<? extends Annotation> annotat
});
}
- 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);
- }
-
}
diff --git a/junit-platform-engine/src/main/java/org/junit/platform/engine/support/discovery/DefaultDiscoveryIssueReporter.java b/junit-platform-engine/src/main/java/org/junit/platform/engine/support/discovery/DefaultDiscoveryIssueReporter.java
deleted file mode 100644
index 1aa686b29a4f..000000000000
--- a/junit-platform-engine/src/main/java/org/junit/platform/engine/support/discovery/DefaultDiscoveryIssueReporter.java
+++ /dev/null
@@ -1,35 +0,0 @@
-/*
- * Copyright 2015-2025 the original author or authors.
- *
- * All rights reserved. This program and the accompanying materials are
- * made available under the terms of the Eclipse Public License v2.0 which
- * accompanies this distribution and is available at
- *
- * https://www.eclipse.org/legal/epl-v20.html
- */
-
-package org.junit.platform.engine.support.discovery;
-
-import org.junit.platform.engine.DiscoveryIssue;
-import org.junit.platform.engine.EngineDiscoveryListener;
-import org.junit.platform.engine.UniqueId;
-
-/**
- * @since 1.13
- */
-class DefaultDiscoveryIssueReporter implements DiscoveryIssueReporter {
-
- private final EngineDiscoveryListener discoveryListener;
- private final UniqueId engineId;
-
- DefaultDiscoveryIssueReporter(EngineDiscoveryListener discoveryListener, UniqueId engineId) {
- this.discoveryListener = discoveryListener;
- this.engineId = engineId;
- }
-
- @Override
- public void reportIssue(DiscoveryIssue issue) {
- this.discoveryListener.issueEncountered(this.engineId, issue);
- }
-
-}
diff --git a/junit-platform-engine/src/main/java/org/junit/platform/engine/support/discovery/DiscoveryIssueReporter.java b/junit-platform-engine/src/main/java/org/junit/platform/engine/support/discovery/DiscoveryIssueReporter.java
index a087b0502e35..1f17d4128b5c 100644
--- a/junit-platform-engine/src/main/java/org/junit/platform/engine/support/discovery/DiscoveryIssueReporter.java
+++ b/junit-platform-engine/src/main/java/org/junit/platform/engine/support/discovery/DiscoveryIssueReporter.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);
+ }
+ }
}
diff --git a/junit-platform-engine/src/main/java/org/junit/platform/engine/support/discovery/EngineDiscoveryRequestResolver.java b/junit-platform-engine/src/main/java/org/junit/platform/engine/support/discovery/EngineDiscoveryRequestResolver.java
index 431975615370..28e24d09a705 100644
--- a/junit-platform-engine/src/main/java/org/junit/platform/engine/support/discovery/EngineDiscoveryRequestResolver.java
+++ b/junit-platform-engine/src/main/java/org/junit/platform/engine/support/discovery/EngineDiscoveryRequestResolver.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);