Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support scanning for classpath resources #3630

Closed
1 task
mpkorstanje opened this issue Jan 4, 2024 · 8 comments · Fixed by #3705
Closed
1 task

Support scanning for classpath resources #3630

mpkorstanje opened this issue Jan 4, 2024 · 8 comments · Fixed by #3705

Comments

@mpkorstanje
Copy link
Contributor

mpkorstanje commented Jan 4, 2024

Currently ReflectionUtils supports resolving class based selectors such as PackageSelector and ClasspathRootSelector. These can be resolved using findAllClassesInPackage and findAllClassesInClasspathRoot respectively.

Unfortunately there are no resource based equivalents to help resolve resources contained in a class path directory targeted by a ClasspathResourceSelector or ClasspathRootSelector e.g:

List<Resource> findAllResourcesInPackage(String basePackageName, ResourceFilter resourceFilter)

List<Resource> findAllResourcesInClasspathRoot(URI root, ResourceFilter resourceFilter) 

As this functionality is backed by the ClasspathScanner I believe it would be prudent to add this functionality prior to making the ClasspathScanner part of the public API as proposed in #3628. This would avoid breaking changes by adding more methods to an interface, or potential bug reports caused by a default implementation that always returns an empty collection of resources.

And I would be happy to provide a PR that facilitates this.

Deliverables

  • Support for scanning class path resources through ReflectionUtils and ClasspathScanner.
@sbrannen
Copy link
Member

sbrannen commented Jan 4, 2024

We never implemented that, because we never needed to scan for resources (other than .class files).

What's the concrete use case you're thinking of -- scanning for Cucumber files or similar?

@mpkorstanje
Copy link
Contributor Author

mpkorstanje commented Jan 4, 2024

Yup. Cucumber uses .feature files which are typically located on the classpath.

The benefits would be two-fold

  • Eventually replace Cucumbers classpath scanning with the JUnit Platform
  • Benefit from implementations that support classpath scanning on Android.

@marcphilipp
Copy link
Member

marcphilipp commented Jan 5, 2024

interface Resource {
	URI getUri();
	default InputStream getInputStream() throws IOException {
		return getUri().toURL().openStream();
	}
}

@marcphilipp
Copy link
Member

marcphilipp commented Jan 5, 2024

Team decision: Let's add the two extra methods to ClasspathScanner and make them available via ReflectionSupport and ReflectionUtils.

As a follow-up task, we should add an addResourceContainerSelectorResolver() method to EngineDiscoveryRequestResolver.Builder analogous to addClassContainerSelectorResolver().

@marcphilipp
Copy link
Member

@mpkorstanje A PR would be most welcome! 🙂

@mpkorstanje
Copy link
Contributor Author

Cheers. I'll give it a shot.

As a follow-up task, we should add a addResourceContainerSelectorResolver() method to EngineDiscoveryRequestResolver.Builder analogous to addClassContainerSelectorResolver().

Oh that would be really nice!

@mpkorstanje
Copy link
Contributor Author

I am still working on this, unfortunately due to circumstances it is going a bit slower than usual.

@sbrannen
Copy link
Member

Thanks for letting us know, @mpkorstanje.

This is currently assigned to the 5.11 M2 milestone, so you've got a bit more time.

mpkorstanje added a commit to mpkorstanje/junit5 that referenced this issue Feb 25, 2024
mpkorstanje added a commit to mpkorstanje/junit5 that referenced this issue Feb 25, 2024
@mpkorstanje mpkorstanje changed the title Support scanning for class path resources Support scanning for classpath resources Mar 7, 2024
mpkorstanje added a commit to mpkorstanje/junit5 that referenced this issue Mar 8, 2024
As a follow up for junit-team#3630 and junit-team#3705 this adds a
`addResourceContainerSelectorResolver()`
method to `EngineDiscoveryRequestResolver.Builder` analogous to
`addClassContainerSelectorResolver()`.

Points of note:

 * As classpath resources can be selected from packages, the package
   filter should also be applied. To make this possible the base path of
   a resource is rewritten to a package name prior to being filtered.

 * The `ClasspathResourceSelector` now has a `getClasspathResource`
   method. This method will lazily try to load the resource if not was
   not already provided when discovering resources in a container.

 * `selectClasspathResource(Resource)` was added to short circuit the
    need to resolve resources twice. And to make it possible to use
    this method as part of the public API,
    `ReflectionSupport.tryToLoadResource` was also added.
@marcphilipp marcphilipp modified the milestones: 5.11 M2, 5.11 M3 May 17, 2024
marcphilipp added a commit that referenced this issue Oct 29, 2024
As a follow up for #3630 and #3705 this adds a
`addResourceContainerSelectorResolver()`
method to `EngineDiscoveryRequestResolver.Builder` analogous to
`addClassContainerSelectorResolver()`.

Points of note:

 * As classpath resources can be selected from packages, the package
   filter should also be applied. To make this possible the base path of
   a resource is rewritten to a package name prior to being filtered.

 * The `ClasspathResourceSelector` now has a `getClasspathResources`
   method. This method will lazily try to load the resources if not already
   provided when discovering resources in a container.

 * `selectClasspathResource(Resource)` was added to short circuit the
    need to resolve resources twice. And to make it possible to use
    this method as part of the public API,
    `ReflectionSupport.tryToLoadResource` was also added.

---------

Co-authored-by: Marc Philipp <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants