diff --git a/documentation/src/docs/asciidoc/release-notes/release-notes-6.0.1.adoc b/documentation/src/docs/asciidoc/release-notes/release-notes-6.0.1.adoc index d5fbb3c02d43..d3dc71c4595b 100644 --- a/documentation/src/docs/asciidoc/release-notes/release-notes-6.0.1.adoc +++ b/documentation/src/docs/asciidoc/release-notes/release-notes-6.0.1.adoc @@ -47,6 +47,8 @@ repository on GitHub. assertion. * Stop reporting discovery issues for synthetic methods, particularly in conjunction with Kotlin suspend functions. +* Fix support for test methods with the same signature as a package-private methods + declared in super classes in different packages. [[release-notes-6.0.1-junit-jupiter-deprecations-and-breaking-changes]] ==== Deprecations and Breaking Changes 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 3515a5cfabd7..2f64d22a7af2 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 @@ -323,11 +323,7 @@ private DiscoverySelector selectClass(List> classes) { } private DiscoverySelector selectMethod(List> classes, Method method) { - if (classes.size() == 1) { - return DiscoverySelectors.selectMethod(classes.get(0), method); - } - int lastIndex = classes.size() - 1; - return DiscoverySelectors.selectNestedMethod(classes.subList(0, lastIndex), classes.get(lastIndex), method); + return new DeclaredMethodSelector(classes, method); } static class DummyClassTemplateInvocationContext implements ClassTemplateInvocationContext { diff --git a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/DeclaredMethodSelector.java b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/DeclaredMethodSelector.java new file mode 100644 index 000000000000..394dd3a8b81c --- /dev/null +++ b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/DeclaredMethodSelector.java @@ -0,0 +1,39 @@ +/* + * 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.discovery; + +import java.lang.reflect.Method; +import java.util.List; + +import org.junit.platform.commons.util.Preconditions; +import org.junit.platform.engine.DiscoverySelector; +import org.junit.platform.engine.discovery.MethodSelector; + +/** + * Jupiter-specific selector for methods, potentially in nested classes. + * + *

The important difference to {@link MethodSelector} is that this selector's + * {@link #equals(Object)} method takes into account the selected method's + * {@linkplain Method#getDeclaringClass() declaring class} to support cases + * where a package-private method is declared in a super class in a different + * package and a method with the same signature is declared in a subclass. In + * that case both methods should be discovered because the one declared in the + * subclass does not override the one in the super class. + * + * @since 6.0.1 + */ +record DeclaredMethodSelector(List> testClasses, Method method) implements DiscoverySelector { + DeclaredMethodSelector { + Preconditions.notEmpty(testClasses, "testClasses must not be empty"); + Preconditions.containsNoNullElements(testClasses, "testClasses must not contain null elements"); + Preconditions.notNull(method, "method must not be null"); + } +} diff --git a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/MethodFinder.java b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/MethodFinder.java deleted file mode 100644 index 10bb24ab5139..000000000000 --- a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/MethodFinder.java +++ /dev/null @@ -1,40 +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.jupiter.engine.discovery; - -import java.lang.reflect.Method; -import java.util.Optional; -import java.util.regex.Matcher; -import java.util.regex.Pattern; - -import org.junit.platform.commons.support.ReflectionSupport; -import org.junit.platform.commons.util.Preconditions; - -/** - * @since 5.0 - */ -class MethodFinder { - - // Pattern: methodName(comma-separated list of parameter type names) - private static final Pattern METHOD_PATTERN = Pattern.compile("(.+)\\((.*)\\)"); - - Optional findMethod(String methodSpecPart, Class clazz) { - Matcher matcher = METHOD_PATTERN.matcher(methodSpecPart); - - Preconditions.condition(matcher.matches(), - () -> "Method [%s] does not match pattern [%s]".formatted(methodSpecPart, METHOD_PATTERN)); - - String methodName = matcher.group(1); - String parameterTypeNames = matcher.group(2); - return ReflectionSupport.findMethod(clazz, methodName, parameterTypeNames); - } - -} diff --git a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/MethodSegmentResolver.java b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/MethodSegmentResolver.java new file mode 100644 index 000000000000..e9492910ec9c --- /dev/null +++ b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/MethodSegmentResolver.java @@ -0,0 +1,68 @@ +/* + * 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.discovery; + +import static org.junit.platform.commons.util.ReflectionUtils.isPackagePrivate; + +import java.lang.reflect.Method; +import java.util.Optional; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import org.junit.platform.commons.PreconditionViolationException; +import org.junit.platform.commons.support.ReflectionSupport; +import org.junit.platform.commons.util.ClassUtils; +import org.junit.platform.commons.util.Preconditions; +import org.junit.platform.commons.util.ReflectionUtils; + +/** + * @since 5.0 + */ +class MethodSegmentResolver { + + // Pattern: [declaringClassName#]methodName(comma-separated list of parameter type names) + private static final Pattern METHOD_PATTERN = Pattern.compile( + "(?:(?.+)#)?(?.+)\\((?.*)\\)"); + + /** + * If the {@code method} is package-private and declared a class in a + * different package than {@code testClass}, the declaring class name is + * included in the method's unique ID segment. Otherwise, it only + * consists of the method name and its parameter types. + */ + String formatMethodSpecPart(Method method, Class testClass) { + var parameterTypes = ClassUtils.nullSafeToString(method.getParameterTypes()); + if (isPackagePrivate(method) + && !method.getDeclaringClass().getPackageName().equals(testClass.getPackageName())) { + return "%s#%s(%s)".formatted(method.getDeclaringClass().getName(), method.getName(), parameterTypes); + } + return "%s(%s)".formatted(method.getName(), parameterTypes); + } + + Optional findMethod(String methodSpecPart, Class testClass) { + Matcher matcher = METHOD_PATTERN.matcher(methodSpecPart); + + Preconditions.condition(matcher.matches(), + () -> "Method [%s] does not match pattern [%s]".formatted(methodSpecPart, METHOD_PATTERN)); + + Class targetClass = testClass; + String declaringClass = matcher.group("declaringClass"); + if (declaringClass != null) { + targetClass = ReflectionUtils.tryToLoadClass(declaringClass).getNonNullOrThrow( + cause -> new PreconditionViolationException( + "Could not load declaring class with name: " + declaringClass, cause)); + } + String methodName = matcher.group("method"); + String parameterTypeNames = matcher.group("parameters"); + return ReflectionSupport.findMethod(targetClass, methodName, parameterTypeNames); + } + +} diff --git a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/MethodSelectorResolver.java b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/MethodSelectorResolver.java index ad70aaa7913f..07dbb1b3d068 100644 --- a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/MethodSelectorResolver.java +++ b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/MethodSelectorResolver.java @@ -39,7 +39,6 @@ import org.junit.jupiter.engine.discovery.predicates.IsTestMethod; import org.junit.jupiter.engine.discovery.predicates.IsTestTemplateMethod; import org.junit.jupiter.engine.discovery.predicates.TestClassPredicates; -import org.junit.platform.commons.util.ClassUtils; import org.junit.platform.engine.DiscoveryIssue; import org.junit.platform.engine.DiscoveryIssue.Severity; import org.junit.platform.engine.DiscoverySelector; @@ -59,7 +58,7 @@ */ class MethodSelectorResolver implements SelectorResolver { - private static final MethodFinder methodFinder = new MethodFinder(); + private static final MethodSegmentResolver methodSegmentResolver = new MethodSegmentResolver(); private final Predicate> testClassPredicate; private final JupiterConfiguration configuration; @@ -84,6 +83,20 @@ public Resolution resolve(NestedMethodSelector selector, Context context) { Match::exact); } + @Override + public Resolution resolve(DiscoverySelector selector, Context context) { + if (selector instanceof DeclaredMethodSelector methodSelector) { + var testClasses = methodSelector.testClasses(); + if (testClasses.size() == 1) { + return resolve(context, emptyList(), testClasses.get(0), methodSelector::method, Match::exact); + } + int lastIndex = testClasses.size() - 1; + return resolve(context, testClasses.subList(0, lastIndex), testClasses.get(lastIndex), + methodSelector::method, Match::exact); + } + return unresolved(); + } + private Resolution resolve(Context context, List> enclosingClasses, Class testClass, Supplier methodSupplier, BiFunction>, Match> matchFactory) { @@ -209,7 +222,7 @@ Optional resolveUniqueIdIntoTestDescriptor(UniqueId uniqueId, Co String methodSpecPart = lastSegment.getValue(); Class testClass = ((TestClassAware) parent).getTestClass(); // @formatter:off - return methodFinder.findMethod(methodSpecPart, testClass) + return methodSegmentResolver.findMethod(methodSpecPart, testClass) .filter(methodPredicate) .map(method -> createTestDescriptor(parent, testClass, method, configuration)); // @formatter:on @@ -223,15 +236,14 @@ Optional resolveUniqueIdIntoTestDescriptor(UniqueId uniqueId, Co private TestDescriptor createTestDescriptor(TestDescriptor parent, Class testClass, Method method, JupiterConfiguration configuration) { - UniqueId uniqueId = createUniqueId(method, parent); + UniqueId uniqueId = createUniqueId(method, parent, testClass); return testDescriptorFactory.create(uniqueId, testClass, method, ((TestClassAware) parent)::getEnclosingTestClasses, configuration); } - private UniqueId createUniqueId(Method method, TestDescriptor parent) { - String methodId = "%s(%s)".formatted(method.getName(), - ClassUtils.nullSafeToString(method.getParameterTypes())); - return parent.getUniqueId().append(segmentType, methodId); + private UniqueId createUniqueId(Method method, TestDescriptor parent, Class testClass) { + return parent.getUniqueId().append(segmentType, + methodSegmentResolver.formatMethodSpecPart(method, testClass)); } interface TestDescriptorFactory { 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 4e47d4c1be0e..d80225890773 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 @@ -1872,7 +1872,11 @@ private static boolean isMethodOverriddenBy(Method upper, Method lower) { return hasCompatibleSignature(upper, lower.getName(), lower.getParameterTypes()); } - private static boolean isPackagePrivate(Member member) { + /** + * @since 6.0.1 + */ + @API(status = INTERNAL, since = "6.0.1") + public static boolean isPackagePrivate(Member member) { int modifiers = member.getModifiers(); return !(Modifier.isPublic(modifiers) || Modifier.isProtected(modifiers) || Modifier.isPrivate(modifiers)); } diff --git a/junit-platform-engine/src/main/java/org/junit/platform/engine/discovery/MethodSelector.java b/junit-platform-engine/src/main/java/org/junit/platform/engine/discovery/MethodSelector.java index 27fd6aea479a..5e90ac31538c 100644 --- a/junit-platform-engine/src/main/java/org/junit/platform/engine/discovery/MethodSelector.java +++ b/junit-platform-engine/src/main/java/org/junit/platform/engine/discovery/MethodSelector.java @@ -82,44 +82,38 @@ public final class MethodSelector implements DiscoverySelector { } MethodSelector(Class javaClass, String methodName, String parameterTypeNames) { - this.classLoader = javaClass.getClassLoader(); + this(javaClass.getClassLoader(), javaClass.getName(), methodName, parameterTypeNames); this.javaClass = javaClass; - this.className = javaClass.getName(); - this.methodName = methodName; - this.parameterTypeNames = parameterTypeNames; } /** * @since 1.10 */ MethodSelector(@Nullable ClassLoader classLoader, String className, String methodName, Class... parameterTypes) { - this.classLoader = classLoader; - this.className = className; - this.methodName = methodName; + this(classLoader, className, methodName, ClassUtils.nullSafeToString(Class::getTypeName, parameterTypes)); this.parameterTypes = parameterTypes.clone(); - this.parameterTypeNames = ClassUtils.nullSafeToString(Class::getTypeName, this.parameterTypes); } /** * @since 1.10 */ MethodSelector(Class javaClass, String methodName, Class... parameterTypes) { - this.classLoader = javaClass.getClassLoader(); + this(javaClass.getClassLoader(), javaClass.getName(), methodName, + ClassUtils.nullSafeToString(Class::getTypeName, parameterTypes)); this.javaClass = javaClass; - this.className = javaClass.getName(); - this.methodName = methodName; this.parameterTypes = parameterTypes.clone(); - this.parameterTypeNames = ClassUtils.nullSafeToString(Class::getTypeName, this.parameterTypes); } MethodSelector(Class javaClass, Method method) { - this.classLoader = javaClass.getClassLoader(); + this(javaClass, method, method.getParameterTypes()); + } + + private MethodSelector(Class javaClass, Method method, Class... parameterTypes) { + this(javaClass.getClassLoader(), javaClass.getName(), method.getName(), + ClassUtils.nullSafeToString(Class::getTypeName, parameterTypes)); this.javaClass = javaClass; - this.className = javaClass.getName(); this.javaMethod = method; - this.methodName = method.getName(); - this.parameterTypes = method.getParameterTypes(); - this.parameterTypeNames = ClassUtils.nullSafeToString(Class::getTypeName, this.parameterTypes); + this.parameterTypes = parameterTypes; } /** diff --git a/jupiter-tests/src/test/java/org/junit/jupiter/engine/TestMethodOverridingTests.java b/jupiter-tests/src/test/java/org/junit/jupiter/engine/TestMethodOverridingTests.java new file mode 100644 index 000000000000..6257a41416ff --- /dev/null +++ b/jupiter-tests/src/test/java/org/junit/jupiter/engine/TestMethodOverridingTests.java @@ -0,0 +1,107 @@ +/* + * 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; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.tuple; +import static org.junit.platform.commons.util.CollectionUtils.getOnlyElement; +import static org.junit.platform.engine.discovery.DiscoverySelectors.selectUniqueId; + +import java.lang.reflect.Method; +import java.util.Map; +import java.util.stream.Stream; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInfo; +import org.junit.jupiter.api.TestReporter; +import org.junit.jupiter.engine.subpackage.SuperClassWithPackagePrivateLifecycleMethodInDifferentPackageTestCase; +import org.junit.platform.engine.TestDescriptor; +import org.junit.platform.engine.reporting.ReportEntry; +import org.junit.platform.engine.support.descriptor.MethodSource; +import org.junit.platform.testkit.engine.EngineExecutionResults; + +/** + * @since 6.0.1 + */ +class TestMethodOverridingTests extends AbstractJupiterTestEngineTests { + + @Test + void bothPackagePrivateTestMethodsAreDiscovered() throws Exception { + var results = discoverTestsForClass(DuplicateTestMethodDueToPackagePrivateVisibilityTestCase.class); + var classDescriptor = getOnlyElement(results.getEngineDescriptor().getChildren()); + + var parentUniqueId = classDescriptor.getUniqueId(); + var inheritedMethodUniqueId = parentUniqueId.append("method", + "org.junit.jupiter.engine.subpackage.SuperClassWithPackagePrivateLifecycleMethodInDifferentPackageTestCase#" + + "test(org.junit.jupiter.api.TestInfo, org.junit.jupiter.api.TestReporter)"); + var declaredMethodUniqueId = parentUniqueId.append("method", + "test(org.junit.jupiter.api.TestInfo, org.junit.jupiter.api.TestReporter)"); + + var inheritedMethod = SuperClassWithPackagePrivateLifecycleMethodInDifferentPackageTestCase.class.getDeclaredMethod( + "test", TestInfo.class, TestReporter.class); + var declaredMethod = DuplicateTestMethodDueToPackagePrivateVisibilityTestCase.class.getDeclaredMethod("test", + TestInfo.class, TestReporter.class); + + assertThat(classDescriptor.getChildren()) // + .hasSize(2) // + .extracting(TestDescriptor::getUniqueId, TestMethodOverridingTests::getSourceMethod) // + .containsExactly(tuple(inheritedMethodUniqueId, inheritedMethod), + tuple(declaredMethodUniqueId, declaredMethod)); + + results = discoverTests(selectUniqueId(inheritedMethodUniqueId)); + classDescriptor = getOnlyElement(results.getEngineDescriptor().getChildren()); + assertThat(classDescriptor.getChildren()) // + .extracting(TestDescriptor::getUniqueId, TestMethodOverridingTests::getSourceMethod) // + .containsExactly(tuple(inheritedMethodUniqueId, inheritedMethod)); + + results = discoverTests(selectUniqueId(declaredMethodUniqueId)); + classDescriptor = getOnlyElement(results.getEngineDescriptor().getChildren()); + assertThat(classDescriptor.getChildren()) // + .extracting(TestDescriptor::getUniqueId, TestMethodOverridingTests::getSourceMethod) // + .containsExactly(tuple(declaredMethodUniqueId, declaredMethod)); + } + + @Test + void bothPackagePrivateTestMethodsAreExecuted() throws Exception { + var results = executeTestsForClass(DuplicateTestMethodDueToPackagePrivateVisibilityTestCase.class); + + results.testEvents().assertStatistics(stats -> stats.started(2).succeeded(2)); + assertThat(allReportEntries(results)) // + .containsExactly( + Map.of("invokedSuper", + SuperClassWithPackagePrivateLifecycleMethodInDifferentPackageTestCase.class.getDeclaredMethod( + "test", TestInfo.class, TestReporter.class).toGenericString()), + Map.of("invokedChild", + DuplicateTestMethodDueToPackagePrivateVisibilityTestCase.class.getDeclaredMethod("test", + TestInfo.class, TestReporter.class).toGenericString())); + } + + private static Method getSourceMethod(TestDescriptor it) { + return ((MethodSource) it.getSource().orElseThrow()).getJavaMethod(); + } + + private static Stream> allReportEntries(EngineExecutionResults results) { + return results.allEvents().reportingEntryPublished() // + .map(event -> event.getRequiredPayload(ReportEntry.class)) // + .map(ReportEntry::getKeyValuePairs); + } + + @SuppressWarnings("JUnitMalformedDeclaration") + static class DuplicateTestMethodDueToPackagePrivateVisibilityTestCase + extends SuperClassWithPackagePrivateLifecycleMethodInDifferentPackageTestCase { + + // @Override + @Test + void test(TestInfo testInfo, TestReporter reporter) { + reporter.publishEntry("invokedChild", testInfo.getTestMethod().orElseThrow().toGenericString()); + } + } +} diff --git a/jupiter-tests/src/test/java/org/junit/jupiter/engine/subpackage/SuperClassWithPackagePrivateLifecycleMethodInDifferentPackageTestCase.java b/jupiter-tests/src/test/java/org/junit/jupiter/engine/subpackage/SuperClassWithPackagePrivateLifecycleMethodInDifferentPackageTestCase.java index 07f895044a7b..97b895a2efae 100644 --- a/jupiter-tests/src/test/java/org/junit/jupiter/engine/subpackage/SuperClassWithPackagePrivateLifecycleMethodInDifferentPackageTestCase.java +++ b/jupiter-tests/src/test/java/org/junit/jupiter/engine/subpackage/SuperClassWithPackagePrivateLifecycleMethodInDifferentPackageTestCase.java @@ -14,6 +14,8 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInfo; +import org.junit.jupiter.api.TestReporter; /** * @since 5.9 @@ -28,7 +30,8 @@ void beforeEach() { } @Test - void test() { + void test(TestInfo testInfo, TestReporter reporter) { + reporter.publishEntry("invokedSuper", testInfo.getTestMethod().orElseThrow().toGenericString()); assertThat(this.beforeEachInvoked).isTrue(); }