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 5103639abc17..c26a0e8f0bee 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 @@ -13,7 +13,6 @@ import static org.junit.platform.commons.support.AnnotationSupport.findAnnotatedMethods; import static org.junit.platform.commons.support.AnnotationSupport.findAnnotation; 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; @@ -75,28 +74,27 @@ static void validateNoClassTemplateInvocationLifecycleMethodsAreDeclared(Class findClassTemplateInvocationLifecycleMethodAnnotation(method) // - .ifPresent(annotation -> { + .ifPresent(annotation -> issueReporter.reportIssue(() -> { String message = String.format( "@%s method '%s' must not be declared in test class '%s' because it is not annotated with @%s.", annotation.lifecycleMethodAnnotation().getSimpleName(), method.toGenericString(), testClass.getName(), annotation.classTemplateAnnotation().getSimpleName()); - issueReporter.reportIssue(createIssue(Severity.ERROR, message, method)); - })); + return createIssue(Severity.ERROR, message, method); + }))); } static void validateClassTemplateInvocationLifecycleMethodsAreDeclaredCorrectly(Class testClass, boolean requireStatic, DiscoveryIssueReporter issueReporter) { findAllClassTemplateInvocationLifecycleMethods(testClass) // - .forEach(allOf( // - isNotPrivateError(issueReporter), // - returnsPrimitiveVoid(issueReporter, - LifecycleMethodUtils::classTemplateInvocationLifecycleMethodAnnotationName), // - requireStatic - ? isStatic(issueReporter, - LifecycleMethodUtils::classTemplateInvocationLifecycleMethodAnnotationName) - : __ -> true // - )); + .forEach(isNotPrivateError(issueReporter) // + .and(returnsPrimitiveVoid(issueReporter, + LifecycleMethodUtils::classTemplateInvocationLifecycleMethodAnnotationName)) // + .and(requireStatic + ? isStatic(issueReporter, + LifecycleMethodUtils::classTemplateInvocationLifecycleMethodAnnotationName) + : __ -> true) // + ); } private static Stream findAllClassTemplateInvocationLifecycleMethods(Class testClass) { @@ -134,8 +132,8 @@ private static List findMethodsAndCheckVoidReturnType(Class testClass return findAnnotatedMethods(testClass, annotationType, traversalMode).stream() // .peek(isNotPrivateDeprecation(issueReporter, annotationType::getSimpleName)) // - .filter(allOf(returnsPrimitiveVoid(issueReporter, __ -> annotationType.getSimpleName()), - additionalCondition)) // + .filter( + returnsPrimitiveVoid(issueReporter, __ -> annotationType.getSimpleName()).and(additionalCondition)) // .collect(toUnmodifiableList()); } diff --git a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/ClassSelectorResolver.java b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/ClassSelectorResolver.java index 3405dad0e7d7..ba1d9dc264fb 100644 --- a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/ClassSelectorResolver.java +++ b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/ClassSelectorResolver.java @@ -15,7 +15,6 @@ import static java.util.stream.Collectors.toCollection; import static java.util.stream.Collectors.toSet; import static org.junit.jupiter.engine.descriptor.NestedClassTestDescriptor.getEnclosingTestClasses; -import static org.junit.jupiter.engine.discovery.predicates.IsTestClassWithTests.isTestOrTestFactoryOrTestTemplateMethod; import static org.junit.platform.commons.support.AnnotationSupport.isAnnotated; import static org.junit.platform.commons.support.HierarchyTraversalMode.TOP_DOWN; import static org.junit.platform.commons.support.ReflectionSupport.findMethods; @@ -45,7 +44,6 @@ import org.junit.jupiter.engine.descriptor.Filterable; import org.junit.jupiter.engine.descriptor.NestedClassTestDescriptor; import org.junit.jupiter.engine.descriptor.TestClassAware; -import org.junit.jupiter.engine.discovery.predicates.IsNestedTestClass; import org.junit.jupiter.engine.discovery.predicates.IsTestClassWithTests; import org.junit.platform.commons.support.ReflectionSupport; import org.junit.platform.engine.DiscoverySelector; @@ -56,6 +54,7 @@ import org.junit.platform.engine.discovery.IterationSelector; import org.junit.platform.engine.discovery.NestedClassSelector; import org.junit.platform.engine.discovery.UniqueIdSelector; +import org.junit.platform.engine.support.discovery.DiscoveryIssueReporter; import org.junit.platform.engine.support.discovery.SelectorResolver; /** @@ -63,30 +62,32 @@ */ class ClassSelectorResolver implements SelectorResolver { - private static final IsTestClassWithTests isTestClassWithTests = new IsTestClassWithTests(); - private static final IsNestedTestClass isNestedTestClass = new IsNestedTestClass(); private static final Predicate> isAnnotatedWithClassTemplate = testClass -> isAnnotated(testClass, ClassTemplate.class); + private final IsTestClassWithTests isTestClassWithTests; + private final Predicate classNameFilter; private final JupiterConfiguration configuration; - ClassSelectorResolver(Predicate classNameFilter, JupiterConfiguration configuration) { + ClassSelectorResolver(Predicate classNameFilter, JupiterConfiguration configuration, + DiscoveryIssueReporter issueReporter) { this.classNameFilter = classNameFilter; this.configuration = configuration; + this.isTestClassWithTests = new IsTestClassWithTests(issueReporter); } @Override public Resolution resolve(ClassSelector selector, Context context) { Class testClass = selector.getJavaClass(); - if (isTestClassWithTests.test(testClass)) { + if (this.isTestClassWithTests.test(testClass)) { // Nested tests are never filtered out if (classNameFilter.test(testClass.getName())) { return toResolution( context.addToParent(parent -> Optional.of(newStaticClassTestDescriptor(parent, testClass)))); } } - else if (isNestedTestClass.test(testClass)) { + else if (this.isTestClassWithTests.isNestedTestClass.test(testClass)) { return toResolution(context.addToParent(() -> DiscoverySelectors.selectClass(testClass.getEnclosingClass()), parent -> Optional.of(newMemberClassTestDescriptor(parent, testClass)))); } @@ -95,7 +96,7 @@ else if (isNestedTestClass.test(testClass)) { @Override public Resolution resolve(NestedClassSelector selector, Context context) { - if (isNestedTestClass.test(selector.getNestedClass())) { + if (this.isTestClassWithTests.isNestedTestClass.test(selector.getNestedClass())) { return toResolution(context.addToParent(() -> selectClass(selector.getEnclosingClasses()), parent -> Optional.of(newMemberClassTestDescriptor(parent, selector.getNestedClass())))); } @@ -165,7 +166,7 @@ private Resolution resolveStaticClassUniqueId(Context context, UniqueId.Segment String className = lastSegment.getValue(); return ReflectionSupport.tryToLoadClass(className).toOptional() // - .filter(isTestClassWithTests) // + .filter(this.isTestClassWithTests) // .filter(condition) // .map(testClass -> toResolution( context.addToParent(parent -> Optional.of(factory.apply(parent, testClass))))) // @@ -179,8 +180,8 @@ private Resolution resolveNestedClassUniqueId(Context context, UniqueId uniqueId String simpleClassName = uniqueId.getLastSegment().getValue(); return toResolution(context.addToParent(() -> selectUniqueId(uniqueId.removeLastSegment()), parent -> { Class parentTestClass = ((TestClassAware) parent).getTestClass(); - return ReflectionSupport.findNestedClasses(parentTestClass, - isNestedTestClass.and(where(Class::getSimpleName, isEqual(simpleClassName)))).stream() // + return ReflectionSupport.findNestedClasses(parentTestClass, this.isTestClassWithTests.isNestedTestClass.and( + where(Class::getSimpleName, isEqual(simpleClassName)))).stream() // .findFirst() // .filter(condition) // .map(testClass -> factory.apply(parent, testClass)); @@ -271,10 +272,12 @@ private Supplier> expansionCallback(TestDescrip } List> testClasses = testClassesSupplier.get(); Class testClass = testClasses.get(testClasses.size() - 1); - Stream methods = findMethods(testClass, isTestOrTestFactoryOrTestTemplateMethod, - TOP_DOWN).stream().map(method -> selectMethod(testClasses, method)); - Stream nestedClasses = streamNestedClasses(testClass, isNestedTestClass).map( - nestedClass -> DiscoverySelectors.selectNestedClass(testClasses, nestedClass)); + Stream methods = findMethods(testClass, + this.isTestClassWithTests.isTestOrTestFactoryOrTestTemplateMethod, TOP_DOWN).stream().map( + method -> selectMethod(testClasses, method)); + Stream nestedClasses = streamNestedClasses(testClass, + this.isTestClassWithTests.isNestedTestClass).map( + nestedClass -> DiscoverySelectors.selectNestedClass(testClasses, nestedClass)); return Stream.concat(methods, nestedClasses).collect( toCollection((Supplier>) LinkedHashSet::new)); }; 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 974684d680cb..b8ade50c6766 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 @@ -19,6 +19,7 @@ import org.junit.jupiter.engine.discovery.predicates.IsTestClassWithTests; import org.junit.platform.engine.EngineDiscoveryRequest; import org.junit.platform.engine.TestDescriptor; +import org.junit.platform.engine.support.discovery.DiscoveryIssueReporter; import org.junit.platform.engine.support.discovery.EngineDiscoveryRequestResolver; import org.junit.platform.engine.support.discovery.EngineDiscoveryRequestResolver.InitializationContext; @@ -37,8 +38,9 @@ public class DiscoverySelectorResolver { private static final EngineDiscoveryRequestResolver resolver = EngineDiscoveryRequestResolver. builder() // - .addClassContainerSelectorResolver(new IsTestClassWithTests()) // - .addSelectorResolver(ctx -> new ClassSelectorResolver(ctx.getClassNameFilter(), getConfiguration(ctx))) // + .addClassContainerSelectorResolverWithContext(ctx -> new IsTestClassWithTests(ctx.getIssueReporter())) // + .addSelectorResolver(ctx -> new ClassSelectorResolver(ctx.getClassNameFilter(), getConfiguration(ctx), + ctx.getIssueReporter())) // .addSelectorResolver(ctx -> new MethodSelectorResolver(getConfiguration(ctx), ctx.getIssueReporter())) // .addTestDescriptorVisitor(ctx -> TestDescriptor.Visitor.composite( // new ClassOrderingVisitor(getConfiguration(ctx)), // @@ -55,7 +57,9 @@ private static JupiterConfiguration getConfiguration(InitializationContext> testClassPredicate = new IsTestClassWithTests().or( - new IsNestedTestClass()); + private final Predicate> testClassPredicate; private final JupiterConfiguration configuration; private final DiscoveryIssueReporter issueReporter; + private final List methodTypes; MethodSelectorResolver(JupiterConfiguration configuration, DiscoveryIssueReporter issueReporter) { this.configuration = configuration; this.issueReporter = issueReporter; + this.methodTypes = MethodType.allPossibilities(issueReporter); + IsTestClassWithTests classPredicate = new IsTestClassWithTests(issueReporter); + this.testClassPredicate = classPredicate.or(classPredicate.isNestedTestClass); } @Override @@ -92,7 +94,7 @@ private Resolution resolve(Context context, List> enclosingClasses, Cla } Method method = methodSupplier.get(); // @formatter:off - Set matches = Arrays.stream(MethodType.values()) + Set matches = methodTypes.stream() .map(methodType -> methodType.resolve(enclosingClasses, testClass, method, context, configuration)) .filter(Optional::isPresent) .map(Optional::get) @@ -100,14 +102,17 @@ private Resolution resolve(Context context, List> enclosingClasses, Cla .collect(toSet()); // @formatter:on if (matches.size() > 1) { - Stream testDescriptors = matches.stream().map(Match::getTestDescriptor); - String message = String.format( - "Possible configuration error: method [%s] resulted in multiple TestDescriptors %s. " - + "This is typically the result of annotating a method with multiple competing annotations " - + "such as @Test, @RepeatedTest, @ParameterizedTest, @TestFactory, etc.", - method.toGenericString(), testDescriptors.map(d -> d.getClass().getName()).collect(toList())); - issueReporter.reportIssue( - DiscoveryIssue.builder(Severity.WARNING, message).source(MethodSource.from(method))); + issueReporter.reportIssue(() -> { + Stream testDescriptors = matches.stream().map(Match::getTestDescriptor); + String message = String.format( + "Possible configuration error: method [%s] resulted in multiple TestDescriptors %s. " + + "This is typically the result of annotating a method with multiple competing annotations " + + "such as @Test, @RepeatedTest, @ParameterizedTest, @TestFactory, etc.", + method.toGenericString(), testDescriptors.map(d -> d.getClass().getName()).collect(toList())); + return DiscoveryIssue.builder(Severity.WARNING, message) // + .source(MethodSource.from(method)) // + .build(); + }); } return matches.isEmpty() ? unresolved() : matches(matches); } @@ -116,7 +121,7 @@ private Resolution resolve(Context context, List> enclosingClasses, Cla public Resolution resolve(UniqueIdSelector selector, Context context) { UniqueId uniqueId = selector.getUniqueId(); // @formatter:off - return Arrays.stream(MethodType.values()) + return methodTypes.stream() .map(methodType -> methodType.resolveUniqueIdIntoTestDescriptor(uniqueId, context, configuration)) .filter(Optional::isPresent) .map(Optional::get) @@ -164,48 +169,34 @@ private Supplier> expansionCallback(TestDescrip }; } - private enum MethodType { - - TEST(new IsTestMethod(), TestMethodTestDescriptor.SEGMENT_TYPE) { - @Override - protected TestDescriptor createTestDescriptor(UniqueId uniqueId, Class testClass, Method method, - Supplier>> enclosingInstanceTypes, JupiterConfiguration configuration) { - return new TestMethodTestDescriptor(uniqueId, testClass, method, enclosingInstanceTypes, configuration); - } - }, - - TEST_FACTORY(new IsTestFactoryMethod(), TestFactoryTestDescriptor.SEGMENT_TYPE, - TestFactoryTestDescriptor.DYNAMIC_CONTAINER_SEGMENT_TYPE, - TestFactoryTestDescriptor.DYNAMIC_TEST_SEGMENT_TYPE) { - @Override - protected TestDescriptor createTestDescriptor(UniqueId uniqueId, Class testClass, Method method, - Supplier>> enclosingInstanceTypes, JupiterConfiguration configuration) { - return new TestFactoryTestDescriptor(uniqueId, testClass, method, enclosingInstanceTypes, - configuration); - } - }, - - TEST_TEMPLATE(new IsTestTemplateMethod(), TestTemplateTestDescriptor.SEGMENT_TYPE, - TestTemplateInvocationTestDescriptor.SEGMENT_TYPE) { - @Override - protected TestDescriptor createTestDescriptor(UniqueId uniqueId, Class testClass, Method method, - Supplier>> enclosingInstanceTypes, JupiterConfiguration configuration) { - return new TestTemplateTestDescriptor(uniqueId, testClass, method, enclosingInstanceTypes, - configuration); - } - }; + private static class MethodType { + + static List allPossibilities(DiscoveryIssueReporter issueReporter) { + return Arrays.asList( // + new MethodType(new IsTestMethod(issueReporter), TestMethodTestDescriptor::new, + TestMethodTestDescriptor.SEGMENT_TYPE), // + new MethodType(new IsTestFactoryMethod(issueReporter), TestFactoryTestDescriptor::new, + TestFactoryTestDescriptor.SEGMENT_TYPE, TestFactoryTestDescriptor.DYNAMIC_CONTAINER_SEGMENT_TYPE, + TestFactoryTestDescriptor.DYNAMIC_TEST_SEGMENT_TYPE), // + new MethodType(new IsTestTemplateMethod(issueReporter), TestTemplateTestDescriptor::new, + TestTemplateTestDescriptor.SEGMENT_TYPE, TestTemplateInvocationTestDescriptor.SEGMENT_TYPE) // + ); + } private final Predicate methodPredicate; + private final TestDescriptorFactory testDescriptorFactory; private final String segmentType; private final Set dynamicDescendantSegmentTypes; - MethodType(Predicate methodPredicate, String segmentType, String... dynamicDescendantSegmentTypes) { + private MethodType(Predicate methodPredicate, TestDescriptorFactory testDescriptorFactory, + String segmentType, String... dynamicDescendantSegmentTypes) { this.methodPredicate = methodPredicate; + this.testDescriptorFactory = testDescriptorFactory; this.segmentType = segmentType; this.dynamicDescendantSegmentTypes = new LinkedHashSet<>(Arrays.asList(dynamicDescendantSegmentTypes)); } - private Optional resolve(List> enclosingClasses, Class testClass, Method method, + Optional resolve(List> enclosingClasses, Class testClass, Method method, Context context, JupiterConfiguration configuration) { if (!methodPredicate.test(method)) { return Optional.empty(); @@ -221,7 +212,7 @@ private DiscoverySelector selectClass(List> enclosingClasses, Class return DiscoverySelectors.selectNestedClass(enclosingClasses, testClass); } - private Optional resolveUniqueIdIntoTestDescriptor(UniqueId uniqueId, Context context, + Optional resolveUniqueIdIntoTestDescriptor(UniqueId uniqueId, Context context, JupiterConfiguration configuration) { UniqueId.Segment lastSegment = uniqueId.getLastSegment(); if (segmentType.equals(lastSegment.getType())) { @@ -244,8 +235,8 @@ private Optional resolveUniqueIdIntoTestDescriptor(UniqueId uniq private TestDescriptor createTestDescriptor(TestDescriptor parent, Class testClass, Method method, JupiterConfiguration configuration) { UniqueId uniqueId = createUniqueId(method, parent); - return createTestDescriptor(uniqueId, testClass, method, ((TestClassAware) parent)::getEnclosingTestClasses, - configuration); + return testDescriptorFactory.create(uniqueId, testClass, method, + ((TestClassAware) parent)::getEnclosingTestClasses, configuration); } private UniqueId createUniqueId(Method method, TestDescriptor parent) { @@ -254,8 +245,10 @@ private UniqueId createUniqueId(Method method, TestDescriptor parent) { return parent.getUniqueId().append(segmentType, methodId); } - protected abstract TestDescriptor createTestDescriptor(UniqueId uniqueId, Class testClass, Method method, - Supplier>> enclosingInstanceTypes, JupiterConfiguration configuration); + interface TestDescriptorFactory { + TestDescriptor create(UniqueId uniqueId, Class testClass, Method method, + Supplier>> enclosingInstanceTypes, JupiterConfiguration configuration); + } } diff --git a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/predicates/IsNestedTestClass.java b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/predicates/IsNestedTestClass.java index 3474d8cf0240..f1fc79a0226b 100644 --- a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/predicates/IsNestedTestClass.java +++ b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/predicates/IsNestedTestClass.java @@ -28,6 +28,9 @@ public class IsNestedTestClass implements Predicate> { private static final IsInnerClass isInnerClass = new IsInnerClass(); + IsNestedTestClass() { + } + @Override public boolean test(Class candidate) { //please do not collapse into single return diff --git a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/predicates/IsTestClassWithTests.java b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/predicates/IsTestClassWithTests.java index 764ef8356067..2b8f85b49e24 100644 --- a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/predicates/IsTestClassWithTests.java +++ b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/predicates/IsTestClassWithTests.java @@ -18,6 +18,7 @@ import org.apiguardian.api.API; import org.junit.platform.commons.support.ReflectionSupport; import org.junit.platform.commons.util.ReflectionUtils; +import org.junit.platform.engine.support.discovery.DiscoveryIssueReporter; /** * Test if a class is a JUnit Jupiter test class containing executable tests, @@ -28,18 +29,18 @@ @API(status = INTERNAL, since = "5.1") public class IsTestClassWithTests implements Predicate> { - private static final IsTestMethod isTestMethod = new IsTestMethod(); - - private static final IsTestFactoryMethod isTestFactoryMethod = new IsTestFactoryMethod(); - - private static final IsTestTemplateMethod isTestTemplateMethod = new IsTestTemplateMethod(); - - public static final Predicate isTestOrTestFactoryOrTestTemplateMethod = isTestMethod.or( - isTestFactoryMethod).or(isTestTemplateMethod); + public final Predicate isTestOrTestFactoryOrTestTemplateMethod; private static final IsPotentialTestContainer isPotentialTestContainer = new IsPotentialTestContainer(); - private static final IsNestedTestClass isNestedTestClass = new IsNestedTestClass(); + public final IsNestedTestClass isNestedTestClass = new IsNestedTestClass(); + + @API(status = INTERNAL, since = "5.13") + public IsTestClassWithTests(DiscoveryIssueReporter issueReporter) { + this.isTestOrTestFactoryOrTestTemplateMethod = new IsTestMethod(issueReporter) // + .or(new IsTestFactoryMethod(issueReporter)) // + .or(new IsTestTemplateMethod(issueReporter)); + } @Override public boolean test(Class candidate) { diff --git a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/predicates/IsTestFactoryMethod.java b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/predicates/IsTestFactoryMethod.java index 2932640add1a..939e6721fa0e 100644 --- a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/predicates/IsTestFactoryMethod.java +++ b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/predicates/IsTestFactoryMethod.java @@ -14,6 +14,7 @@ import org.apiguardian.api.API; import org.junit.jupiter.api.TestFactory; +import org.junit.platform.engine.support.discovery.DiscoveryIssueReporter; /** * Test if a method is a JUnit Jupiter {@link TestFactory @TestFactory} method. @@ -26,8 +27,8 @@ @API(status = INTERNAL, since = "5.0") public class IsTestFactoryMethod extends IsTestableMethod { - public IsTestFactoryMethod() { - super(TestFactory.class, false); + public IsTestFactoryMethod(DiscoveryIssueReporter issueReporter) { + super(TestFactory.class, false, issueReporter); } } diff --git a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/predicates/IsTestMethod.java b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/predicates/IsTestMethod.java index 8cbc1b7b6cca..5c5c8d50e40e 100644 --- a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/predicates/IsTestMethod.java +++ b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/predicates/IsTestMethod.java @@ -14,6 +14,7 @@ import org.apiguardian.api.API; import org.junit.jupiter.api.Test; +import org.junit.platform.engine.support.discovery.DiscoveryIssueReporter; /** * Test if a method is a JUnit Jupiter {@link Test @Test} method. @@ -23,8 +24,8 @@ @API(status = INTERNAL, since = "5.0") public class IsTestMethod extends IsTestableMethod { - public IsTestMethod() { - super(Test.class, true); + public IsTestMethod(DiscoveryIssueReporter issueReporter) { + super(Test.class, true, issueReporter); } } diff --git a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/predicates/IsTestTemplateMethod.java b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/predicates/IsTestTemplateMethod.java index fded3a83eab9..4c9715f254f6 100644 --- a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/predicates/IsTestTemplateMethod.java +++ b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/predicates/IsTestTemplateMethod.java @@ -14,6 +14,7 @@ import org.apiguardian.api.API; import org.junit.jupiter.api.TestTemplate; +import org.junit.platform.engine.support.discovery.DiscoveryIssueReporter; /** * Test if a method is a JUnit Jupiter {@link TestTemplate @TestTemplate} method. @@ -23,8 +24,8 @@ @API(status = INTERNAL, since = "5.0") public class IsTestTemplateMethod extends IsTestableMethod { - public IsTestTemplateMethod() { - super(TestTemplate.class, true); + public IsTestTemplateMethod(DiscoveryIssueReporter issueReporter) { + super(TestTemplate.class, true, issueReporter); } } diff --git a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/predicates/IsTestableMethod.java b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/predicates/IsTestableMethod.java index 7852d382c627..b689c9b7b0d3 100644 --- a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/predicates/IsTestableMethod.java +++ b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/predicates/IsTestableMethod.java @@ -11,45 +11,79 @@ package org.junit.jupiter.engine.discovery.predicates; import static org.junit.platform.commons.support.AnnotationSupport.isAnnotated; -import static org.junit.platform.commons.support.ModifierSupport.isAbstract; -import static org.junit.platform.commons.support.ModifierSupport.isPrivate; -import static org.junit.platform.commons.support.ModifierSupport.isStatic; import static org.junit.platform.commons.util.ReflectionUtils.returnsPrimitiveVoid; import java.lang.annotation.Annotation; import java.lang.reflect.Method; import java.util.function.Predicate; +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; +import org.junit.platform.engine.support.discovery.DiscoveryIssueReporter.Condition; + /** * @since 5.0 */ abstract class IsTestableMethod implements Predicate { private final Class annotationType; - private final boolean mustReturnPrimitiveVoid; + private final Condition condition; - IsTestableMethod(Class annotationType, boolean mustReturnPrimitiveVoid) { + IsTestableMethod(Class annotationType, boolean mustReturnPrimitiveVoid, + DiscoveryIssueReporter issueReporter) { this.annotationType = annotationType; - this.mustReturnPrimitiveVoid = mustReturnPrimitiveVoid; + this.condition = isNotStatic(issueReporter) // + .and(isNotPrivate(issueReporter)) // + .and(isNotAbstract(issueReporter)) // + .and(hasCompatibleReturnType(mustReturnPrimitiveVoid, issueReporter)); } @Override public boolean test(Method candidate) { - // Please do not collapse the following into a single statement. - if (isStatic(candidate)) { - return false; - } - if (isPrivate(candidate)) { - return false; + if (isAnnotated(candidate, this.annotationType)) { + return condition.test(candidate); } - if (isAbstract(candidate)) { - return false; + return false; + } + + private Condition isNotStatic(DiscoveryIssueReporter issueReporter) { + return issueReporter.createReportingCondition(ModifierSupport::isNotStatic, + method -> createIssue(method, "must not be static")); + } + + private Condition isNotPrivate(DiscoveryIssueReporter issueReporter) { + return issueReporter.createReportingCondition(ModifierSupport::isNotPrivate, + method -> createIssue(method, "must not be private")); + } + + private Condition isNotAbstract(DiscoveryIssueReporter issueReporter) { + return issueReporter.createReportingCondition(ModifierSupport::isNotAbstract, + method -> createIssue(method, "must not be abstract")); + } + + private Condition hasCompatibleReturnType(boolean mustReturnPrimitiveVoid, + DiscoveryIssueReporter issueReporter) { + if (mustReturnPrimitiveVoid) { + return issueReporter.createReportingCondition(ReflectionUtils::returnsPrimitiveVoid, + method -> createIssue(method, "must not return a value")); } - if (returnsPrimitiveVoid(candidate) != this.mustReturnPrimitiveVoid) { - return false; + else { + // TODO [#4246] Use `Predicate.not` + return issueReporter.createReportingCondition(method -> !returnsPrimitiveVoid(method), + method -> createIssue(method, "must return a value")); } + } - return isAnnotated(candidate, this.annotationType); + private DiscoveryIssue createIssue(Method method, String condition) { + String message = String.format("@%s method '%s' %s. It will be not be executed.", + this.annotationType.getSimpleName(), method.toGenericString(), condition); + return DiscoveryIssue.builder(Severity.WARNING, message) // + .source(MethodSource.from(method)) // + .build(); } } diff --git a/junit-platform-commons/src/main/java/org/junit/platform/commons/support/ModifierSupport.java b/junit-platform-commons/src/main/java/org/junit/platform/commons/support/ModifierSupport.java index 21302c9f24b0..e92289ecc05e 100644 --- a/junit-platform-commons/src/main/java/org/junit/platform/commons/support/ModifierSupport.java +++ b/junit-platform-commons/src/main/java/org/junit/platform/commons/support/ModifierSupport.java @@ -10,6 +10,7 @@ package org.junit.platform.commons.support; +import static org.apiguardian.api.API.Status.EXPERIMENTAL; import static org.apiguardian.api.API.Status.MAINTAINED; import java.lang.reflect.Member; @@ -142,6 +143,32 @@ public static boolean isAbstract(Member member) { return ReflectionUtils.isAbstract(member); } + /** + * Determine if the supplied class is not {@code abstract}. + * + * @param clazz the class to check; never {@code null} + * @return {@code true} if the class is not {@code abstract} + * @since 1.13 + * @see java.lang.reflect.Modifier#isAbstract(int) + */ + @API(status = EXPERIMENTAL, since = "1.13") + public static boolean isNotAbstract(Class clazz) { + return ReflectionUtils.isNotAbstract(clazz); + } + + /** + * Determine if the supplied member is not {@code abstract}. + * + * @param member the class to check; never {@code null} + * @return {@code true} if the member is not {@code abstract} + * @since 1.13 + * @see java.lang.reflect.Modifier#isAbstract(int) + */ + @API(status = EXPERIMENTAL, since = "1.13") + public static boolean isNotAbstract(Member member) { + return ReflectionUtils.isNotAbstract(member); + } + /** * Determine if the supplied class is {@code static}. * diff --git a/junit-platform-commons/src/main/java/org/junit/platform/commons/util/ReflectionUtils.java b/junit-platform-commons/src/main/java/org/junit/platform/commons/util/ReflectionUtils.java index 01f0e89d0fd2..c774b0a39c59 100644 --- a/junit-platform-commons/src/main/java/org/junit/platform/commons/util/ReflectionUtils.java +++ b/junit-platform-commons/src/main/java/org/junit/platform/commons/util/ReflectionUtils.java @@ -289,11 +289,21 @@ public static boolean isAbstract(Class clazz) { return Modifier.isAbstract(clazz.getModifiers()); } + @API(status = INTERNAL, since = "1.13") + public static boolean isNotAbstract(Class clazz) { + return !isAbstract(clazz); + } + public static boolean isAbstract(Member member) { Preconditions.notNull(member, "Member must not be null"); return Modifier.isAbstract(member.getModifiers()); } + @API(status = INTERNAL, since = "1.13") + public static boolean isNotAbstract(Member member) { + return !isAbstract(member); + } + public static boolean isStatic(Class clazz) { Preconditions.notNull(clazz, "Class must not be null"); return Modifier.isStatic(clazz.getModifiers()); diff --git a/junit-platform-engine/src/main/java/org/junit/platform/engine/support/discovery/ClassContainerSelectorResolver.java b/junit-platform-engine/src/main/java/org/junit/platform/engine/support/discovery/ClassContainerSelectorResolver.java index 6c383e418847..4dfae26276f5 100644 --- a/junit-platform-engine/src/main/java/org/junit/platform/engine/support/discovery/ClassContainerSelectorResolver.java +++ b/junit-platform-engine/src/main/java/org/junit/platform/engine/support/discovery/ClassContainerSelectorResolver.java @@ -20,6 +20,7 @@ import java.util.List; import java.util.function.Predicate; +import org.junit.platform.commons.util.Preconditions; import org.junit.platform.engine.discovery.ClasspathRootSelector; import org.junit.platform.engine.discovery.DiscoverySelectors; import org.junit.platform.engine.discovery.ModuleSelector; @@ -34,8 +35,8 @@ class ClassContainerSelectorResolver implements SelectorResolver { private final Predicate classNameFilter; ClassContainerSelectorResolver(Predicate> classFilter, Predicate classNameFilter) { - this.classFilter = classFilter; - this.classNameFilter = classNameFilter; + this.classFilter = Preconditions.notNull(classFilter, "classFilter must not be null"); + this.classNameFilter = Preconditions.notNull(classNameFilter, "classNameFilter must not be null"); } @Override diff --git a/junit-platform-engine/src/main/java/org/junit/platform/engine/support/discovery/DiscardingDiscoveryIssueReporter.java b/junit-platform-engine/src/main/java/org/junit/platform/engine/support/discovery/DiscardingDiscoveryIssueReporter.java new file mode 100644 index 000000000000..b3dcb44d4d4b --- /dev/null +++ b/junit-platform-engine/src/main/java/org/junit/platform/engine/support/discovery/DiscardingDiscoveryIssueReporter.java @@ -0,0 +1,52 @@ +/* + * 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 java.util.function.Function; +import java.util.function.Predicate; +import java.util.function.Supplier; + +import org.junit.platform.commons.util.Preconditions; +import org.junit.platform.engine.DiscoveryIssue; + +/** + * A {@code DiscoveryIssueReporter} that discards all reported issues. + * + * @since 1.13 + */ +class DiscardingDiscoveryIssueReporter implements DiscoveryIssueReporter { + + static final DiscardingDiscoveryIssueReporter INSTANCE = new DiscardingDiscoveryIssueReporter(); + + private DiscardingDiscoveryIssueReporter() { + } + + @Override + public void reportIssue(DiscoveryIssue.Builder builder) { + // discard + } + + @Override + public void reportIssue(Supplier issue) { + // discard + } + + @Override + public void reportIssue(DiscoveryIssue issue) { + // discard + } + + @Override + public Condition createReportingCondition(Predicate predicate, Function issueCreator) { + Preconditions.notNull(predicate, "predicate must not be null"); + return predicate::test; + } +} 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 1f17d4128b5c..b528398df27a 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,9 +12,12 @@ import static org.apiguardian.api.API.Status.EXPERIMENTAL; +import java.util.HashSet; +import java.util.Set; import java.util.function.Consumer; import java.util.function.Function; import java.util.function.Predicate; +import java.util.function.Supplier; import org.apiguardian.api.API; import org.junit.platform.commons.util.Preconditions; @@ -34,27 +37,64 @@ public interface DiscoveryIssueReporter { /** - * Create a new {@code DiscoveryIssueReporter} that reports issues to the + * Create a new {@code DiscoveryIssueReporter} that forwards issues to the * supplied {@link EngineDiscoveryListener} for the specified engine. * - * @param engineDiscoveryListener the listener to report issues to; never + * @param engineDiscoveryListener the listener to forward issues to; never * {@code null} * @param engineId the unique identifier of the engine; never {@code null} */ - static DiscoveryIssueReporter create(EngineDiscoveryListener engineDiscoveryListener, UniqueId engineId) { + static DiscoveryIssueReporter forwarding(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); } + /** + * Create a new {@code DiscoveryIssueReporter} that avoids reporting + * duplicate issues. + * + *

The implementation returned by this method is not thread-safe. + * + * @param delegate the delegate to forward issues to; never {@code null} + */ + static DiscoveryIssueReporter deduplicating(DiscoveryIssueReporter delegate) { + Preconditions.notNull(delegate, "delegate must not be null"); + Set seen = new HashSet<>(); + return issue -> { + boolean notSeen = seen.add(issue); + if (notSeen) { + delegate.reportIssue(issue); + } + }; + } + + /** + * Create a new {@code DiscoveryIssueReporter} that discards all reported + * issues. + */ + static DiscoveryIssueReporter discarding() { + return DiscardingDiscoveryIssueReporter.INSTANCE; + } + /** * Build the supplied {@link DiscoveryIssue.Builder Builder} and report the * resulting {@link DiscoveryIssue}. */ default void reportIssue(DiscoveryIssue.Builder builder) { + Preconditions.notNull(builder, "builder must not be null"); reportIssue(builder.build()); } + /** + * Build the supplied {@link DiscoveryIssue.Builder Builder} and report the + * resulting {@link DiscoveryIssue}. + */ + default void reportIssue(Supplier issueCreator) { + Preconditions.notNull(issueCreator, "issueCreator must not be null"); + reportIssue(issueCreator.get()); + } + /** * Report the supplied {@link DiscoveryIssue}. */ @@ -98,30 +138,35 @@ default Condition createReportingCondition(Predicate predicate, interface Condition extends Predicate, Consumer { /** - * Return a composed condition that represents a logical AND of the - * supplied conditions without short-circuiting. + * Return a composed condition that represents a logical AND of this + * and the supplied condition. + * + *

The default implementation avoids short-circuiting so + * both conditions will be evaluated even if this condition + * returns {@code false} to ensure that all issues are reported. + * + * @return the composed condition; never {@code null} + */ + @Override + default Condition and(Predicate other) { + Preconditions.notNull(other, "other condition must not be null"); + return (t) -> test(t) & other.test(t); + } + + /** + * Return a composed condition that represents a logical AND of this + * and the supplied condition. * - *

All of the supplied conditions will be evaluated even if - * one or more of them return {@code false} to ensure that all issues - * are reported. + *

The default implementation avoids short-circuiting so + * both conditions will be evaluated even if this condition + * returns {@code true} 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 Condition allOf(Condition... 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 condition : conditions) { - result &= condition.test(value); - } - return result; - }; + @Override + default Predicate or(Predicate other) { + Preconditions.notNull(other, "other condition must not be null"); + return (t) -> test(t) | other.test(t); } /** @@ -133,4 +178,5 @@ 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 28e24d09a705..726746ae47cc 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 @@ -65,7 +65,9 @@ private EngineDiscoveryRequestResolver(List, S /** * Resolve the supplied {@link EngineDiscoveryRequest} and collect the - * results into the supplied {@link TestDescriptor}. + * results into the supplied {@link TestDescriptor} while forwarding + * encountered discovery issues to the {@link EngineDiscoveryRequest}'s + * {@link org.junit.platform.engine.EngineDiscoveryListener}. * *

The algorithm works as follows: * @@ -110,8 +112,35 @@ private EngineDiscoveryRequestResolver(List, 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 = DiscoveryIssueReporter.create(request.getDiscoveryListener(), + DiscoveryIssueReporter issueReporter = DiscoveryIssueReporter.forwarding(request.getDiscoveryListener(), engineDescriptor.getUniqueId()); + resolve(request, engineDescriptor, issueReporter); + } + + /** + * Resolve the supplied {@link EngineDiscoveryRequest} and collect the + * results into the supplied {@link TestDescriptor} using the supplied + * {@link DiscoveryIssueReporter} to report issues encountered during + * resolution. + * + *

The algorithm works as described in + * {@link #resolve(EngineDiscoveryRequest, TestDescriptor)}. + * + * @param request the request to be resolved; never {@code null} + * @param engineDescriptor the engine's {@code TestDescriptor} to be used + * for adding direct children + * @param issueReporter the {@link DiscoveryIssueReporter} to report issues + * encountered during resolution + * @since 1.13 + * @see #resolve(EngineDiscoveryRequest, TestDescriptor) + * @see SelectorResolver + * @see TestDescriptor.Visitor + */ + @API(status = EXPERIMENTAL, since = "1.13") + public void resolve(EngineDiscoveryRequest request, T engineDescriptor, DiscoveryIssueReporter issueReporter) { + Preconditions.notNull(request, "request must not be null"); + Preconditions.notNull(engineDescriptor, "engineDescriptor must not be null"); + Preconditions.notNull(issueReporter, "issueReporter must not be null"); InitializationContext initializationContext = new DefaultInitializationContext<>(request, engineDescriptor, issueReporter); List resolvers = instantiate(resolverCreators, initializationContext); @@ -162,8 +191,28 @@ private Builder() { */ public Builder addClassContainerSelectorResolver(Predicate> classFilter) { Preconditions.notNull(classFilter, "classFilter must not be null"); - return addSelectorResolver( - context -> new ClassContainerSelectorResolver(classFilter, context.getClassNameFilter())); + return addClassContainerSelectorResolverWithContext(__ -> classFilter); + } + + /** + * Add a predefined resolver that resolves {@link ClasspathRootSelector + * ClasspathRootSelectors}, {@link ModuleSelector ModuleSelectors}, and + * {@link PackageSelector PackageSelectors} into {@link ClassSelector + * ClassSelectors} by scanning for classes that satisfy the predicate + * created by the supplied {@code Function} in the respective class + * containers to this builder. + * + * @param classFilterCreator the function that will be called to create + * the predicate the resolved classes must satisfy; never + * {@code null} + * @return this builder for method chaining + */ + @API(status = EXPERIMENTAL, since = "1.13") + public Builder addClassContainerSelectorResolverWithContext( + Function, Predicate>> classFilterCreator) { + Preconditions.notNull(classFilterCreator, "classFilterCreator must not be null"); + return addSelectorResolver(context -> new ClassContainerSelectorResolver(classFilterCreator.apply(context), + context.getClassNameFilter())); } /** diff --git a/junit-platform-suite-engine/src/main/java/org/junit/platform/suite/engine/LifecycleMethodUtils.java b/junit-platform-suite-engine/src/main/java/org/junit/platform/suite/engine/LifecycleMethodUtils.java index 87ce1ae662a4..5844ee0261b4 100644 --- a/junit-platform-suite-engine/src/main/java/org/junit/platform/suite/engine/LifecycleMethodUtils.java +++ b/junit-platform-suite-engine/src/main/java/org/junit/platform/suite/engine/LifecycleMethodUtils.java @@ -12,7 +12,6 @@ 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; @@ -54,12 +53,12 @@ private static List findMethodsAndCheckStaticAndNonPrivate(Class test DiscoveryIssueReporter issueReporter) { return findAnnotatedMethods(testClass, annotationType, traversalMode).stream() // - .filter(allOf( // - returnsPrimitiveVoid(annotationType, issueReporter), // - isStatic(annotationType, issueReporter), // - isNotPrivate(annotationType, issueReporter), // - hasNoParameters(annotationType, issueReporter) // - )) // + .filter(// + returnsPrimitiveVoid(annotationType, issueReporter) // + .and(isStatic(annotationType, issueReporter)) // + .and(isNotPrivate(annotationType, issueReporter)) // + .and(hasNoParameters(annotationType, issueReporter)) // + ) // .collect(toUnmodifiableList()); } diff --git a/jupiter-tests/src/test/java/org/junit/jupiter/engine/AbstractJupiterTestEngineTests.java b/jupiter-tests/src/test/java/org/junit/jupiter/engine/AbstractJupiterTestEngineTests.java index c8b93723188c..56459fc1e3e8 100644 --- a/jupiter-tests/src/test/java/org/junit/jupiter/engine/AbstractJupiterTestEngineTests.java +++ b/jupiter-tests/src/test/java/org/junit/jupiter/engine/AbstractJupiterTestEngineTests.java @@ -62,20 +62,30 @@ protected EngineDiscoveryResults discoverTestsForClass(Class testClass) { return discoverTests(selectClass(testClass)); } + protected EngineDiscoveryResults discoverTests(Consumer configurer) { + var builder = defaultRequest(); + configurer.accept(builder); + return discoverTests(builder); + } + protected EngineDiscoveryResults discoverTests(DiscoverySelector... selectors) { - return discoverTests(defaultRequest().selectors(selectors).build()); + return discoverTests(defaultRequest().selectors(selectors)); } - private static LauncherDiscoveryRequestBuilder defaultRequest() { - return request() // - .outputDirectoryProvider(dummyOutputDirectoryProvider()) // - .configurationParameter(STACKTRACE_PRUNING_ENABLED_PROPERTY_NAME, String.valueOf(false)); + protected EngineDiscoveryResults discoverTests(LauncherDiscoveryRequestBuilder builder) { + return discoverTests(builder.build()); } protected EngineDiscoveryResults discoverTests(LauncherDiscoveryRequest request) { return EngineTestKit.discover(this.engine, request); } + private static LauncherDiscoveryRequestBuilder defaultRequest() { + return request() // + .outputDirectoryProvider(dummyOutputDirectoryProvider()) // + .configurationParameter(STACKTRACE_PRUNING_ENABLED_PROPERTY_NAME, String.valueOf(false)); + } + protected UniqueId discoverUniqueId(Class clazz, String methodName) { var results = discoverTests(selectMethod(clazz, methodName)); var engineDescriptor = results.getEngineDescriptor(); diff --git a/jupiter-tests/src/test/java/org/junit/jupiter/engine/discovery/DiscoveryTests.java b/jupiter-tests/src/test/java/org/junit/jupiter/engine/discovery/DiscoveryTests.java index 002980399131..6583b4e2e8a1 100644 --- a/jupiter-tests/src/test/java/org/junit/jupiter/engine/discovery/DiscoveryTests.java +++ b/jupiter-tests/src/test/java/org/junit/jupiter/engine/discovery/DiscoveryTests.java @@ -10,13 +10,18 @@ package org.junit.jupiter.engine.discovery; +import static java.util.Comparator.comparing; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.fail; +import static org.junit.jupiter.api.Named.named; import static org.junit.jupiter.engine.discovery.JupiterUniqueIdBuilder.uniqueIdForTestTemplateMethod; import static org.junit.platform.commons.util.CollectionUtils.getOnlyElement; +import static org.junit.platform.engine.discovery.ClassNameFilter.includeClassNamePatterns; import static org.junit.platform.engine.discovery.DiscoverySelectors.selectClass; import static org.junit.platform.engine.discovery.DiscoverySelectors.selectMethod; import static org.junit.platform.engine.discovery.DiscoverySelectors.selectNestedMethod; +import static org.junit.platform.engine.discovery.DiscoverySelectors.selectPackage; import static org.junit.platform.engine.discovery.DiscoverySelectors.selectUniqueId; import static org.junit.platform.launcher.core.LauncherDiscoveryRequestBuilder.request; @@ -24,7 +29,9 @@ import java.lang.annotation.RetentionPolicy; import java.lang.reflect.Method; import java.util.List; +import java.util.regex.Pattern; +import org.junit.jupiter.api.Named; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestInfo; @@ -34,6 +41,9 @@ import org.junit.jupiter.engine.descriptor.ClassTestDescriptor; import org.junit.jupiter.engine.descriptor.NestedClassTestDescriptor; import org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; +import org.junit.platform.engine.DiscoveryIssue; import org.junit.platform.engine.TestDescriptor; import org.junit.platform.launcher.LauncherDiscoveryRequest; @@ -160,13 +170,48 @@ void discoverDeeplyNestedTestMethodByNestedMethodSelector() throws Exception { assertThat(methodDescriptor.getTestMethod().getName()).isEqualTo("test"); } + @ParameterizedTest + @MethodSource("requestsForTestClassWithInvalidTestMethod") + void reportsWarningForTestClassWithInvalidTestMethod(LauncherDiscoveryRequest request) throws Exception { + + var method = InvalidTestMethodTestCase.class.getDeclaredMethod("test"); + + var results = discoverTests(request); + + var discoveryIssues = results.getDiscoveryIssues().stream().sorted(comparing(DiscoveryIssue::message)).toList(); + assertThat(discoveryIssues).hasSize(3); + assertThat(discoveryIssues.getFirst().message()) // + .isEqualTo("@Test method '%s' must not be private. It will be not be executed.", + method.toGenericString()); + assertThat(discoveryIssues.get(1).message()) // + .isEqualTo("@Test method '%s' must not be static. It will be not be executed.", + method.toGenericString()); + assertThat(discoveryIssues.getLast().message()) // + .isEqualTo("@Test method '%s' must not return a value. It will be not be executed.", + method.toGenericString()); + } + + static List> requestsForTestClassWithInvalidTestMethod() { + return List.of( // + named("directly selected", request().selectors(selectClass(InvalidTestMethodTestCase.class)).build()), // + named("indirectly selected", request() // + .selectors(selectPackage(InvalidTestMethodTestCase.class.getPackageName())) // + .filters( + includeClassNamePatterns(Pattern.quote(InvalidTestMethodTestCase.class.getName()))).build()), // + named("subclasses", request() // + .selectors(selectClass(InvalidTestMethodSubclass1TestCase.class), + selectClass(InvalidTestMethodSubclass2TestCase.class)) // + .build()) // + ); + } + // ------------------------------------------------------------------- + @SuppressWarnings("unused") private static abstract class AbstractTestCase { @Test void abstractTest() { - } } @@ -228,4 +273,18 @@ class ConcreteInner1 extends AbstractSuperClass { } } + @SuppressWarnings("JUnitMalformedDeclaration") + static class InvalidTestMethodTestCase { + @Test + private static int test() { + return fail("should not be called"); + } + } + + static class InvalidTestMethodSubclass1TestCase extends InvalidTestMethodTestCase { + } + + static class InvalidTestMethodSubclass2TestCase extends InvalidTestMethodTestCase { + } + } diff --git a/jupiter-tests/src/test/java/org/junit/jupiter/engine/discovery/predicates/IsTestClassWithTestsTests.java b/jupiter-tests/src/test/java/org/junit/jupiter/engine/discovery/predicates/IsTestClassWithTestsTests.java index c07e5eff03c1..53658ebe7a8f 100644 --- a/jupiter-tests/src/test/java/org/junit/jupiter/engine/discovery/predicates/IsTestClassWithTestsTests.java +++ b/jupiter-tests/src/test/java/org/junit/jupiter/engine/discovery/predicates/IsTestClassWithTestsTests.java @@ -30,7 +30,8 @@ */ class IsTestClassWithTestsTests { - private final Predicate> isTestClassWithTests = new IsTestClassWithTests(); + private final Predicate> isTestClassWithTests = new IsTestClassWithTests(__ -> { + }); @Test void classWithTestMethodEvaluatesToTrue() { diff --git a/jupiter-tests/src/test/java/org/junit/jupiter/engine/discovery/predicates/IsTestFactoryMethodTests.java b/jupiter-tests/src/test/java/org/junit/jupiter/engine/discovery/predicates/IsTestFactoryMethodTests.java index 6487d0927db3..d01027a83dd0 100644 --- a/jupiter-tests/src/test/java/org/junit/jupiter/engine/discovery/predicates/IsTestFactoryMethodTests.java +++ b/jupiter-tests/src/test/java/org/junit/jupiter/engine/discovery/predicates/IsTestFactoryMethodTests.java @@ -11,10 +11,12 @@ package org.junit.jupiter.engine.discovery.predicates; import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.platform.commons.util.CollectionUtils.getOnlyElement; import java.lang.reflect.Method; import java.util.ArrayList; import java.util.Collection; +import java.util.List; import java.util.function.Predicate; import org.junit.jupiter.api.Disabled; @@ -22,6 +24,8 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestFactory; import org.junit.platform.commons.support.ReflectionSupport; +import org.junit.platform.engine.DiscoveryIssue; +import org.junit.platform.engine.support.descriptor.MethodSource; /** * Unit tests for {@link IsTestFactoryMethod}. @@ -30,7 +34,8 @@ */ class IsTestFactoryMethodTests { - private static final Predicate isTestFactoryMethod = new IsTestFactoryMethod(); + final List discoveryIssues = new ArrayList<>(); + final Predicate isTestFactoryMethod = new IsTestFactoryMethod(discoveryIssues::add); @Test void factoryMethodReturningCollectionOfDynamicTests() { @@ -39,7 +44,15 @@ void factoryMethodReturningCollectionOfDynamicTests() { @Test void bogusFactoryMethodReturningVoid() { - assertThat(isTestFactoryMethod).rejects(method("bogusVoidFactory")); + var method = method("bogusVoidFactory"); + + assertThat(isTestFactoryMethod).rejects(method); + + var issue = getOnlyElement(discoveryIssues); + assertThat(issue.severity()).isEqualTo(DiscoveryIssue.Severity.WARNING); + assertThat(issue.message()).isEqualTo( + "@TestFactory method '%s' must return a value. It will be not be executed.", method.toGenericString()); + assertThat(issue.source()).contains(MethodSource.from(method)); } // TODO [#949] Enable test once IsTestFactoryMethod properly checks return type. @@ -57,9 +70,10 @@ void bogusFactoryMethodReturningCollectionOfStrings() { } private static Method method(String name) { - return ReflectionSupport.findMethod(ClassWithTestFactoryMethods.class, name).get(); + return ReflectionSupport.findMethod(ClassWithTestFactoryMethods.class, name).orElseThrow(); } + @SuppressWarnings("unused") private static class ClassWithTestFactoryMethods { @TestFactory diff --git a/jupiter-tests/src/test/java/org/junit/jupiter/engine/discovery/predicates/IsTestMethodTests.java b/jupiter-tests/src/test/java/org/junit/jupiter/engine/discovery/predicates/IsTestMethodTests.java index c8787aafaa20..5336386614f4 100644 --- a/jupiter-tests/src/test/java/org/junit/jupiter/engine/discovery/predicates/IsTestMethodTests.java +++ b/jupiter-tests/src/test/java/org/junit/jupiter/engine/discovery/predicates/IsTestMethodTests.java @@ -10,16 +10,25 @@ package org.junit.jupiter.engine.discovery.predicates; +import static java.util.Comparator.comparing; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.platform.commons.util.CollectionUtils.getOnlyElement; import java.lang.reflect.Method; +import java.util.ArrayList; +import java.util.List; import java.util.function.Predicate; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestInfo; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.junit.platform.commons.support.ModifierSupport; import org.junit.platform.commons.support.ReflectionSupport; +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 IsTestMethod}. @@ -28,7 +37,8 @@ */ class IsTestMethodTests { - private static final Predicate isTestMethod = new IsTestMethod(); + final List discoveryIssues = new ArrayList<>(); + final Predicate isTestMethod = new IsTestMethod(discoveryIssues::add); @Test void publicTestMethod() { @@ -58,75 +68,138 @@ void packageVisibleTestMethod() { @Test void bogusAbstractTestMethod() { - assertThat(isTestMethod).rejects(abstractMethod("bogusAbstractTestMethod")); + var method = abstractMethod("bogusAbstractTestMethod"); + + assertThat(isTestMethod).rejects(method); + + var issue = getOnlyElement(discoveryIssues); + assertThat(issue.severity()).isEqualTo(Severity.WARNING); + assertThat(issue.message()).isEqualTo("@Test method '%s' must not be abstract. It will be not be executed.", + method.toGenericString()); + assertThat(issue.source()).contains(MethodSource.from(method)); } @Test - void bogusStaticTestMethod() { - assertThat(isTestMethod).rejects(method("bogusStaticTestMethod")); + void bogusAbstractNonVoidTestMethod() { + var method = abstractMethod("bogusAbstractNonVoidTestMethod"); + + assertThat(isTestMethod).rejects(method); + + assertThat(discoveryIssues).hasSize(2); + discoveryIssues.sort(comparing(DiscoveryIssue::message)); + assertThat(discoveryIssues.getFirst().message()) // + .isEqualTo("@Test method '%s' must not be abstract. It will be not be executed.", + method.toGenericString()); + assertThat(discoveryIssues.getLast().message()) // + .isEqualTo("@Test method '%s' must not return a value. It will be not be executed.", + method.toGenericString()); } @Test - void bogusPrivateTestMethod() { - assertThat(isTestMethod).rejects(method("bogusPrivateTestMethod")); + void bogusStaticTestMethod() { + var method = method("bogusStaticTestMethod"); + + assertThat(isTestMethod).rejects(method); + + var issue = getOnlyElement(discoveryIssues); + assertThat(issue.severity()).isEqualTo(Severity.WARNING); + assertThat(issue.message()).isEqualTo("@Test method '%s' must not be static. It will be not be executed.", + method.toGenericString()); + assertThat(issue.source()).contains(MethodSource.from(method)); } @Test - void bogusTestMethodReturningObject() { - assertThat(isTestMethod).rejects(method("bogusTestMethodReturningObject")); + void bogusPrivateTestMethod() { + var method = method("bogusPrivateTestMethod"); + + assertThat(isTestMethod).rejects(method); + + var issue = getOnlyElement(discoveryIssues); + assertThat(issue.severity()).isEqualTo(Severity.WARNING); + assertThat(issue.message()).isEqualTo("@Test method '%s' must not be private. It will be not be executed.", + method.toGenericString()); + assertThat(issue.source()).contains(MethodSource.from(method)); } - @Test - void bogusTestMethodReturningVoidReference() { - assertThat(isTestMethod).rejects(method("bogusTestMethodReturningVoidReference")); + @ParameterizedTest + @ValueSource(strings = { "bogusTestMethodReturningObject", "bogusTestMethodReturningVoidReference", + "bogusTestMethodReturningPrimitive" }) + void bogusNonVoidTestMethods(String methodName) { + var method = method(methodName); + + assertThat(isTestMethod).rejects(method); + + var issue = getOnlyElement(discoveryIssues); + assertThat(issue.severity()).isEqualTo(Severity.WARNING); + assertThat(issue.message()).isEqualTo("@Test method '%s' must not return a value. It will be not be executed.", + method.toGenericString()); + assertThat(issue.source()).contains(MethodSource.from(method)); } @Test - void bogusTestMethodReturningPrimitive() { - assertThat(isTestMethod).rejects(method("bogusTestMethodReturningPrimitive")); + void bogusStaticPrivateNonVoidTestMethod() { + var method = method("bogusStaticPrivateNonVoidTestMethod"); + + assertThat(isTestMethod).rejects(method); + + assertThat(discoveryIssues).hasSize(3); + discoveryIssues.sort(comparing(DiscoveryIssue::message)); + assertThat(discoveryIssues.getFirst().message()) // + .isEqualTo("@Test method '%s' must not be private. It will be not be executed.", + method.toGenericString()); + assertThat(discoveryIssues.get(1).message()) // + .isEqualTo("@Test method '%s' must not be static. It will be not be executed.", + method.toGenericString()); + assertThat(discoveryIssues.getLast().message()) // + .isEqualTo("@Test method '%s' must not return a value. It will be not be executed.", + method.toGenericString()); } private static Method method(String name, Class... parameterTypes) { - return ReflectionSupport.findMethod(ClassWithTestMethods.class, name, parameterTypes).get(); + return ReflectionSupport.findMethod(ClassWithTestMethods.class, name, parameterTypes).orElseThrow(); } private Method abstractMethod(String name) { - return ReflectionSupport.findMethod(AbstractClassWithAbstractTestMethod.class, name).get(); + return ReflectionSupport.findMethod(AbstractClassWithAbstractTestMethod.class, name).orElseThrow(); } + @SuppressWarnings({ "JUnitMalformedDeclaration", "unused" }) private static abstract class AbstractClassWithAbstractTestMethod { @Test abstract void bogusAbstractTestMethod(); + @Test + abstract int bogusAbstractNonVoidTestMethod(); + } - @SuppressWarnings("JUnitMalformedDeclaration") + @SuppressWarnings({ "JUnitMalformedDeclaration", "unused" }) private static class ClassWithTestMethods { - @SuppressWarnings("JUnitMalformedDeclaration") @Test static void bogusStaticTestMethod() { } - @SuppressWarnings("JUnitMalformedDeclaration") @Test private void bogusPrivateTestMethod() { } - @SuppressWarnings("JUnitMalformedDeclaration") + @Test + private static int bogusStaticPrivateNonVoidTestMethod() { + return 42; + } + @Test String bogusTestMethodReturningObject() { return ""; } - @SuppressWarnings("JUnitMalformedDeclaration") @Test Void bogusTestMethodReturningVoidReference() { return null; } - @SuppressWarnings("JUnitMalformedDeclaration") @Test int bogusTestMethodReturningPrimitive() { return 0; diff --git a/jupiter-tests/src/test/java/org/junit/jupiter/engine/discovery/predicates/IsTestTemplateMethodTests.java b/jupiter-tests/src/test/java/org/junit/jupiter/engine/discovery/predicates/IsTestTemplateMethodTests.java index a71da9ec39b0..82473b4e0163 100644 --- a/jupiter-tests/src/test/java/org/junit/jupiter/engine/discovery/predicates/IsTestTemplateMethodTests.java +++ b/jupiter-tests/src/test/java/org/junit/jupiter/engine/discovery/predicates/IsTestTemplateMethodTests.java @@ -11,12 +11,17 @@ package org.junit.jupiter.engine.discovery.predicates; import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.platform.commons.util.CollectionUtils.getOnlyElement; import java.lang.reflect.Method; +import java.util.ArrayList; +import java.util.List; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestTemplate; import org.junit.platform.commons.support.ReflectionSupport; +import org.junit.platform.engine.DiscoveryIssue; +import org.junit.platform.engine.support.descriptor.MethodSource; /** * Unit tests for {@link IsTestTemplateMethod}. @@ -25,7 +30,8 @@ */ class IsTestTemplateMethodTests { - private static final IsTestTemplateMethod isTestTemplateMethod = new IsTestTemplateMethod(); + final List discoveryIssues = new ArrayList<>(); + final IsTestTemplateMethod isTestTemplateMethod = new IsTestTemplateMethod(discoveryIssues::add); @Test void testTemplateMethodReturningVoid() { @@ -34,13 +40,22 @@ void testTemplateMethodReturningVoid() { @Test void bogusTestTemplateMethodReturningObject() { - assertThat(isTestTemplateMethod).rejects(method("bogusTemplateReturningObject")); + var method = method("bogusTemplateReturningObject"); + + assertThat(isTestTemplateMethod).rejects(method); + + var issue = getOnlyElement(discoveryIssues); + assertThat(issue.severity()).isEqualTo(DiscoveryIssue.Severity.WARNING); + assertThat(issue.message()).isEqualTo( + "@TestTemplate method '%s' must not return a value. It will be not be executed.", method.toGenericString()); + assertThat(issue.source()).contains(MethodSource.from(method)); } private static Method method(String name) { - return ReflectionSupport.findMethod(ClassWithTestTemplateMethods.class, name).get(); + return ReflectionSupport.findMethod(ClassWithTestTemplateMethods.class, name).orElseThrow(); } + @SuppressWarnings("unused") private static class ClassWithTestTemplateMethods { @TestTemplate diff --git a/platform-tests/src/test/java/org/junit/platform/commons/support/ModifierSupportTests.java b/platform-tests/src/test/java/org/junit/platform/commons/support/ModifierSupportTests.java index 27e1092c4441..18332317aa51 100644 --- a/platform-tests/src/test/java/org/junit/platform/commons/support/ModifierSupportTests.java +++ b/platform-tests/src/test/java/org/junit/platform/commons/support/ModifierSupportTests.java @@ -102,6 +102,23 @@ void isAbstractDelegates(Method method) { assertEquals(ReflectionUtils.isAbstract(method), ModifierSupport.isAbstract(method)); } + @Test + void isNotAbstractPreconditions() { + assertPreconditionViolationException("Class", () -> ModifierSupport.isNotAbstract((Class) null)); + assertPreconditionViolationException("Member", () -> ModifierSupport.isNotAbstract((Member) null)); + } + + @Classes + void isNotAbstractDelegates(Class clazz) { + assertEquals(ReflectionUtils.isNotAbstract(clazz), ModifierSupport.isNotAbstract(clazz)); + } + + @SuppressWarnings("JUnitMalformedDeclaration") + @Methods + void isNotAbstractDelegates(Method method) { + assertEquals(ReflectionUtils.isNotAbstract(method), ModifierSupport.isNotAbstract(method)); + } + @Test void isStaticPreconditions() { assertPreconditionViolationException("Class", () -> ModifierSupport.isStatic((Class) null)); diff --git a/platform-tests/src/test/java/org/junit/platform/suite/engine/SuiteTestDescriptorTests.java b/platform-tests/src/test/java/org/junit/platform/suite/engine/SuiteTestDescriptorTests.java index fac2f2cf97ea..c3dafe5132e7 100644 --- a/platform-tests/src/test/java/org/junit/platform/suite/engine/SuiteTestDescriptorTests.java +++ b/platform-tests/src/test/java/org/junit/platform/suite/engine/SuiteTestDescriptorTests.java @@ -13,7 +13,6 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.junit.jupiter.api.Assertions.assertAll; -import static org.mockito.Mockito.mock; import java.util.Collections; import java.util.Optional; @@ -50,7 +49,7 @@ class SuiteTestDescriptorTests { final ConfigurationParameters configurationParameters = new EmptyConfigurationParameters(); final OutputDirectoryProvider outputDirectoryProvider = OutputDirectoryProviders.dummyOutputDirectoryProvider(); - final DiscoveryIssueReporter discoveryIssueReporter = DiscoveryIssueReporter.create(mock(), engineId); + final DiscoveryIssueReporter discoveryIssueReporter = DiscoveryIssueReporter.discarding(); final SuiteTestDescriptor suite = new SuiteTestDescriptor(suiteId, TestSuite.class, configurationParameters, outputDirectoryProvider, discoveryIssueReporter);