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

Add tests for multi source sets #888

Merged
merged 6 commits into from
Nov 1, 2024
Merged

Add tests for multi source sets #888

merged 6 commits into from
Nov 1, 2024

Conversation

nielsdebruin
Copy link
Contributor

@nielsdebruin nielsdebruin commented Oct 31, 2024

What's changed?

This PR adds support and test cases for running recipes on test source files located outside the standard maven test sources directory src/test/java for example src/integration-test/java.

Important: The PR showed a bug with our current integration testing method for the Maven plugin. This causes the test cases that have been added to fail, even though manual testing of the plugin by myself and @timtebeek showed it works as desired. For this reason, the added tests have currently been disabled.

What's your motivation?

Currently, recipes that do not rely on classpath resources, e.g., rewriting a boolean expression, will still work. However, recipes that rely on classpath resources such as FindTypes will not work.

Checklist

  • I've added unit tests to cover both positive and negative cases
  • I've read and applied the recipe conventions and best practices
  • I've used the IntelliJ IDEA auto-formatter on affected files

@nielsdebruin nielsdebruin self-assigned this Oct 31, 2024
@nielsdebruin nielsdebruin force-pushed the tests/multi-source-sets branch from 426461a to e9e17e1 Compare October 31, 2024 13:32
@nielsdebruin nielsdebruin force-pushed the tests/multi-source-sets branch from e08f01a to 638fcc5 Compare November 1, 2024 11:30
@nielsdebruin nielsdebruin added bug Something isn't working enhancement New feature or request java Pull requests that update Java code labels Nov 1, 2024
@nielsdebruin nielsdebruin marked this pull request as ready for review November 1, 2024 11:37
@timtebeek
Copy link
Contributor

I noticed that in Maven Core the build source directories are added to the project source roots as well;
https://github.com/apache/maven/blob/f8cba5dc2717230a17c9589efe111d8dd0c1abbf/impl/maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuilder.java#L576-L582

Are we sure we need to separately add those build source directories here? Or can we rely on the above?

@timtebeek timtebeek merged commit d86d22c into main Nov 1, 2024
1 check passed
@timtebeek timtebeek deleted the tests/multi-source-sets branch November 1, 2024 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request java Pull requests that update Java code
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Rewrite sources added through build-helper-maven-plugin
3 participants