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 testRuntimeOnly dependency on junit-platform-launcher with Gradle 8.+ #4786

Merged
merged 1 commit into from
Dec 15, 2024

Conversation

timtebeek
Copy link
Contributor

@timtebeek timtebeek commented Dec 15, 2024

What's changed?

  • Add recipe to add Gradle dependency on org.junit.platform:junit-platform-launcher
  • Add transitive dependency when using rewrite-test

What's your motivation?

Reduce the warnings seen in build logs when using Gradle 8.x, ahead of a migration to Gradle 9.
https://docs.gradle.org/8.11.1/userguide/upgrading_version_8.html#test_framework_implementation_dependencies

Anything in particular you'd like reviewers to focus on?

Would the api dependency already prevent that we have to add the testRuntimeOnly dependency for rewrite-*?

Have you considered any alternatives or workarounds?

We could delay this until Gradle 9, but the warnings are already annoying right now.

@timtebeek timtebeek added the recipe Requested Recipe label Dec 15, 2024
@timtebeek timtebeek requested a review from shanman190 December 15, 2024 16:07
@timtebeek timtebeek self-assigned this Dec 15, 2024
@shanman190
Copy link
Contributor

shanman190 commented Dec 15, 2024

Would the api dependency already prevent that we have to add the testRuntimeOnly dependency for rewrite-*?

Yeah, based on my understanding once rewrite-test is exposing the dependency in the api configuration, then all second-level modules will get the benefit of it being available. The one part about this that we will want to keep an eye on is if there version conflicts introduced due to the api dependency with the imported junit-platform-launcher in transitives which are depending upon how Gradle interacts with it at runtime.

On the flip side, the recommendation of placing the dependency into testRuntimeOnly means that it would be available for testing tasks, but would not be exported for others to use and consume.

@timtebeek
Copy link
Contributor Author

You're very right to point out that the api("org.junit.platform:junit-platform-launcher") now comes in with the wrong scope, making it available to sources as opposed to runtime only. But without this change we'd have to update ~50+ build.gradle.kts files which for now only depend on rewrite-test, and expect the other transitive junit api dependencies already. I'd wanted to avoid those updates, especially since .kts isn't supported yet. With that I think we can tolerate the potential downsides, at least for now.

@timtebeek timtebeek merged commit d449490 into main Dec 15, 2024
2 checks passed
@timtebeek timtebeek deleted the add-testRuntimeOnly-junit-platform-launcher branch December 15, 2024 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
recipe Requested Recipe
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants