ClasspathResourceSelector is more lenient than ClasspathResourceSource

[mpkorstanje]

Steps to reproduce

0. Have a test engine use JUnits EngineDiscoveryRequestResolver.
1. Discover with DiscoverySelectors.selectClasspathResource("/").
2. Have the test engine call .getClasspathResources() on that selector. This will result in a PreconditionViolationException.
3. This exception is then caught by the EngineDiscoveryRequestResolver which tries to create an issue for it.
4. This then fails because the ClasspathResourceSource does not accept the empty resource.

To reproduce clone: https://github.com/cucumber/cucumber-java-skeleton and replace the package selector in RunCucumberTest with @SelectClasspathResource("/"). Observe:

org.junit.platform.commons.JUnitException: TestEngine with ID 'junit-platform-suite' failed to discover tests
	at org.junit.platform.launcher.core.EngineDiscoveryOrchestrator.discoverEngineRoot(EngineDiscoveryOrchestrator.java:208)
	at org.junit.platform.launcher.core.EngineDiscoveryOrchestrator.discoverSafely(EngineDiscoveryOrchestrator.java:174)
	at org.junit.platform.launcher.core.EngineDiscoveryOrchestrator.discover(EngineDiscoveryOrchestrator.java:119)
	at org.junit.platform.launcher.core.EngineDiscoveryOrchestrator.discover(EngineDiscoveryOrchestrator.java:84)
	at org.junit.platform.launcher.core.DefaultLauncher.discover(DefaultLauncher.java:104)
	at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:91)
	at org.junit.platform.launcher.core.DelegatingLauncher.execute(DelegatingLauncher.java:47)
	at org.junit.platform.launcher.core.InterceptingLauncher.lambda$execute$1(InterceptingLauncher.java:39)
	at org.junit.platform.launcher.core.ClasspathAlignmentCheckingLauncherInterceptor.intercept(ClasspathAlignmentCheckingLauncherInterceptor.java:25)
	at org.junit.platform.launcher.core.InterceptingLauncher.execute(InterceptingLauncher.java:38)
	at org.junit.platform.launcher.core.DelegatingLauncher.execute(DelegatingLauncher.java:47)
	at org.junit.platform.launcher.core.SessionPerRequestLauncher.execute(SessionPerRequestLauncher.java:66)
	at com.intellij.junit5.JUnit5IdeaTestRunner.startRunnerWithArgs(JUnit5IdeaTestRunner.java:57)
	at com.intellij.rt.junit.IdeaTestRunner$Repeater$1.execute(IdeaTestRunner.java:38)
	at com.intellij.rt.execution.junit.TestsRepeater.repeat(TestsRepeater.java:11)
	at com.intellij.rt.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:35)
	at com.intellij.rt.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:231)
	at com.intellij.rt.junit.JUnitStarter.main(JUnitStarter.java:55)
Caused by: org.junit.platform.commons.JUnitException: TestEngine with ID 'cucumber' failed to discover tests
	at org.junit.platform.launcher.core.EngineDiscoveryOrchestrator.discoverEngineRoot(EngineDiscoveryOrchestrator.java:208)
	at org.junit.platform.launcher.core.EngineDiscoveryOrchestrator.discoverSafely(EngineDiscoveryOrchestrator.java:174)
	at org.junit.platform.launcher.core.EngineDiscoveryOrchestrator.discover(EngineDiscoveryOrchestrator.java:119)
	at org.junit.platform.launcher.core.EngineDiscoveryOrchestrator.discover(EngineDiscoveryOrchestrator.java:102)
	at org.junit.platform.suite.engine.SuiteLauncher.discover(SuiteLauncher.java:58)
	at org.junit.platform.suite.engine.SuiteTestDescriptor.discover(SuiteTestDescriptor.java:126)
	at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:183)
	at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
	at java.base/java.util.Iterator.forEachRemaining(Iterator.java:133)
	at java.base/java.util.Spliterators$IteratorSpliterator.forEachRemaining(Spliterators.java:1845)
	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
	at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:150)
	at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:173)
	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
	at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:596)
	at org.junit.platform.suite.engine.DiscoverySelectorResolver.discoverSuites(DiscoverySelectorResolver.java:40)
	at org.junit.platform.suite.engine.DiscoverySelectorResolver.resolveSelectors(DiscoverySelectorResolver.java:48)
	at org.junit.platform.suite.engine.SuiteTestEngine.discover(SuiteTestEngine.java:60)
	at org.junit.platform.launcher.core.EngineDiscoveryOrchestrator.discoverEngineRoot(EngineDiscoveryOrchestrator.java:195)
	... 17 more
Caused by: org.junit.platform.commons.PreconditionViolationException: Classpath resource name must not be null or blank
	at org.junit.platform.commons.util.Preconditions.condition(Preconditions.java:298)
	at org.junit.platform.commons.util.Preconditions.notBlank(Preconditions.java:266)
	at org.junit.platform.engine.support.descriptor.ClasspathResourceSource.<init>(ClasspathResourceSource.java:120)
	at org.junit.platform.engine.support.descriptor.ClasspathResourceSource.<init>(ClasspathResourceSource.java:116)
	at org.junit.platform.engine.support.descriptor.ClasspathResourceSource.from(ClasspathResourceSource.java:63)
	at org.junit.platform.launcher.core.DiscoveryIssueCollector.lambda$toSource$1(DiscoveryIssueCollector.java:95)
	at java.base/java.util.Optional.orElseGet(Optional.java:364)
	at org.junit.platform.launcher.core.DiscoveryIssueCollector.toSource(DiscoveryIssueCollector.java:95)
	at org.junit.platform.launcher.core.DiscoveryIssueCollector.selectorProcessed(DiscoveryIssueCollector.java:69)
	at org.junit.platform.launcher.listeners.discovery.CompositeLauncherDiscoveryListener.lambda$selectorProcessed$4(CompositeLauncherDiscoveryListener.java:61)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at java.base/java.util.Collections$UnmodifiableCollection.forEach(Collections.java:1092)
	at org.junit.platform.launcher.listeners.discovery.CompositeLauncherDiscoveryListener.selectorProcessed(CompositeLauncherDiscoveryListener.java:61)
	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:148)
	at io.cucumber.junit.platform.engine.DiscoverySelectorResolver.resolveSelectors(DiscoverySelectorResolver.java:32)
	at io.cucumber.junit.platform.engine.FeaturesPropertyResolver.resolveSelectors(FeaturesPropertyResolver.java:52)
	at io.cucumber.junit.platform.engine.CucumberTestEngine.discover(CucumberTestEngine.java:57)
	at org.junit.platform.launcher.core.EngineDiscoveryOrchestrator.discoverEngineRoot(EngineDiscoveryOrchestrator.java:195)
	... 36 more

Context

With JUnit 5.13.3.

Originally reported as cucumber/cucumber-jvm#3024.

Deliverables

• Make ClasspathResourceSelector reject the empty classpath resource name.
• Ensure that Suite Engine reports annotations with the empty resource name as a problem and doesn't throw when doing so.

[marcphilipp]

This is about @SelectClasspathResource("/"), not @SelectClasspathResource("") which would already be rejected in DiscoverySelectors, wouldn't it?

How did this work before? Cucumber calling ClasspathResourceSelector.getClasspathResources() already threw an exception before introducing DiscoveryIssues, didn't it?

[mpkorstanje]

This is about @SelectClasspathResource("/"), not @SelectClasspathResource("") which would already be rejected in DiscoverySelectors, wouldn't it?

Ah. My bad. Yes, @SelectClasspathResource("") would be rejected. And @SelectClasspathResource("/") would not be rejected. But the / is normalized away when creating the selector.

junit-framework/junit-platform-engine/src/main/java/org/junit/platform/engine/discovery/ClasspathResourceSelector.java

Lines 64 to 68 in 3c42e16

	ClasspathResourceSelector(String classpathResourceName, @Nullable FilePosition position) {
	boolean startsWithSlash = classpathResourceName.startsWith("/");
	this.classpathResourceName = (startsWithSlash ? classpathResourceName.substring(1) : classpathResourceName);
	this.position = position;
	}

How did this work before? Cucumber calling ClasspathResourceSelector.getClasspathResources() already threw an exception before introducing DiscoveryIssues, didn't it?

Previously Cucumber was not using the EngineDiscoveryRequestResolver and did its used its own classpath scanning which didn't make a distinction between scanning packages or classpath resources - it assumed the empty resource meant that the classpath root should be scanned for feature files. I am happy with @SelectClasspathResource("/") not being allowed though.

Though the exception thrown is an incidental detail. When using the EngineDiscoveryRequestResolver any exception thrown when resolving DiscoverySelectors.selectClasspathResource("/") will end up here:

junit-framework/junit-platform-launcher/src/main/java/org/junit/platform/launcher/core/DiscoveryIssueCollector.java

Lines 89 to 95 in 3c42e16

	if (selector instanceof ClasspathResourceSelector resourceSelector) {
	String resourceName = resourceSelector.getClasspathResourceName();
	return resourceSelector.getPosition() //
	.map(DiscoveryIssueCollector::convert) //
	.map(position -> ClasspathResourceSource.from(resourceName, position)) //
	.orElseGet(() -> ClasspathResourceSource.from(resourceName));
	}

And thus cause another exception because the slash is normalized to the empty string, which is not a valid class-path resource name..