Introduce `selectClasspathResources(Set)` in `DiscoverySelectors` and deprecate `selectClasspathResource(Set)`

[sbrannen]

Overview

While working on #4868, I realized that we accidentally left of the "s" in the name of selectClasspathResource(Set) in DiscoverySelectors.

It should rather be called selectClasspathResources(Set) (plural), analogous to selectClasspathRoots() and selectModules().

Since selectClasspathResource(Set) was introduced in JUnit Platform 1.12, we need to leave it in place for the time being but in a @Deprecated state.

Related Issues

• Document Set semantics for selectClasspathResource(), selectClasspathRoots(), and selectModules() in DiscoverySelectors #4868

[sbrannen]

I looked into this in greater detail and noticed that the semantics of selectClasspathResource(Set) are rather "special".

selectClasspathResource(Set) selects from a Set but returns a single ClasspathResourceSelector.

Whereas, other "plural" selector methods in DiscoverySelectors return a List of selectors.

ClasspathResourceSelector was recently retrofitted to provide a Set<Resource> getClasspathResources() method. So, you still end up with potentially multiple resources, but that is in strict contrast to how other DiscoverySelector implementations work.

In other words, selectClasspathResource(Set) selects from multiple things (a set) and returns a single thing (a ClasspathResourceSelector) that can in turn be queried for multiple things (getClasspathResources()).

Compared to how selection typically/historically works, I would have rather expected something like the following in DicoverySelectors.

@API(status = EXPERIMENTAL, since = "6.0")
public static List<ClasspathResourceSelector> selectClasspathResources(Resource... classpathResources) {
	Preconditions.notEmpty(classpathResources, "classpath resources must not be null or empty");
	return selectClasspathResources(List.of(classpathResources));
}

@API(status = EXPERIMENTAL, since = "6.0")
public static List<ClasspathResourceSelector> selectClasspathResources(List<Resource> classpathResources) {
	Preconditions.notEmpty(classpathResources, "classpath resources must not be null or empty");
	Preconditions.containsNoNullElements(classpathResources, "individual classpath resources must not be null");

	// @formatter:off
	return classpathResources.stream()
			.distinct()
			.peek(resource -> Preconditions.notBlank(resource.getName(),
				"classpath resource names must not be null or blank"))
			.map(ClasspathResourceSelector::new)
			.toList();
	// @formatter:on
}

With the following new constructor for ClasspathResourceSelector.

ClasspathResourceSelector(Resource classpathResource) {
	this(classpathResource.getName(), null);
	this.classpathResources = Set.of(classpathResource);
}

See: main...sbrannen:junit-framework:issues/4873-DiscoverySelectors-selectClasspathResources

---

Note that selectClasspathResource(Set) was introduced in conjunction with #3718, which was a rather involved PR with lots of back-and-forth discussion.

So, I'm not really sure how we came up with the current semantics of selectClasspathResource(Set) or whether we want/need to keep that in place.

In light of that, I think we need to discuss this topic before making any changes prior to 6.0 GA.

@mpkorstanje and @marcphilipp, can you shed some light on the rationale for the current naming and/or behavior of selectClasspathResource(Set)?

[mpkorstanje]

The selectClasspathResource(Set<Resource>) is an optimized version of selectClasspathResource(String) such that

var selectorA = DiscoverySelectors.selectClasspathResource("example.resource");
var resources = selector.getClasspathResources();
var selectorB = DiscoverySelectors.selectClasspathResource(resources);
assertTrue(selectorA.equals(selectorB));

This optimization is important for the ResourceContainerSelectorResolver. It scans the class-path, producing resources and turns these into classpath resource selectors. It would be wasteful if a test engine then had to scan the classpath again when calling getClasspathResources so we optimize that with the selectClasspathResource(Set<Resource>) selector.

The method name is singular because both only select a single classpath resource by name. But a single classpath resource by name may resolve into multiple actual resources. Also note that the pre-conditions for selectClasspathResource(Set<Resource>) enforce these semantics.

It is up to a test engine implementation to decide how to deal with multiple identically named resources. With that in mind, returning a list of selectors, one for each resource, would not be correct. Test engines are assumed to resolve selectors individually, so they wouldn't be able to detect duplicate resources.

[mpkorstanje]

If a selectClasspathResources is needed then I imagine this would be sufficient for the common use case of selecting multiple resources by name:

@API(status = EXPERIMENTAL, since = "6.0")
public static List<ClasspathResourceSelector> selectClasspathResources(String... classpathResourceNames) {

}

@API(status = EXPERIMENTAL, since = "6.0")
public static List<ClasspathResourceSelector> selectClasspathResources(List<String> classpathResourceNames) {
}