Skip to content

Commit 0b9617e

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 d43c3ca commit 0b9617e

File tree

10 files changed

+298
-76
lines changed

10 files changed

+298
-76
lines changed

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,8 @@ repository on GitHub.
3535
[[release-notes-5.14.1-junit-jupiter-bug-fixes]]
3636
==== Bug Fixes
3737

38-
* ❓
38+
* Fix support for test methods with the same signature as a package-private methods
39+
declared in super classes in different packages.
3940

4041
[[release-notes-5.14.1-junit-jupiter-deprecations-and-breaking-changes]]
4142
==== 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
@@ -329,11 +329,7 @@ private DiscoverySelector selectClass(List<Class<?>> classes) {
329329
}
330330

331331
private DiscoverySelector selectMethod(List<Class<?>> classes, Method method) {
332-
if (classes.size() == 1) {
333-
return DiscoverySelectors.selectMethod(classes.get(0), method);
334-
}
335-
int lastIndex = classes.size() - 1;
336-
return DiscoverySelectors.selectNestedMethod(classes.subList(0, lastIndex), classes.get(lastIndex), method);
332+
return new DeclaredMethodSelector(classes, method);
337333
}
338334

339335
static class DummyClassTemplateInvocationContext implements ClassTemplateInvocationContext {
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 java.lang.reflect.Method;
14+
import java.util.List;
15+
import java.util.Objects;
16+
17+
import org.junit.platform.commons.util.Preconditions;
18+
import org.junit.platform.engine.DiscoverySelector;
19+
import org.junit.platform.engine.discovery.MethodSelector;
20+
21+
/**
22+
* Jupiter-specific selector for methods, potentially in nested classes.
23+
*
24+
* <p>The important difference to {@link MethodSelector} is that this selector's
25+
* {@link #equals(Object)} method takes into account the selected method's
26+
* {@linkplain Method#getDeclaringClass() declaring class} to support cases
27+
* where a package-private method is declared in a super class in a different
28+
* package and a method with the same signature is declared in a subclass. In
29+
* that case both methods should be discovered because the one declared in the
30+
* subclass does <em>not</em> override the one in the super class.
31+
*
32+
* @since 6.0.1
33+
*/
34+
final class DeclaredMethodSelector implements DiscoverySelector {
35+
36+
private final List<Class<?>> testClasses;
37+
private final Method method;
38+
39+
DeclaredMethodSelector(List<Class<?>> testClasses, Method method) {
40+
Preconditions.notEmpty(testClasses, "testClasses must not be empty");
41+
this.testClasses = Preconditions.containsNoNullElements(testClasses,
42+
"testClasses must not contain null elements");
43+
this.method = Preconditions.notNull(method, "method must not be null");
44+
}
45+
46+
List<Class<?>> testClasses() {
47+
return testClasses;
48+
}
49+
50+
Method method() {
51+
return method;
52+
}
53+
54+
@Override
55+
public boolean equals(Object o) {
56+
if (!(o instanceof DeclaredMethodSelector)) {
57+
return false;
58+
}
59+
DeclaredMethodSelector that = (DeclaredMethodSelector) o;
60+
return Objects.equals(testClasses, that.testClasses) //
61+
&& Objects.equals(method, that.method);
62+
}
63+
64+
@Override
65+
public int hashCode() {
66+
return Objects.hash(testClasses, method);
67+
}
68+
}

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.isDeclaredInSamePackage;
14+
import static org.junit.platform.commons.util.ReflectionUtils.isPackagePrivate;
15+
16+
import java.lang.reflect.Method;
17+
import java.util.Optional;
18+
import java.util.regex.Matcher;
19+
import java.util.regex.Pattern;
20+
21+
import org.junit.platform.commons.PreconditionViolationException;
22+
import org.junit.platform.commons.support.ReflectionSupport;
23+
import org.junit.platform.commons.util.ClassUtils;
24+
import org.junit.platform.commons.util.Preconditions;
25+
import org.junit.platform.commons.util.ReflectionUtils;
26+
27+
/**
28+
* @since 5.0
29+
*/
30+
class MethodSegmentResolver {
31+
32+
// Pattern: [declaringClassName#]methodName(comma-separated list of parameter type names)
33+
private static final Pattern METHOD_PATTERN = Pattern.compile(
34+
"(?:(?<declaringClass>.+)#)?(?<method>.+)\\((?<parameters>.*)\\)");
35+
36+
/**
37+
* If the {@code method} is package-private and declared a class in a
38+
* different package than {@code testClass}, the declaring class name is
39+
* included in the method's unique ID segment. Otherwise, it only
40+
* consists of the method name and its parameter types.
41+
*/
42+
String formatMethodSpecPart(Method method, Class<?> testClass) {
43+
String parameterTypes = ClassUtils.nullSafeToString(method.getParameterTypes());
44+
if (isPackagePrivate(method) && !isDeclaredInSamePackage(method.getDeclaringClass(), testClass)) {
45+
return String.format("%s#%s(%s)", method.getDeclaringClass().getName(), method.getName(), parameterTypes);
46+
}
47+
return String.format("%s(%s)", 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+
() -> String.format("Method [%s] does not match pattern [%s]", methodSpecPart, METHOD_PATTERN));
55+
56+
Class<?> targetClass = testClass;
57+
String declaringClass = matcher.group("declaringClass");
58+
if (declaringClass != null) {
59+
targetClass = ReflectionUtils.tryToLoadClass(declaringClass).getOrThrow(
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: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@
4040
import org.junit.jupiter.engine.discovery.predicates.IsTestMethod;
4141
import org.junit.jupiter.engine.discovery.predicates.IsTestTemplateMethod;
4242
import org.junit.jupiter.engine.discovery.predicates.TestClassPredicates;
43-
import org.junit.platform.commons.util.ClassUtils;
4443
import org.junit.platform.engine.DiscoveryIssue;
4544
import org.junit.platform.engine.DiscoveryIssue.Severity;
4645
import org.junit.platform.engine.DiscoverySelector;
@@ -60,7 +59,7 @@
6059
*/
6160
class MethodSelectorResolver implements SelectorResolver {
6261

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

6665
private final JupiterConfiguration configuration;
@@ -85,6 +84,21 @@ public Resolution resolve(NestedMethodSelector selector, Context context) {
8584
Match::exact);
8685
}
8786

87+
@Override
88+
public Resolution resolve(DiscoverySelector selector, Context context) {
89+
if (selector instanceof DeclaredMethodSelector) {
90+
DeclaredMethodSelector methodSelector = (DeclaredMethodSelector) selector;
91+
List<Class<?>> testClasses = methodSelector.testClasses();
92+
if (testClasses.size() == 1) {
93+
return resolve(context, emptyList(), testClasses.get(0), methodSelector::method, Match::exact);
94+
}
95+
int lastIndex = testClasses.size() - 1;
96+
return resolve(context, testClasses.subList(0, lastIndex), testClasses.get(lastIndex),
97+
methodSelector::method, Match::exact);
98+
}
99+
return unresolved();
100+
}
101+
88102
private Resolution resolve(Context context, List<Class<?>> enclosingClasses, Class<?> testClass,
89103
Supplier<Method> methodSupplier,
90104
BiFunction<TestDescriptor, Supplier<Set<? extends DiscoverySelector>>, Match> matchFactory) {
@@ -216,7 +230,7 @@ Optional<TestDescriptor> resolveUniqueIdIntoTestDescriptor(UniqueId uniqueId, Co
216230
String methodSpecPart = lastSegment.getValue();
217231
Class<?> testClass = ((TestClassAware) parent).getTestClass();
218232
// @formatter:off
219-
return methodFinder.findMethod(methodSpecPart, testClass)
233+
return methodSegmentResolver.findMethod(methodSpecPart, testClass)
220234
.filter(methodPredicate)
221235
.map(method -> createTestDescriptor(parent, testClass, method, configuration));
222236
// @formatter:on
@@ -230,15 +244,14 @@ Optional<TestDescriptor> resolveUniqueIdIntoTestDescriptor(UniqueId uniqueId, Co
230244

231245
private TestDescriptor createTestDescriptor(TestDescriptor parent, Class<?> testClass, Method method,
232246
JupiterConfiguration configuration) {
233-
UniqueId uniqueId = createUniqueId(method, parent);
247+
UniqueId uniqueId = createUniqueId(method, parent, testClass);
234248
return testDescriptorFactory.create(uniqueId, testClass, method,
235249
((TestClassAware) parent)::getEnclosingTestClasses, configuration);
236250
}
237251

238-
private UniqueId createUniqueId(Method method, TestDescriptor parent) {
239-
String methodId = String.format("%s(%s)", method.getName(),
240-
ClassUtils.nullSafeToString(method.getParameterTypes()));
241-
return parent.getUniqueId().append(segmentType, methodId);
252+
private UniqueId createUniqueId(Method method, TestDescriptor parent, Class<?> testClass) {
253+
return parent.getUniqueId().append(segmentType,
254+
methodSegmentResolver.formatMethodSpecPart(method, testClass));
242255
}
243256

244257
interface TestDescriptorFactory {

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

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1997,21 +1997,33 @@ private static boolean isMethodOverriddenBy(Method upper, Method lower) {
19971997
}
19981998

19991999
// Cannot override a package-private method in another package.
2000-
if (isPackagePrivate(upper) && !declaredInSamePackage(upper, lower)) {
2000+
if (isPackagePrivate(upper) && !isDeclaredInSamePackage(upper, lower)) {
20012001
return false;
20022002
}
20032003
}
20042004

20052005
return hasCompatibleSignature(upper, lower.getName(), lower.getParameterTypes());
20062006
}
20072007

2008-
private static boolean isPackagePrivate(Member member) {
2008+
/**
2009+
* @since 6.0.1
2010+
*/
2011+
@API(status = INTERNAL, since = "6.0.1")
2012+
public static boolean isPackagePrivate(Member member) {
20092013
int modifiers = member.getModifiers();
20102014
return !(Modifier.isPublic(modifiers) || Modifier.isProtected(modifiers) || Modifier.isPrivate(modifiers));
20112015
}
20122016

2013-
private static boolean declaredInSamePackage(Method m1, Method m2) {
2014-
return getPackageName(m1.getDeclaringClass()).equals(getPackageName(m2.getDeclaringClass()));
2017+
private static boolean isDeclaredInSamePackage(Method m1, Method m2) {
2018+
return isDeclaredInSamePackage(m1.getDeclaringClass(), m2.getDeclaringClass());
2019+
}
2020+
2021+
/**
2022+
* @since 6.0.1
2023+
*/
2024+
@API(status = INTERNAL, since = "6.0.1")
2025+
public static boolean isDeclaredInSamePackage(Class<?> c1, Class<?> c2) {
2026+
return getPackageName(c1).equals(getPackageName(c2));
20152027
}
20162028

20172029
/**

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
@@ -80,44 +80,38 @@ public class MethodSelector implements DiscoverySelector {
8080
}
8181

8282
MethodSelector(Class<?> javaClass, String methodName, String parameterTypeNames) {
83-
this.classLoader = javaClass.getClassLoader();
83+
this(javaClass.getClassLoader(), javaClass.getName(), methodName, parameterTypeNames);
8484
this.javaClass = javaClass;
85-
this.className = javaClass.getName();
86-
this.methodName = methodName;
87-
this.parameterTypeNames = parameterTypeNames;
8885
}
8986

9087
/**
9188
* @since 1.10
9289
*/
9390
MethodSelector(ClassLoader classLoader, String className, String methodName, Class<?>... parameterTypes) {
94-
this.classLoader = classLoader;
95-
this.className = className;
96-
this.methodName = methodName;
91+
this(classLoader, className, methodName, ClassUtils.nullSafeToString(Class::getTypeName, parameterTypes));
9792
this.parameterTypes = parameterTypes.clone();
98-
this.parameterTypeNames = ClassUtils.nullSafeToString(Class::getTypeName, this.parameterTypes);
9993
}
10094

10195
/**
10296
* @since 1.10
10397
*/
10498
MethodSelector(Class<?> javaClass, String methodName, Class<?>... parameterTypes) {
105-
this.classLoader = javaClass.getClassLoader();
99+
this(javaClass.getClassLoader(), javaClass.getName(), methodName,
100+
ClassUtils.nullSafeToString(Class::getTypeName, parameterTypes));
106101
this.javaClass = javaClass;
107-
this.className = javaClass.getName();
108-
this.methodName = methodName;
109102
this.parameterTypes = parameterTypes.clone();
110-
this.parameterTypeNames = ClassUtils.nullSafeToString(Class::getTypeName, this.parameterTypes);
111103
}
112104

113105
MethodSelector(Class<?> javaClass, Method method) {
114-
this.classLoader = javaClass.getClassLoader();
106+
this(javaClass, method, method.getParameterTypes());
107+
}
108+
109+
private MethodSelector(Class<?> javaClass, Method method, Class<?>... parameterTypes) {
110+
this(javaClass.getClassLoader(), javaClass.getName(), method.getName(),
111+
ClassUtils.nullSafeToString(Class::getTypeName, parameterTypes));
115112
this.javaClass = javaClass;
116-
this.className = javaClass.getName();
117113
this.javaMethod = method;
118-
this.methodName = method.getName();
119-
this.parameterTypes = method.getParameterTypes();
120-
this.parameterTypeNames = ClassUtils.nullSafeToString(Class::getTypeName, this.parameterTypes);
114+
this.parameterTypes = parameterTypes;
121115
}
122116

123117
/**

0 commit comments

Comments
 (0)