Skip to content

Commit 13adf9d

Browse files
committed
Fix support for package-private test methods (#5100)
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.
1 parent cdbdf42 commit 13adf9d

File tree

10 files changed

+257
-72
lines changed

10 files changed

+257
-72
lines changed

documentation/src/docs/asciidoc/release-notes/release-notes-6.0.1.adoc

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@ repository on GitHub.
4545
assertion.
4646
* Stop reporting discovery issues for synthetic methods, particularly in conjunction with
4747
Kotlin suspend functions.
48+
* Fix support for test methods with the same signature as a package-private methods
49+
declared in super classes in different packages.
4850

4951
[[release-notes-6.0.1-junit-jupiter-deprecations-and-breaking-changes]]
5052
==== Deprecations and Breaking Changes

junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/ClassSelectorResolver.java

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -323,11 +323,7 @@ private DiscoverySelector selectClass(List<Class<?>> classes) {
323323
}
324324

325325
private DiscoverySelector selectMethod(List<Class<?>> classes, Method method) {
326-
if (classes.size() == 1) {
327-
return DiscoverySelectors.selectMethod(classes.get(0), method);
328-
}
329-
int lastIndex = classes.size() - 1;
330-
return DiscoverySelectors.selectNestedMethod(classes.subList(0, lastIndex), classes.get(lastIndex), method);
326+
return new DeclaredMethodSelector(classes, method);
331327
}
332328

333329
static class DummyClassTemplateInvocationContext implements ClassTemplateInvocationContext {
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
/*
2+
* Copyright 2015-2025 the original author or authors.
3+
*
4+
* All rights reserved. This program and the accompanying materials are
5+
* made available under the terms of the Eclipse Public License v2.0 which
6+
* accompanies this distribution and is available at
7+
*
8+
* https://www.eclipse.org/legal/epl-v20.html
9+
*/
10+
11+
package org.junit.jupiter.engine.discovery;
12+
13+
import java.lang.reflect.Method;
14+
import java.util.List;
15+
16+
import org.junit.platform.commons.util.Preconditions;
17+
import org.junit.platform.engine.DiscoverySelector;
18+
import org.junit.platform.engine.discovery.MethodSelector;
19+
20+
/**
21+
* Jupiter-specific selector for methods, potentially in nested classes.
22+
*
23+
* <p>The important difference to {@link MethodSelector} is that this selector's
24+
* {@link #equals(Object)} method takes into account the selected method's
25+
* {@linkplain Method#getDeclaringClass() declaring class} to support cases
26+
* where a package-private method is declared in a super class in a different
27+
* package and a method with the same signature is declared in a subclass. In
28+
* that case both methods should be discovered because the one declared in the
29+
* subclass does <em>not</em> override the one in the super class.
30+
*
31+
* @since 6.0.1
32+
*/
33+
record DeclaredMethodSelector(List<Class<?>> testClasses, Method method) implements DiscoverySelector {
34+
DeclaredMethodSelector {
35+
Preconditions.notEmpty(testClasses, "testClasses must not be empty");
36+
Preconditions.containsNoNullElements(testClasses, "testClasses must not contain null elements");
37+
Preconditions.notNull(method, "method must not be null");
38+
}
39+
}

junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/MethodFinder.java

Lines changed: 0 additions & 40 deletions
This file was deleted.
Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
/*
2+
* Copyright 2015-2025 the original author or authors.
3+
*
4+
* All rights reserved. This program and the accompanying materials are
5+
* made available under the terms of the Eclipse Public License v2.0 which
6+
* accompanies this distribution and is available at
7+
*
8+
* https://www.eclipse.org/legal/epl-v20.html
9+
*/
10+
11+
package org.junit.jupiter.engine.discovery;
12+
13+
import static org.junit.platform.commons.util.ReflectionUtils.isPackagePrivate;
14+
15+
import java.lang.reflect.Method;
16+
import java.util.Optional;
17+
import java.util.regex.Matcher;
18+
import java.util.regex.Pattern;
19+
20+
import org.junit.platform.commons.PreconditionViolationException;
21+
import org.junit.platform.commons.support.ReflectionSupport;
22+
import org.junit.platform.commons.util.ClassUtils;
23+
import org.junit.platform.commons.util.Preconditions;
24+
import org.junit.platform.commons.util.ReflectionUtils;
25+
26+
/**
27+
* @since 5.0
28+
*/
29+
class MethodSegmentResolver {
30+
31+
// Pattern: [declaringClassName#]methodName(comma-separated list of parameter type names)
32+
private static final Pattern METHOD_PATTERN = Pattern.compile(
33+
"(?:(?<declaringClass>.+)#)?(?<method>.+)\\((?<parameters>.*)\\)");
34+
35+
/**
36+
* If the {@code method} is package-private and declared a class in a
37+
* different package than {@code testClass}, the declaring class name is
38+
* included in the method's unique ID segment. Otherwise, it only
39+
* consists of the method name and its parameter types.
40+
*/
41+
String formatMethodSpecPart(Method method, Class<?> testClass) {
42+
var parameterTypes = ClassUtils.nullSafeToString(method.getParameterTypes());
43+
if (isPackagePrivate(method)
44+
&& !method.getDeclaringClass().getPackageName().equals(testClass.getPackageName())) {
45+
return "%s#%s(%s)".formatted(method.getDeclaringClass().getName(), method.getName(), parameterTypes);
46+
}
47+
return "%s(%s)".formatted(method.getName(), parameterTypes);
48+
}
49+
50+
Optional<Method> findMethod(String methodSpecPart, Class<?> testClass) {
51+
Matcher matcher = METHOD_PATTERN.matcher(methodSpecPart);
52+
53+
Preconditions.condition(matcher.matches(),
54+
() -> "Method [%s] does not match pattern [%s]".formatted(methodSpecPart, METHOD_PATTERN));
55+
56+
Class<?> targetClass = testClass;
57+
String declaringClass = matcher.group("declaringClass");
58+
if (declaringClass != null) {
59+
targetClass = ReflectionUtils.tryToLoadClass(declaringClass).getNonNullOrThrow(
60+
cause -> new PreconditionViolationException(
61+
"Could not load declaring class with name: " + declaringClass, cause));
62+
}
63+
String methodName = matcher.group("method");
64+
String parameterTypeNames = matcher.group("parameters");
65+
return ReflectionSupport.findMethod(targetClass, methodName, parameterTypeNames);
66+
}
67+
68+
}

junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/MethodSelectorResolver.java

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@
3939
import org.junit.jupiter.engine.discovery.predicates.IsTestMethod;
4040
import org.junit.jupiter.engine.discovery.predicates.IsTestTemplateMethod;
4141
import org.junit.jupiter.engine.discovery.predicates.TestClassPredicates;
42-
import org.junit.platform.commons.util.ClassUtils;
4342
import org.junit.platform.engine.DiscoveryIssue;
4443
import org.junit.platform.engine.DiscoveryIssue.Severity;
4544
import org.junit.platform.engine.DiscoverySelector;
@@ -59,7 +58,7 @@
5958
*/
6059
class MethodSelectorResolver implements SelectorResolver {
6160

62-
private static final MethodFinder methodFinder = new MethodFinder();
61+
private static final MethodSegmentResolver methodSegmentResolver = new MethodSegmentResolver();
6362
private final Predicate<Class<?>> testClassPredicate;
6463

6564
private final JupiterConfiguration configuration;
@@ -84,6 +83,20 @@ public Resolution resolve(NestedMethodSelector selector, Context context) {
8483
Match::exact);
8584
}
8685

86+
@Override
87+
public Resolution resolve(DiscoverySelector selector, Context context) {
88+
if (selector instanceof DeclaredMethodSelector methodSelector) {
89+
var testClasses = methodSelector.testClasses();
90+
if (testClasses.size() == 1) {
91+
return resolve(context, emptyList(), testClasses.get(0), methodSelector::method, Match::exact);
92+
}
93+
int lastIndex = testClasses.size() - 1;
94+
return resolve(context, testClasses.subList(0, lastIndex), testClasses.get(lastIndex),
95+
methodSelector::method, Match::exact);
96+
}
97+
return unresolved();
98+
}
99+
87100
private Resolution resolve(Context context, List<Class<?>> enclosingClasses, Class<?> testClass,
88101
Supplier<Method> methodSupplier,
89102
BiFunction<TestDescriptor, Supplier<Set<? extends DiscoverySelector>>, Match> matchFactory) {
@@ -209,7 +222,7 @@ Optional<TestDescriptor> resolveUniqueIdIntoTestDescriptor(UniqueId uniqueId, Co
209222
String methodSpecPart = lastSegment.getValue();
210223
Class<?> testClass = ((TestClassAware) parent).getTestClass();
211224
// @formatter:off
212-
return methodFinder.findMethod(methodSpecPart, testClass)
225+
return methodSegmentResolver.findMethod(methodSpecPart, testClass)
213226
.filter(methodPredicate)
214227
.map(method -> createTestDescriptor(parent, testClass, method, configuration));
215228
// @formatter:on
@@ -223,15 +236,14 @@ Optional<TestDescriptor> resolveUniqueIdIntoTestDescriptor(UniqueId uniqueId, Co
223236

224237
private TestDescriptor createTestDescriptor(TestDescriptor parent, Class<?> testClass, Method method,
225238
JupiterConfiguration configuration) {
226-
UniqueId uniqueId = createUniqueId(method, parent);
239+
UniqueId uniqueId = createUniqueId(method, parent, testClass);
227240
return testDescriptorFactory.create(uniqueId, testClass, method,
228241
((TestClassAware) parent)::getEnclosingTestClasses, configuration);
229242
}
230243

231-
private UniqueId createUniqueId(Method method, TestDescriptor parent) {
232-
String methodId = "%s(%s)".formatted(method.getName(),
233-
ClassUtils.nullSafeToString(method.getParameterTypes()));
234-
return parent.getUniqueId().append(segmentType, methodId);
244+
private UniqueId createUniqueId(Method method, TestDescriptor parent, Class<?> testClass) {
245+
return parent.getUniqueId().append(segmentType,
246+
methodSegmentResolver.formatMethodSpecPart(method, testClass));
235247
}
236248

237249
interface TestDescriptorFactory {

junit-platform-commons/src/main/java/org/junit/platform/commons/util/ReflectionUtils.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1848,7 +1848,11 @@ private static boolean isMethodOverriddenBy(Method upper, Method lower) {
18481848
return hasCompatibleSignature(upper, lower.getName(), lower.getParameterTypes());
18491849
}
18501850

1851-
private static boolean isPackagePrivate(Member member) {
1851+
/**
1852+
* @since 6.0.1
1853+
*/
1854+
@API(status = INTERNAL, since = "6.0.1")
1855+
public static boolean isPackagePrivate(Member member) {
18521856
int modifiers = member.getModifiers();
18531857
return !(Modifier.isPublic(modifiers) || Modifier.isProtected(modifiers) || Modifier.isPrivate(modifiers));
18541858
}

junit-platform-engine/src/main/java/org/junit/platform/engine/discovery/MethodSelector.java

Lines changed: 11 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -82,44 +82,38 @@ public final class MethodSelector implements DiscoverySelector {
8282
}
8383

8484
MethodSelector(Class<?> javaClass, String methodName, String parameterTypeNames) {
85-
this.classLoader = javaClass.getClassLoader();
85+
this(javaClass.getClassLoader(), javaClass.getName(), methodName, parameterTypeNames);
8686
this.javaClass = javaClass;
87-
this.className = javaClass.getName();
88-
this.methodName = methodName;
89-
this.parameterTypeNames = parameterTypeNames;
9087
}
9188

9289
/**
9390
* @since 1.10
9491
*/
9592
MethodSelector(@Nullable ClassLoader classLoader, String className, String methodName, Class<?>... parameterTypes) {
96-
this.classLoader = classLoader;
97-
this.className = className;
98-
this.methodName = methodName;
93+
this(classLoader, className, methodName, ClassUtils.nullSafeToString(Class::getTypeName, parameterTypes));
9994
this.parameterTypes = parameterTypes.clone();
100-
this.parameterTypeNames = ClassUtils.nullSafeToString(Class::getTypeName, this.parameterTypes);
10195
}
10296

10397
/**
10498
* @since 1.10
10599
*/
106100
MethodSelector(Class<?> javaClass, String methodName, Class<?>... parameterTypes) {
107-
this.classLoader = javaClass.getClassLoader();
101+
this(javaClass.getClassLoader(), javaClass.getName(), methodName,
102+
ClassUtils.nullSafeToString(Class::getTypeName, parameterTypes));
108103
this.javaClass = javaClass;
109-
this.className = javaClass.getName();
110-
this.methodName = methodName;
111104
this.parameterTypes = parameterTypes.clone();
112-
this.parameterTypeNames = ClassUtils.nullSafeToString(Class::getTypeName, this.parameterTypes);
113105
}
114106

115107
MethodSelector(Class<?> javaClass, Method method) {
116-
this.classLoader = javaClass.getClassLoader();
108+
this(javaClass, method, method.getParameterTypes());
109+
}
110+
111+
private MethodSelector(Class<?> javaClass, Method method, Class<?>... parameterTypes) {
112+
this(javaClass.getClassLoader(), javaClass.getName(), method.getName(),
113+
ClassUtils.nullSafeToString(Class::getTypeName, parameterTypes));
117114
this.javaClass = javaClass;
118-
this.className = javaClass.getName();
119115
this.javaMethod = method;
120-
this.methodName = method.getName();
121-
this.parameterTypes = method.getParameterTypes();
122-
this.parameterTypeNames = ClassUtils.nullSafeToString(Class::getTypeName, this.parameterTypes);
116+
this.parameterTypes = parameterTypes;
123117
}
124118

125119
/**

0 commit comments

Comments
 (0)