From b2c6c302f408afc949c6d6eac785b1727d5f96e5 Mon Sep 17 00:00:00 2001 From: Marc Philipp Date: Mon, 27 Oct 2025 16:43:20 +0100 Subject: [PATCH 1/8] Fix support for package-private test methods Prior to this commit, when the test super class wss declared in another package and declared a package-private test method with the same signature as the subclass, the method from the subclass was silently ignored. With the changes in this commit, both test methods are executed, albeit with the same display name. Fixes #5098. --- .../release-notes/release-notes-6.0.1.adoc | 2 + .../engine/discovery/MethodFinder.java | 20 +++- .../discovery/MethodSelectorResolver.java | 18 +++- .../commons/util/ReflectionUtils.java | 6 +- .../engine/discovery/MethodSelector.java | 10 +- .../engine/TestMethodOverridingTests.java | 95 +++++++++++++++++++ ...cycleMethodInDifferentPackageTestCase.java | 5 +- .../engine/discovery/MethodSelectorTests.java | 20 +++- 8 files changed, 162 insertions(+), 14 deletions(-) create mode 100644 jupiter-tests/src/test/java/org/junit/jupiter/engine/TestMethodOverridingTests.java 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 af78da2bc298..da945e63ffc7 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 @@ -45,6 +45,8 @@ repository on GitHub. to avoid conflicts with other control characters. * Fix `IllegalAccessError` thrown when using the Kotlin-specific `assertDoesNotThrow` assertion. +* 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/MethodFinder.java b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/MethodFinder.java index 10bb24ab5139..fe29a9bc5639 100644 --- 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 @@ -15,16 +15,19 @@ 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.Preconditions; +import org.junit.platform.commons.util.ReflectionUtils; /** * @since 5.0 */ class MethodFinder { - // Pattern: methodName(comma-separated list of parameter type names) - private static final Pattern METHOD_PATTERN = Pattern.compile("(.+)\\((.*)\\)"); + // Pattern: [declaringClassName#]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); @@ -32,9 +35,16 @@ Optional findMethod(String methodSpecPart, Class clazz) { 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); + Class targetClass = clazz; + 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..2f4af4665239 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 @@ -13,6 +13,7 @@ import static java.util.Collections.emptyList; import static java.util.Collections.emptySet; import static java.util.stream.Collectors.toSet; +import static org.junit.platform.commons.util.ReflectionUtils.isPackagePrivate; import static org.junit.platform.engine.discovery.DiscoverySelectors.selectUniqueId; import static org.junit.platform.engine.support.discovery.SelectorResolver.Resolution.matches; import static org.junit.platform.engine.support.discovery.SelectorResolver.Resolution.unresolved; @@ -223,15 +224,22 @@ 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, computeMethodId(method, testClass)); + } + + private static String computeMethodId(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); } 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..ddb74efafbb2 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 @@ -275,9 +275,17 @@ public boolean equals(Object o) { return false; } MethodSelector that = (MethodSelector) o; - return Objects.equals(this.className, that.className)// + var equal = Objects.equals(this.className, that.className)// && Objects.equals(this.methodName, that.methodName)// && Objects.equals(this.parameterTypeNames, that.parameterTypeNames); + if (equal) { + var thisJavaMethod = this.javaMethod; + var thatJavaMethod = that.javaMethod; + if (thisJavaMethod != null && thatJavaMethod != null) { + return thisJavaMethod.equals(thatJavaMethod); + } + } + return equal; } /** 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..b2bfb0848d12 --- /dev/null +++ b/jupiter-tests/src/test/java/org/junit/jupiter/engine/TestMethodOverridingTests.java @@ -0,0 +1,95 @@ +/* + * 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.junit.platform.commons.util.CollectionUtils.getOnlyElement; +import static org.junit.platform.engine.discovery.DiscoverySelectors.selectUniqueId; + +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.testkit.engine.EngineExecutionResults; + +/** + * @since 6.0.1 + */ +class TestMethodOverridingTests extends AbstractJupiterTestEngineTests { + + @Test + void bothPackagePrivateTestMethodsAreDiscovered() { + var results = discoverTestsForClass(DuplicateTestMethodDueToPackagePrivateVisibilityTestCase.class); + var classDescriptor = getOnlyElement(results.getEngineDescriptor().getChildren()); + + assertThat(classDescriptor.getChildren()).hasSize(2); + + 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)"); + + assertThat(classDescriptor.getChildren()) // + .extracting(TestDescriptor::getUniqueId) // + .containsExactly(inheritedMethodUniqueId, declaredMethodUniqueId); + + results = discoverTests(selectUniqueId(inheritedMethodUniqueId)); + classDescriptor = getOnlyElement(results.getEngineDescriptor().getChildren()); + assertThat(classDescriptor.getChildren()) // + .extracting(TestDescriptor::getUniqueId) // + .containsExactly(inheritedMethodUniqueId); + + results = discoverTests(selectUniqueId(declaredMethodUniqueId)); + classDescriptor = getOnlyElement(results.getEngineDescriptor().getChildren()); + assertThat(classDescriptor.getChildren()) // + .extracting(TestDescriptor::getUniqueId) // + .containsExactly(declaredMethodUniqueId); + } + + @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 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(); } diff --git a/platform-tests/src/test/java/org/junit/platform/engine/discovery/MethodSelectorTests.java b/platform-tests/src/test/java/org/junit/platform/engine/discovery/MethodSelectorTests.java index 3b888e6696e2..a8a190464ed9 100644 --- a/platform-tests/src/test/java/org/junit/platform/engine/discovery/MethodSelectorTests.java +++ b/platform-tests/src/test/java/org/junit/platform/engine/discovery/MethodSelectorTests.java @@ -82,7 +82,25 @@ void usesClassClassLoader() { assertThat(selector.getClassLoader()).isNotNull().isSameAs(getClass().getClassLoader()); } - private static class TestCase { + @Test + void distinguishesDeclaringClass() throws Exception { + var parentMethodSelector = new MethodSelector(TestCase.class, + ParentTestCase.class.getDeclaredMethod("method", int.class, boolean.class)); + var childMethodSelector = new MethodSelector(TestCase.class, + TestCase.class.getDeclaredMethod("method", int.class, boolean.class)); + + assertThat(parentMethodSelector).isNotEqualTo(childMethodSelector); + assertThat(childMethodSelector).isNotEqualTo(parentMethodSelector); + } + + private static class ParentTestCase { + + @SuppressWarnings("unused") + private void method(int num, boolean flag) { + } + } + + private static class TestCase extends ParentTestCase { @SuppressWarnings("unused") void method(int num, boolean flag) { From 7b67a8c9ffc348f39cfbd2e3ec8b8fc79c789e4d Mon Sep 17 00:00:00 2001 From: Marc Philipp Date: Tue, 28 Oct 2025 17:03:08 +0100 Subject: [PATCH 2/8] Avoid depending on mutable fields in `MethodSelector.equals()` --- .../engine/discovery/MethodSelector.java | 58 +++++++++---------- 1 file changed, 26 insertions(+), 32 deletions(-) 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 ddb74efafbb2..f31075b118ae 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 @@ -64,6 +64,7 @@ public final class MethodSelector implements DiscoverySelector { private final String className; private final String methodName; private final String parameterTypeNames; + private final @Nullable String declaringClassName; private volatile @Nullable Class javaClass; @@ -75,51 +76,51 @@ public final class MethodSelector implements DiscoverySelector { * @since 1.10 */ MethodSelector(@Nullable ClassLoader classLoader, String className, String methodName, String parameterTypeNames) { - this.classLoader = classLoader; - this.className = className; - this.methodName = methodName; - this.parameterTypeNames = parameterTypeNames; + this(classLoader, className, methodName, parameterTypeNames, null); } MethodSelector(Class javaClass, String methodName, String parameterTypeNames) { - this.classLoader = javaClass.getClassLoader(); + this(javaClass.getClassLoader(), javaClass.getName(), methodName, parameterTypeNames, null); 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), null); 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), null); 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), method.getDeclaringClass().getName()); 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; + } + + private MethodSelector(@Nullable ClassLoader classLoader, String className, String methodName, + String parameterTypeNames, @Nullable String declaringClassName) { + this.classLoader = classLoader; + this.className = className; + this.methodName = methodName; + this.parameterTypeNames = parameterTypeNames; + this.declaringClassName = declaringClassName; } /** @@ -275,17 +276,10 @@ public boolean equals(Object o) { return false; } MethodSelector that = (MethodSelector) o; - var equal = Objects.equals(this.className, that.className)// + return Objects.equals(this.className, that.className)// && Objects.equals(this.methodName, that.methodName)// - && Objects.equals(this.parameterTypeNames, that.parameterTypeNames); - if (equal) { - var thisJavaMethod = this.javaMethod; - var thatJavaMethod = that.javaMethod; - if (thisJavaMethod != null && thatJavaMethod != null) { - return thisJavaMethod.equals(thatJavaMethod); - } - } - return equal; + && Objects.equals(this.parameterTypeNames, that.parameterTypeNames)// + && Objects.equals(this.declaringClassName, that.declaringClassName); } /** @@ -294,7 +288,7 @@ public boolean equals(Object o) { @API(status = STABLE, since = "1.3") @Override public int hashCode() { - return Objects.hash(this.className, this.methodName, this.parameterTypeNames); + return Objects.hash(this.className, this.methodName, this.parameterTypeNames, this.declaringClassName); } @Override From aa4a84aa4a64d66f97fdbc1225da6b31cbd9d83c Mon Sep 17 00:00:00 2001 From: Marc Philipp Date: Tue, 28 Oct 2025 17:14:35 +0100 Subject: [PATCH 3/8] Put segment formatting and parsing into `MethodSegmentResolver` --- ...Finder.java => MethodSegmentResolver.java} | 24 ++++++++++++++++--- .../discovery/MethodSelectorResolver.java | 18 ++++---------- 2 files changed, 25 insertions(+), 17 deletions(-) rename junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/{MethodFinder.java => MethodSegmentResolver.java} (62%) 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/MethodSegmentResolver.java similarity index 62% rename from junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/MethodFinder.java rename to junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/MethodSegmentResolver.java index fe29a9bc5639..e9492910ec9c 100644 --- 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/MethodSegmentResolver.java @@ -10,6 +10,8 @@ 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; @@ -17,25 +19,41 @@ 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 MethodFinder { +class MethodSegmentResolver { // Pattern: [declaringClassName#]methodName(comma-separated list of parameter type names) private static final Pattern METHOD_PATTERN = Pattern.compile( "(?:(?.+)#)?(?.+)\\((?.*)\\)"); - Optional findMethod(String methodSpecPart, Class clazz) { + /** + * 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 = clazz; + Class targetClass = testClass; String declaringClass = matcher.group("declaringClass"); if (declaringClass != null) { targetClass = ReflectionUtils.tryToLoadClass(declaringClass).getNonNullOrThrow( 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 2f4af4665239..eb0e0850a8f9 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 @@ -13,7 +13,6 @@ import static java.util.Collections.emptyList; import static java.util.Collections.emptySet; import static java.util.stream.Collectors.toSet; -import static org.junit.platform.commons.util.ReflectionUtils.isPackagePrivate; import static org.junit.platform.engine.discovery.DiscoverySelectors.selectUniqueId; import static org.junit.platform.engine.support.discovery.SelectorResolver.Resolution.matches; import static org.junit.platform.engine.support.discovery.SelectorResolver.Resolution.unresolved; @@ -40,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; @@ -60,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; @@ -210,7 +208,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 @@ -230,16 +228,8 @@ private TestDescriptor createTestDescriptor(TestDescriptor parent, Class test } private UniqueId createUniqueId(Method method, TestDescriptor parent, Class testClass) { - return parent.getUniqueId().append(segmentType, computeMethodId(method, testClass)); - } - - private static String computeMethodId(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); + return parent.getUniqueId().append(segmentType, + methodSegmentResolver.formatMethodSpecPart(method, testClass)); } interface TestDescriptorFactory { From 31128d6a991afad7a30d62b803ad0344429c0f4c Mon Sep 17 00:00:00 2001 From: "M.P. Korstanje" Date: Wed, 29 Oct 2025 02:30:11 +0100 Subject: [PATCH 4/8] Ignore declaring class when equal to class name --- .../junit/platform/engine/discovery/MethodSelector.java | 2 +- .../platform/engine/discovery/MethodSelectorTests.java | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) 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 f31075b118ae..11b859a64dc0 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 @@ -120,7 +120,7 @@ private MethodSelector(@Nullable ClassLoader classLoader, String className, Stri this.className = className; this.methodName = methodName; this.parameterTypeNames = parameterTypeNames; - this.declaringClassName = declaringClassName; + this.declaringClassName = className.equals(declaringClassName) ? null : declaringClassName; } /** diff --git a/platform-tests/src/test/java/org/junit/platform/engine/discovery/MethodSelectorTests.java b/platform-tests/src/test/java/org/junit/platform/engine/discovery/MethodSelectorTests.java index a8a190464ed9..7f5514562396 100644 --- a/platform-tests/src/test/java/org/junit/platform/engine/discovery/MethodSelectorTests.java +++ b/platform-tests/src/test/java/org/junit/platform/engine/discovery/MethodSelectorTests.java @@ -93,6 +93,14 @@ void distinguishesDeclaringClass() throws Exception { assertThat(childMethodSelector).isNotEqualTo(parentMethodSelector); } + @Test + void ignoresDeclaringClassWhenEqualToClassname() throws Exception { + var selectorByMethod = new MethodSelector(TestCase.class, + TestCase.class.getDeclaredMethod("method", int.class, boolean.class)); + var selectorByName = new MethodSelector(TestCase.class, "method", int.class, boolean.class); + assertThat(selectorByMethod).isEqualTo(selectorByName); + } + private static class ParentTestCase { @SuppressWarnings("unused") From c6999dc652bfca4ae07b5954587ca9df516e2caf Mon Sep 17 00:00:00 2001 From: Marc Philipp Date: Wed, 29 Oct 2025 08:41:29 +0100 Subject: [PATCH 5/8] Remove `declaringClassName` from `MethodSelector` --- .../engine/discovery/MethodSelector.java | 28 +++++++------------ .../engine/discovery/MethodSelectorTests.java | 28 +------------------ 2 files changed, 11 insertions(+), 45 deletions(-) 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 11b859a64dc0..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 @@ -64,7 +64,6 @@ public final class MethodSelector implements DiscoverySelector { private final String className; private final String methodName; private final String parameterTypeNames; - private final @Nullable String declaringClassName; private volatile @Nullable Class javaClass; @@ -76,11 +75,14 @@ public final class MethodSelector implements DiscoverySelector { * @since 1.10 */ MethodSelector(@Nullable ClassLoader classLoader, String className, String methodName, String parameterTypeNames) { - this(classLoader, className, methodName, parameterTypeNames, null); + this.classLoader = classLoader; + this.className = className; + this.methodName = methodName; + this.parameterTypeNames = parameterTypeNames; } MethodSelector(Class javaClass, String methodName, String parameterTypeNames) { - this(javaClass.getClassLoader(), javaClass.getName(), methodName, parameterTypeNames, null); + this(javaClass.getClassLoader(), javaClass.getName(), methodName, parameterTypeNames); this.javaClass = javaClass; } @@ -88,7 +90,7 @@ public final class MethodSelector implements DiscoverySelector { * @since 1.10 */ MethodSelector(@Nullable ClassLoader classLoader, String className, String methodName, Class... parameterTypes) { - this(classLoader, className, methodName, ClassUtils.nullSafeToString(Class::getTypeName, parameterTypes), null); + this(classLoader, className, methodName, ClassUtils.nullSafeToString(Class::getTypeName, parameterTypes)); this.parameterTypes = parameterTypes.clone(); } @@ -97,7 +99,7 @@ public final class MethodSelector implements DiscoverySelector { */ MethodSelector(Class javaClass, String methodName, Class... parameterTypes) { this(javaClass.getClassLoader(), javaClass.getName(), methodName, - ClassUtils.nullSafeToString(Class::getTypeName, parameterTypes), null); + ClassUtils.nullSafeToString(Class::getTypeName, parameterTypes)); this.javaClass = javaClass; this.parameterTypes = parameterTypes.clone(); } @@ -108,21 +110,12 @@ public final class MethodSelector implements DiscoverySelector { private MethodSelector(Class javaClass, Method method, Class... parameterTypes) { this(javaClass.getClassLoader(), javaClass.getName(), method.getName(), - ClassUtils.nullSafeToString(Class::getTypeName, parameterTypes), method.getDeclaringClass().getName()); + ClassUtils.nullSafeToString(Class::getTypeName, parameterTypes)); this.javaClass = javaClass; this.javaMethod = method; this.parameterTypes = parameterTypes; } - private MethodSelector(@Nullable ClassLoader classLoader, String className, String methodName, - String parameterTypeNames, @Nullable String declaringClassName) { - this.classLoader = classLoader; - this.className = className; - this.methodName = methodName; - this.parameterTypeNames = parameterTypeNames; - this.declaringClassName = className.equals(declaringClassName) ? null : declaringClassName; - } - /** * Get the {@link ClassLoader} used to load the specified class. * @@ -278,8 +271,7 @@ public boolean equals(Object o) { MethodSelector that = (MethodSelector) o; return Objects.equals(this.className, that.className)// && Objects.equals(this.methodName, that.methodName)// - && Objects.equals(this.parameterTypeNames, that.parameterTypeNames)// - && Objects.equals(this.declaringClassName, that.declaringClassName); + && Objects.equals(this.parameterTypeNames, that.parameterTypeNames); } /** @@ -288,7 +280,7 @@ public boolean equals(Object o) { @API(status = STABLE, since = "1.3") @Override public int hashCode() { - return Objects.hash(this.className, this.methodName, this.parameterTypeNames, this.declaringClassName); + return Objects.hash(this.className, this.methodName, this.parameterTypeNames); } @Override diff --git a/platform-tests/src/test/java/org/junit/platform/engine/discovery/MethodSelectorTests.java b/platform-tests/src/test/java/org/junit/platform/engine/discovery/MethodSelectorTests.java index 7f5514562396..3b888e6696e2 100644 --- a/platform-tests/src/test/java/org/junit/platform/engine/discovery/MethodSelectorTests.java +++ b/platform-tests/src/test/java/org/junit/platform/engine/discovery/MethodSelectorTests.java @@ -82,33 +82,7 @@ void usesClassClassLoader() { assertThat(selector.getClassLoader()).isNotNull().isSameAs(getClass().getClassLoader()); } - @Test - void distinguishesDeclaringClass() throws Exception { - var parentMethodSelector = new MethodSelector(TestCase.class, - ParentTestCase.class.getDeclaredMethod("method", int.class, boolean.class)); - var childMethodSelector = new MethodSelector(TestCase.class, - TestCase.class.getDeclaredMethod("method", int.class, boolean.class)); - - assertThat(parentMethodSelector).isNotEqualTo(childMethodSelector); - assertThat(childMethodSelector).isNotEqualTo(parentMethodSelector); - } - - @Test - void ignoresDeclaringClassWhenEqualToClassname() throws Exception { - var selectorByMethod = new MethodSelector(TestCase.class, - TestCase.class.getDeclaredMethod("method", int.class, boolean.class)); - var selectorByName = new MethodSelector(TestCase.class, "method", int.class, boolean.class); - assertThat(selectorByMethod).isEqualTo(selectorByName); - } - - private static class ParentTestCase { - - @SuppressWarnings("unused") - private void method(int num, boolean flag) { - } - } - - private static class TestCase extends ParentTestCase { + private static class TestCase { @SuppressWarnings("unused") void method(int num, boolean flag) { From 1928f366374607ec90e4aa1b8a5cd9ce6411692d Mon Sep 17 00:00:00 2001 From: Marc Philipp Date: Wed, 29 Oct 2025 08:55:11 +0100 Subject: [PATCH 6/8] Introduce Jupiter-specific `DeclaredMethodSelector` --- .../discovery/ClassSelectorResolver.java | 6 +--- .../discovery/DeclaredMethodSelector.java | 28 +++++++++++++++++++ .../discovery/MethodSelectorResolver.java | 14 ++++++++++ 3 files changed, 43 insertions(+), 5 deletions(-) create mode 100644 junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/DeclaredMethodSelector.java 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..50f1cb09d768 --- /dev/null +++ b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/DeclaredMethodSelector.java @@ -0,0 +1,28 @@ +/* + * 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; + +/** + * @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/MethodSelectorResolver.java b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/MethodSelectorResolver.java index eb0e0850a8f9..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 @@ -83,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) { From 8d22e784cc93f6aa4d870cc0f853631bbfcf45ba Mon Sep 17 00:00:00 2001 From: Marc Philipp Date: Wed, 29 Oct 2025 09:09:12 +0100 Subject: [PATCH 7/8] Document `DeclaredMethodSelector` --- .../engine/discovery/DeclaredMethodSelector.java | 11 +++++++++++ 1 file changed, 11 insertions(+) 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 index 50f1cb09d768..394dd3a8b81c 100644 --- 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 @@ -15,8 +15,19 @@ 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 { From cc0ce23e1cb637b2332394ecb6e4a16e2c74699d Mon Sep 17 00:00:00 2001 From: Marc Philipp Date: Wed, 29 Oct 2025 16:57:20 +0100 Subject: [PATCH 8/8] Check that `MethodSource` preserves `Method` --- .../engine/TestMethodOverridingTests.java | 30 +++++++++++++------ 1 file changed, 21 insertions(+), 9 deletions(-) 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 index b2bfb0848d12..6257a41416ff 100644 --- a/jupiter-tests/src/test/java/org/junit/jupiter/engine/TestMethodOverridingTests.java +++ b/jupiter-tests/src/test/java/org/junit/jupiter/engine/TestMethodOverridingTests.java @@ -11,9 +11,11 @@ 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; @@ -23,6 +25,7 @@ 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; /** @@ -31,12 +34,10 @@ class TestMethodOverridingTests extends AbstractJupiterTestEngineTests { @Test - void bothPackagePrivateTestMethodsAreDiscovered() { + void bothPackagePrivateTestMethodsAreDiscovered() throws Exception { var results = discoverTestsForClass(DuplicateTestMethodDueToPackagePrivateVisibilityTestCase.class); var classDescriptor = getOnlyElement(results.getEngineDescriptor().getChildren()); - assertThat(classDescriptor.getChildren()).hasSize(2); - var parentUniqueId = classDescriptor.getUniqueId(); var inheritedMethodUniqueId = parentUniqueId.append("method", "org.junit.jupiter.engine.subpackage.SuperClassWithPackagePrivateLifecycleMethodInDifferentPackageTestCase#" @@ -44,21 +45,28 @@ void bothPackagePrivateTestMethodsAreDiscovered() { 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()) // - .extracting(TestDescriptor::getUniqueId) // - .containsExactly(inheritedMethodUniqueId, declaredMethodUniqueId); + .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) // - .containsExactly(inheritedMethodUniqueId); + .extracting(TestDescriptor::getUniqueId, TestMethodOverridingTests::getSourceMethod) // + .containsExactly(tuple(inheritedMethodUniqueId, inheritedMethod)); results = discoverTests(selectUniqueId(declaredMethodUniqueId)); classDescriptor = getOnlyElement(results.getEngineDescriptor().getChildren()); assertThat(classDescriptor.getChildren()) // - .extracting(TestDescriptor::getUniqueId) // - .containsExactly(declaredMethodUniqueId); + .extracting(TestDescriptor::getUniqueId, TestMethodOverridingTests::getSourceMethod) // + .containsExactly(tuple(declaredMethodUniqueId, declaredMethod)); } @Test @@ -76,6 +84,10 @@ void bothPackagePrivateTestMethodsAreExecuted() throws Exception { 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)) //