Surefire test exclusion can cause NoClassDefFoundError on case-insensitive file system

[Stephan202]

Running the Checkstyle Maven build using -Dtest='!someTest' on a case-insensitive filesystem, Surefire reports TestEngine with ID 'junit-jupiter' failed to discover tests. If a user hits this issue, then the root cause will not be easy to debug, as it requires (very) careful inspection of the *.dump file inside target/surefire-reports.

Steps to reproduce

The following commands demonstrate the issue, if executed in the context of a case-insensitive filesystem:

git clone --depth 1 https://github.com/checkstyle/checkstyle.git checkstyle
cd checkstyle
mvn test -Dtest='!foo'
cat target/surefire-reports/*.dump

This will produce:

# Created at 2023-11-04T16:24:14.346
org.junit.platform.commons.JUnitException: TestEngine with ID 'junit-jupiter' failed to discover tests
	at org.junit.platform.launcher.core.EngineDiscoveryOrchestrator.discoverEngineRoot(EngineDiscoveryOrchestrator.java:160)
	at org.junit.platform.launcher.core.EngineDiscoveryOrchestrator.discoverSafely(EngineDiscoveryOrchestrator.java:132)
	at org.junit.platform.launcher.core.EngineDiscoveryOrchestrator.discover(EngineDiscoveryOrchestrator.java:107)
	at org.junit.platform.launcher.core.EngineDiscoveryOrchestrator.discover(EngineDiscoveryOrchestrator.java:78)
	at org.junit.platform.launcher.core.DefaultLauncher.discover(DefaultLauncher.java:110)
	at org.junit.platform.launcher.core.DefaultLauncher.discover(DefaultLauncher.java:78)
	at org.junit.platform.launcher.core.DefaultLauncherSession$DelegatingLauncher.discover(DefaultLauncherSession.java:81)
	at org.apache.maven.surefire.junitplatform.LazyLauncher.discover(LazyLauncher.java:50)
	at org.apache.maven.surefire.junitplatform.TestPlanScannerFilter.accept(TestPlanScannerFilter.java:52)
	at org.apache.maven.surefire.api.util.DefaultScanResult.applyFilter(DefaultScanResult.java:87)
	at org.apache.maven.surefire.junitplatform.JUnitPlatformProvider.scanClasspath(JUnitPlatformProvider.java:142)
	at org.apache.maven.surefire.junitplatform.JUnitPlatformProvider.invoke(JUnitPlatformProvider.java:122)
	at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:385)
	at org.apache.maven.surefire.booter.ForkedBooter.execute(ForkedBooter.java:162)
	at org.apache.maven.surefire.booter.ForkedBooter.run(ForkedBooter.java:507)
	at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:495)
Caused by: org.junit.platform.commons.JUnitException: ClassSelector [className = 'com.puppycrawl.tools.checkstyle.checks.coding.unusedlocalvariable.InputUnusedLocalVariableDepthOfClasses$p$r'] resolution failed
	at org.junit.platform.launcher.listeners.discovery.AbortOnFailureLauncherDiscoveryListener.selectorProcessed(AbortOnFailureLauncherDiscoveryListener.java:39)
	at org.junit.platform.engine.support.discovery.EngineDiscoveryRequestResolution.resolveCompletely(EngineDiscoveryRequestResolution.java:103)
	at org.junit.platform.engine.support.discovery.EngineDiscoveryRequestResolution.run(EngineDiscoveryRequestResolution.java:83)
	at org.junit.platform.engine.support.discovery.EngineDiscoveryRequestResolver.resolve(EngineDiscoveryRequestResolver.java:113)
	at org.junit.jupiter.engine.discovery.DiscoverySelectorResolver.resolveSelectors(DiscoverySelectorResolver.java:46)
	at org.junit.jupiter.engine.JupiterTestEngine.discover(JupiterTestEngine.java:69)
	at org.junit.platform.launcher.core.EngineDiscoveryOrchestrator.discoverEngineRoot(EngineDiscoveryOrchestrator.java:152)
	... 15 more
Caused by: java.lang.NoClassDefFoundError: com/puppycrawl/tools/checkstyle/checks/coding/unusedlocalvariable/InputUnusedLocalVariableDepthOfClasses$P (wrong name: com/puppycrawl/tools/checkstyle/checks/coding/unusedlocalvariable/InputUnusedLocalVariableDepthOfClasses$p)
	at java.base/java.lang.ClassLoader.defineClass1(Native Method)
	at java.base/java.lang.ClassLoader.defineClass(ClassLoader.java:1017)
	at java.base/java.security.SecureClassLoader.defineClass(SecureClassLoader.java:150)
	at java.base/jdk.internal.loader.BuiltinClassLoader.defineClass(BuiltinClassLoader.java:862)
	at java.base/jdk.internal.loader.BuiltinClassLoader.findClassOnClassPathOrNull(BuiltinClassLoader.java:760)
	at java.base/jdk.internal.loader.BuiltinClassLoader.loadClassOrNull(BuiltinClassLoader.java:681)
	at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:639)
	at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:188)
	at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:525)
	at java.base/java.lang.Class.getDeclaringClass0(Native Method)
	at java.base/java.lang.Class.isMemberClass(Class.java:1780)
	at org.junit.platform.commons.util.ReflectionUtils.isInnerClass(ReflectionUtils.java:340)
	at org.junit.jupiter.engine.discovery.predicates.IsPotentialTestContainer.test(IsPotentialTestContainer.java:46)
	at org.junit.jupiter.engine.discovery.predicates.IsTestClassWithTests.test(IsTestClassWithTests.java:45)
	at org.junit.jupiter.engine.discovery.ClassSelectorResolver.resolve(ClassSelectorResolver.java:67)
	at org.junit.platform.engine.support.discovery.EngineDiscoveryRequestResolution.lambda$resolve$2(EngineDiscoveryRequestResolution.java:135)
	at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
	at java.base/java.util.ArrayList$ArrayListSpliterator.tryAdvance(ArrayList.java:1602)
	at java.base/java.util.stream.ReferencePipeline.forEachWithCancel(ReferencePipeline.java:129)
	at java.base/java.util.stream.AbstractPipeline.copyIntoWithCancel(AbstractPipeline.java:527)
	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:513)
	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
	at java.base/java.util.stream.FindOps$FindOp.evaluateSequential(FindOps.java:150)
	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
	at java.base/java.util.stream.ReferencePipeline.findFirst(ReferencePipeline.java:647)
	at org.junit.platform.engine.support.discovery.EngineDiscoveryRequestResolution.resolve(EngineDiscoveryRequestResolution.java:189)
	at org.junit.platform.engine.support.discovery.EngineDiscoveryRequestResolution.resolve(EngineDiscoveryRequestResolution.java:126)
	at org.junit.platform.engine.support.discovery.EngineDiscoveryRequestResolution.resolveCompletely(EngineDiscoveryRequestResolution.java:92)
	... 20 more

Context

The above reproduction steps were executed with Java 17.0.8.1 and Maven 3.9.5; the selected Checkstyle tag uses Surefire 3.2.1 and Junit 5.9.2. Macos-using colleagues of mine hit this issue due to this code. On Linux I did not hit the issue, though I could reproduce it by creating an in-memory FAT filesystem.

The issue is triggered by these two nested classes, named p and P, which on a case-insensitive filesystem cause bytecode to be written to the same .class file. Defining such "colliding" classes is an anti-pattern, but presumably Checkstyle did this for testing purposes, and ideally JUnit handles this case more robustly.

Of note: the issue does not manifest if Surefire is instructed to perform a "positive" rather than a "negative" filter, using e.g. -Dtest='foo'.

(I filed this issue against JUnit rather than Surefire, as the stack trace indicates the error happens quite "deep" inside JUnit code. Did not yet investigate whether an easy fix exists.)

Deliverables

• Ideally: correct test filtering in case of a class name/file name clash.
• Alternatively: improved error reporting.

CC @romani and @rnveach as Checkstyle maintainers (just as an FYI). CC @rickie and @mohamedsamehsalah as the colleagues who helped debug this.

[marcphilipp]

Here's what roughly happens:

1. Maven compiles com.puppycrawl.tools.checkstyle.checks.coding.unusedlocalvariable.InputUnusedLocalVariableDept. First the ...$p class is compiled and written to the file system, and then ...$P. But since the file system is case-insensitive the lower-case name is used for the upper case class name.
2. Surefire scans the test-classes dirs for all .class files that do not match the specified exclude pattern.
3. Since the pattern is supplied via -Dtest, the default includes are overridden and thus not applied.
4. Surefire creates a ClassSelector for all matching classes, including inner classes.
5. JUnit tries to inspect the class via reflection which fails in Class.isMemberClass since the parent class ...$p is not available (file contains byte code for ...$P)

I've tried setting the junit.platform.discovery.listener.default config param to logging (via systemPropertyVariables in the goal config). That causes JUnit to no longer propagate the exception above. But now Surefire throws an exception later when it gets to ...$p:

# Created at 2023-11-05T18:40:35.960
java.lang.NoClassDefFoundError: com/puppycrawl/tools/checkstyle/checks/coding/unusedlocalvariable/InputUnusedLocalVariableDepthOfClasses$p (wrong name: com/puppycrawl/tools/checkstyle/checks/coding/unusedlocalvariable/InputUnusedLocalVariableDepthOfClasses$P)
	at java.base/java.lang.ClassLoader.defineClass1(Native Method)
	at java.base/java.lang.ClassLoader.defineClass(ClassLoader.java:1027)
	at java.base/java.security.SecureClassLoader.defineClass(SecureClassLoader.java:150)
	at java.base/jdk.internal.loader.BuiltinClassLoader.defineClass(BuiltinClassLoader.java:862)
	at java.base/jdk.internal.loader.BuiltinClassLoader.findClassOnClassPathOrNull(BuiltinClassLoader.java:760)
	at java.base/jdk.internal.loader.BuiltinClassLoader.loadClassOrNull(BuiltinClassLoader.java:681)
	at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:639)
	at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:188)
	at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:526)
	at org.apache.maven.surefire.api.util.DefaultScanResult.loadClass(DefaultScanResult.java:115)
	at org.apache.maven.surefire.api.util.DefaultScanResult.applyFilter(DefaultScanResult.java:85)
	at org.apache.maven.surefire.junitplatform.JUnitPlatformProvider.scanClasspath(JUnitPlatformProvider.java:142)
	at org.apache.maven.surefire.junitplatform.JUnitPlatformProvider.invoke(JUnitPlatformProvider.java:122)
	at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:385)
	at org.apache.maven.surefire.booter.ForkedBooter.execute(ForkedBooter.java:162)
	at org.apache.maven.surefire.booter.ForkedBooter.run(ForkedBooter.java:507)
	at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:495)

The root cause is the same. This is bound to happen at some point when these classes are used. If it doesn't happen during test discovery it would happen at runtime when these classes are loaded. Therefore, I think this needs to be changed in Checkstyle, if they decide to support development on macOS (and potentially other case-insensitive file systems). I know too little about the project to suggest a fix but I wonder why these resources are even compiled (maybe to check that they actually do compile?) since IIRC Checkstyle operates on source code, not byte code. If the .class files are not needed to run tests, maybe they could be compiled to a separate output dir that's not being scanned by Surefire?