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 #3705

Open
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

mpkorstanje
Copy link
Contributor

@mpkorstanje mpkorstanje commented Feb 25, 2024

Overview

Adds several new methods to ReflectionSupport to find or stream resource objects in the classpath root, package, and modules.

A resource is represented as:

public interface Resource {
	String getName();

	URI getUri();

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

Fixes: #3630


I hereby agree to the terms of the JUnit Contributor License Agreement.


Definition of Done

*
* @return the resource name; never {@code null}
*/
String getName();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have intentionally expanded the original definition of Resource somewhat.

A few reasons:

  • While the URI does identify a resource uniquely, it is rooted in the file system. This makes it a less convenient as a human readable identifier. For example in Cucumber, feature files locations are identified by their classpath resource name or the path relative to the current working directory. For example
Scenario: Addition                   # io/cucumber/examples/calculator/basic_arithmetic.feature:7    <<<<<<<<<<<<
 Given a calculator I just turned on # io.cucumber.examples.calculator.RpnCalculatorStepDefinitions.a_calculator_I_just_turned_on()
 When I add 4 and 5                  # io.cucumber.examples.calculator.RpnCalculatorStepDefinitions.adding(int,int)
 Then the result is 9                # io.cucumber.examples.calculator.RpnCalculatorStepDefinitions.the_result_is(double)

Scenario Outline: Many additions     # io/cucumber/examples/calculator/basic_arithmetic.feature:30     <<<<<<<<<<<<
 Given a calculator I just turned on # io.cucumber.examples.calculator.RpnCalculatorStepDefinitions.a_calculator_I_just_turned_on()
 Given the previous entries:         # io.cucumber.examples.calculator.RpnCalculatorStepDefinitions.thePreviousEntries(java.util.List<io.cucumber.examples.calculator.RpnCalculatorStepDefinitions$Entry>)
   | first | second | operation |
   | 1     | 1      | +         |
   | 2     | 1      | +         |
 When I press +                      # io.cucumber.examples.calculator.RpnCalculatorStepDefinitions.I_press(java.lang.String)
 And I add 2 and 3                   # io.cucumber.examples.calculator.RpnCalculatorStepDefinitions.adding(int,int)
 And I press +                       # io.cucumber.examples.calculator.RpnCalculatorStepDefinitions.I_press(java.lang.String)
 Then the result is 10               # io.cucumber.examples.calculator.RpnCalculatorStepDefinitions.the_result_is(double)
  • To make it possible to use the package filter in the EngineDiscoveryRequestResolver in combination with the discovery to resources, ReflectionUtils. needs a Predicate<String> resourceNameFilter. The Predicate<Resource> resourceFilter should have access to the at least as much information as the Predicate<String>.

    To actually implement the package filter with resources some more work will have to be done to get the resource path and change it from a / separated path into . separated package name. But that is for later.

@mpkorstanje mpkorstanje changed the title Support scanning for class path resources Support scanning for classpath resources Mar 7, 2024
"org/junit/platform/jartest/included/included.resource",
"org/junit/platform/jartest/included/recursive/recursively-included.resource",
// TODO: This is interesting. Would we also scan classes in META-INF/versions?
"META-INF/MANIFEST.MF");
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if this is surfacing a bug in the class-path scanner or if this is expected behavior.

@mpkorstanje mpkorstanje marked this pull request as ready for review March 7, 2024 23:05
}

static Predicate<Path> resourceFiles() {
return file -> !isClassFile(file);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am explicitly excluding .class files when scanning for resources. Tests that are located in class files will always be selected with a class selector, so it doesn't make sense to all consider them when scanning for resources. This does assume that every .class file is a Java class file, but that seems reasonable enough.

mpkorstanje added a commit to mpkorstanje/junit5 that referenced this pull request 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.
@sbrannen
Copy link
Member

sbrannen commented Mar 9, 2024

Thanks for the PR, @mpkorstanje! 👍

We'll try to find some time to properly review it in the coming weeks (months?).

@mpkorstanje
Copy link
Contributor Author

Cheers! No problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support scanning for classpath resources
2 participants