Skip to content

Do not ever exclude junit if Testcontainers is used #746

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

Conversation

DidierLoiseau
Copy link
Contributor

@DidierLoiseau DidierLoiseau commented Jun 7, 2025

What's changed?

Instead of adding the JUnit 4 exclusion to all dependencies and then removing it from specific ones that depend on Testcontainers, just don’t add the exclusion when Testcontainers is used.

What's your motivation?

We have implemented some test libraries that (transitively) depend on Testcontainers. When (indirectly) running the JUnit 5 migration recipe, it always adds the junit exclusion on those, which breaks the build. This forces us to un-exclude JUnit 4 as a followup recipe.

Have you considered any alternatives or workarounds?

Current workaround is to remove the exclusion explicitly for all dependencies after JUnit 5 migration:

  - org.openrewrite.maven.RemoveExclusion:
      groupId: '*'
      artifactId: '*'
      exclusionGroupId: junit
      exclusionArtifactId: junit

Any additional context

This makes the recipe more conservative. If a project transitively depends on Testcontainers, it won’t add the exclusion to dependencies that do not depend on Testcontainers (but still depend on JUnit 4) but that’s ok since exclusions have to be added everywhere to work properly.

This provides a more generic solution to #429 and #477.

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

@github-project-automation github-project-automation bot moved this to In Progress in OpenRewrite Jun 7, 2025
@@ -157,9 +157,76 @@ void upgradeMavenPluginVersions() {
);
}

@Test
void excludeJunit4Dependency() {
// Just using play-test_2.13 as an example because it appears to still depend on junit.
Copy link
Contributor Author

@DidierLoiseau DidierLoiseau Jun 7, 2025

Choose a reason for hiding this comment

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

I actually wanted to provide a pom with the junit dependency in the test, and then run it as:

        rewriteRun(
          mavenProject("depends-on-junit", pomXml(remoteDepWithJunit, SourceSpec::skip)),
          mavenProject("needs-exclusion", pomXml(before, after)));

and somehow it works – the exclusion is added as intended – but it also gets a MavenDownloadException added in the output.

I was trying to do the same as other Maven tests that use SourceSpec::skip, but they all seem to do that for a parent, not a dependency.

Copy link
Member

@timtebeek timtebeek left a comment

Choose a reason for hiding this comment

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

Welcome back! Great use of a precondition indeed for this longer standing wart in our recipes.

Still will keep fingers crossed for a Junit4-less-version-of-testcontainers

@github-project-automation github-project-automation bot moved this from In Progress to Ready to Review in OpenRewrite Jun 7, 2025
@timtebeek timtebeek added recipe Recipe request junit labels Jun 7, 2025
@timtebeek timtebeek merged commit 3dd349c into openrewrite:main Jun 7, 2025
@github-project-automation github-project-automation bot moved this from Ready to Review to Done in OpenRewrite Jun 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
junit recipe Recipe request
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants