Skip to content

Commit b2c6c30

Browse files
committed
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.
1 parent c99fba6 commit b2c6c30

File tree

8 files changed

+162
-14
lines changed

8 files changed

+162
-14
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
to avoid conflicts with other control characters.
4646
* Fix `IllegalAccessError` thrown when using the Kotlin-specific `assertDoesNotThrow`
4747
assertion.
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/MethodFinder.java

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,26 +15,36 @@
1515
import java.util.regex.Matcher;
1616
import java.util.regex.Pattern;
1717

18+
import org.junit.platform.commons.PreconditionViolationException;
1819
import org.junit.platform.commons.support.ReflectionSupport;
1920
import org.junit.platform.commons.util.Preconditions;
21+
import org.junit.platform.commons.util.ReflectionUtils;
2022

2123
/**
2224
* @since 5.0
2325
*/
2426
class MethodFinder {
2527

26-
// Pattern: methodName(comma-separated list of parameter type names)
27-
private static final Pattern METHOD_PATTERN = Pattern.compile("(.+)\\((.*)\\)");
28+
// Pattern: [declaringClassName#]methodName(comma-separated list of parameter type names)
29+
private static final Pattern METHOD_PATTERN = Pattern.compile(
30+
"(?:(?<declaringClass>.+)#)?(?<method>.+)\\((?<parameters>.*)\\)");
2831

2932
Optional<Method> findMethod(String methodSpecPart, Class<?> clazz) {
3033
Matcher matcher = METHOD_PATTERN.matcher(methodSpecPart);
3134

3235
Preconditions.condition(matcher.matches(),
3336
() -> "Method [%s] does not match pattern [%s]".formatted(methodSpecPart, METHOD_PATTERN));
3437

35-
String methodName = matcher.group(1);
36-
String parameterTypeNames = matcher.group(2);
37-
return ReflectionSupport.findMethod(clazz, methodName, parameterTypeNames);
38+
Class<?> targetClass = clazz;
39+
String declaringClass = matcher.group("declaringClass");
40+
if (declaringClass != null) {
41+
targetClass = ReflectionUtils.tryToLoadClass(declaringClass).getNonNullOrThrow(
42+
cause -> new PreconditionViolationException(
43+
"Could not load declaring class with name: " + declaringClass, cause));
44+
}
45+
String methodName = matcher.group("method");
46+
String parameterTypeNames = matcher.group("parameters");
47+
return ReflectionSupport.findMethod(targetClass, methodName, parameterTypeNames);
3848
}
3949

4050
}

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

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import static java.util.Collections.emptyList;
1414
import static java.util.Collections.emptySet;
1515
import static java.util.stream.Collectors.toSet;
16+
import static org.junit.platform.commons.util.ReflectionUtils.isPackagePrivate;
1617
import static org.junit.platform.engine.discovery.DiscoverySelectors.selectUniqueId;
1718
import static org.junit.platform.engine.support.discovery.SelectorResolver.Resolution.matches;
1819
import static org.junit.platform.engine.support.discovery.SelectorResolver.Resolution.unresolved;
@@ -223,15 +224,22 @@ Optional<TestDescriptor> resolveUniqueIdIntoTestDescriptor(UniqueId uniqueId, Co
223224

224225
private TestDescriptor createTestDescriptor(TestDescriptor parent, Class<?> testClass, Method method,
225226
JupiterConfiguration configuration) {
226-
UniqueId uniqueId = createUniqueId(method, parent);
227+
UniqueId uniqueId = createUniqueId(method, parent, testClass);
227228
return testDescriptorFactory.create(uniqueId, testClass, method,
228229
((TestClassAware) parent)::getEnclosingTestClasses, configuration);
229230
}
230231

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);
232+
private UniqueId createUniqueId(Method method, TestDescriptor parent, Class<?> testClass) {
233+
return parent.getUniqueId().append(segmentType, computeMethodId(method, testClass));
234+
}
235+
236+
private static String computeMethodId(Method method, Class<?> testClass) {
237+
var parameterTypes = ClassUtils.nullSafeToString(method.getParameterTypes());
238+
if (isPackagePrivate(method)
239+
&& !method.getDeclaringClass().getPackageName().equals(testClass.getPackageName())) {
240+
return "%s#%s(%s)".formatted(method.getDeclaringClass().getName(), method.getName(), parameterTypes);
241+
}
242+
return "%s(%s)".formatted(method.getName(), parameterTypes);
235243
}
236244

237245
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
@@ -1872,7 +1872,11 @@ private static boolean isMethodOverriddenBy(Method upper, Method lower) {
18721872
return hasCompatibleSignature(upper, lower.getName(), lower.getParameterTypes());
18731873
}
18741874

1875-
private static boolean isPackagePrivate(Member member) {
1875+
/**
1876+
* @since 6.0.1
1877+
*/
1878+
@API(status = INTERNAL, since = "6.0.1")
1879+
public static boolean isPackagePrivate(Member member) {
18761880
int modifiers = member.getModifiers();
18771881
return !(Modifier.isPublic(modifiers) || Modifier.isProtected(modifiers) || Modifier.isPrivate(modifiers));
18781882
}

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

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -275,9 +275,17 @@ public boolean equals(Object o) {
275275
return false;
276276
}
277277
MethodSelector that = (MethodSelector) o;
278-
return Objects.equals(this.className, that.className)//
278+
var equal = Objects.equals(this.className, that.className)//
279279
&& Objects.equals(this.methodName, that.methodName)//
280280
&& Objects.equals(this.parameterTypeNames, that.parameterTypeNames);
281+
if (equal) {
282+
var thisJavaMethod = this.javaMethod;
283+
var thatJavaMethod = that.javaMethod;
284+
if (thisJavaMethod != null && thatJavaMethod != null) {
285+
return thisJavaMethod.equals(thatJavaMethod);
286+
}
287+
}
288+
return equal;
281289
}
282290

283291
/**
Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
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;
12+
13+
import static org.assertj.core.api.Assertions.assertThat;
14+
import static org.junit.platform.commons.util.CollectionUtils.getOnlyElement;
15+
import static org.junit.platform.engine.discovery.DiscoverySelectors.selectUniqueId;
16+
17+
import java.util.Map;
18+
import java.util.stream.Stream;
19+
20+
import org.junit.jupiter.api.Test;
21+
import org.junit.jupiter.api.TestInfo;
22+
import org.junit.jupiter.api.TestReporter;
23+
import org.junit.jupiter.engine.subpackage.SuperClassWithPackagePrivateLifecycleMethodInDifferentPackageTestCase;
24+
import org.junit.platform.engine.TestDescriptor;
25+
import org.junit.platform.engine.reporting.ReportEntry;
26+
import org.junit.platform.testkit.engine.EngineExecutionResults;
27+
28+
/**
29+
* @since 6.0.1
30+
*/
31+
class TestMethodOverridingTests extends AbstractJupiterTestEngineTests {
32+
33+
@Test
34+
void bothPackagePrivateTestMethodsAreDiscovered() {
35+
var results = discoverTestsForClass(DuplicateTestMethodDueToPackagePrivateVisibilityTestCase.class);
36+
var classDescriptor = getOnlyElement(results.getEngineDescriptor().getChildren());
37+
38+
assertThat(classDescriptor.getChildren()).hasSize(2);
39+
40+
var parentUniqueId = classDescriptor.getUniqueId();
41+
var inheritedMethodUniqueId = parentUniqueId.append("method",
42+
"org.junit.jupiter.engine.subpackage.SuperClassWithPackagePrivateLifecycleMethodInDifferentPackageTestCase#"
43+
+ "test(org.junit.jupiter.api.TestInfo, org.junit.jupiter.api.TestReporter)");
44+
var declaredMethodUniqueId = parentUniqueId.append("method",
45+
"test(org.junit.jupiter.api.TestInfo, org.junit.jupiter.api.TestReporter)");
46+
47+
assertThat(classDescriptor.getChildren()) //
48+
.extracting(TestDescriptor::getUniqueId) //
49+
.containsExactly(inheritedMethodUniqueId, declaredMethodUniqueId);
50+
51+
results = discoverTests(selectUniqueId(inheritedMethodUniqueId));
52+
classDescriptor = getOnlyElement(results.getEngineDescriptor().getChildren());
53+
assertThat(classDescriptor.getChildren()) //
54+
.extracting(TestDescriptor::getUniqueId) //
55+
.containsExactly(inheritedMethodUniqueId);
56+
57+
results = discoverTests(selectUniqueId(declaredMethodUniqueId));
58+
classDescriptor = getOnlyElement(results.getEngineDescriptor().getChildren());
59+
assertThat(classDescriptor.getChildren()) //
60+
.extracting(TestDescriptor::getUniqueId) //
61+
.containsExactly(declaredMethodUniqueId);
62+
}
63+
64+
@Test
65+
void bothPackagePrivateTestMethodsAreExecuted() throws Exception {
66+
var results = executeTestsForClass(DuplicateTestMethodDueToPackagePrivateVisibilityTestCase.class);
67+
68+
results.testEvents().assertStatistics(stats -> stats.started(2).succeeded(2));
69+
assertThat(allReportEntries(results)) //
70+
.containsExactly(
71+
Map.of("invokedSuper",
72+
SuperClassWithPackagePrivateLifecycleMethodInDifferentPackageTestCase.class.getDeclaredMethod(
73+
"test", TestInfo.class, TestReporter.class).toGenericString()),
74+
Map.of("invokedChild",
75+
DuplicateTestMethodDueToPackagePrivateVisibilityTestCase.class.getDeclaredMethod("test",
76+
TestInfo.class, TestReporter.class).toGenericString()));
77+
}
78+
79+
private static Stream<Map<String, String>> allReportEntries(EngineExecutionResults results) {
80+
return results.allEvents().reportingEntryPublished() //
81+
.map(event -> event.getRequiredPayload(ReportEntry.class)) //
82+
.map(ReportEntry::getKeyValuePairs);
83+
}
84+
85+
@SuppressWarnings("JUnitMalformedDeclaration")
86+
static class DuplicateTestMethodDueToPackagePrivateVisibilityTestCase
87+
extends SuperClassWithPackagePrivateLifecycleMethodInDifferentPackageTestCase {
88+
89+
// @Override
90+
@Test
91+
void test(TestInfo testInfo, TestReporter reporter) {
92+
reporter.publishEntry("invokedChild", testInfo.getTestMethod().orElseThrow().toGenericString());
93+
}
94+
}
95+
}

jupiter-tests/src/test/java/org/junit/jupiter/engine/subpackage/SuperClassWithPackagePrivateLifecycleMethodInDifferentPackageTestCase.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414

1515
import org.junit.jupiter.api.BeforeEach;
1616
import org.junit.jupiter.api.Test;
17+
import org.junit.jupiter.api.TestInfo;
18+
import org.junit.jupiter.api.TestReporter;
1719

1820
/**
1921
* @since 5.9
@@ -28,7 +30,8 @@ void beforeEach() {
2830
}
2931

3032
@Test
31-
void test() {
33+
void test(TestInfo testInfo, TestReporter reporter) {
34+
reporter.publishEntry("invokedSuper", testInfo.getTestMethod().orElseThrow().toGenericString());
3235
assertThat(this.beforeEachInvoked).isTrue();
3336
}
3437

platform-tests/src/test/java/org/junit/platform/engine/discovery/MethodSelectorTests.java

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,25 @@ void usesClassClassLoader() {
8282
assertThat(selector.getClassLoader()).isNotNull().isSameAs(getClass().getClassLoader());
8383
}
8484

85-
private static class TestCase {
85+
@Test
86+
void distinguishesDeclaringClass() throws Exception {
87+
var parentMethodSelector = new MethodSelector(TestCase.class,
88+
ParentTestCase.class.getDeclaredMethod("method", int.class, boolean.class));
89+
var childMethodSelector = new MethodSelector(TestCase.class,
90+
TestCase.class.getDeclaredMethod("method", int.class, boolean.class));
91+
92+
assertThat(parentMethodSelector).isNotEqualTo(childMethodSelector);
93+
assertThat(childMethodSelector).isNotEqualTo(parentMethodSelector);
94+
}
95+
96+
private static class ParentTestCase {
97+
98+
@SuppressWarnings("unused")
99+
private void method(int num, boolean flag) {
100+
}
101+
}
102+
103+
private static class TestCase extends ParentTestCase {
86104

87105
@SuppressWarnings("unused")
88106
void method(int num, boolean flag) {

0 commit comments

Comments
 (0)