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

Move junit-jupiter-api's dependency configuration to implementation #5985

Merged
merged 4 commits into from
Feb 16, 2023

Conversation

edysli
Copy link
Contributor

@edysli edysli commented Oct 11, 2022

Stop exposing org.junit.jupiter:junit-jupiter-api as a transitive dependency of org.testcontainers:junit-jupiter by including it using the implementation configuration instead of api. This should prevent conflicts with dependencies on JUnit 5 declared by users of org.testcontainers:junit-jupiter.

Also use JUnit's BOM to simplify version management of all included JUnit artifacts. For running this module's tests, a testImplementation dependency on org.junit.jupiter:junit-jupiter is enough, as recommended in the JUnit 5 User Guide.

Users of the @Testcontainers annotation must ensure their project direcly declares a test-scoped dependency on org.junit.jupiter:junit-jupiter in addition to org.testcontainers:junit-jupiter. See the JUnit 5 User Guide for recommended configurations.

Fixes: #5680

Stop exposing `org.junit.jupiter:junit-jupiter-api` as a transitive
dependency of `org.testcontainers:junit-jupiter` by including it using
the `implementation` configuration instead of `api`. This should
prevent conflicts with dependencies on JUnit 5 declared by users of
`org.testcontainers:junit-jupiter`.

Also use JUnit's BOM to simplify version management of all included
JUnit artifacts. For running this module's tests, a
`testImplementation` dependency on `org.junit.jupiter:junit-jupiter`
is enough, as recommended in the [JUnit 5 User Guide][junit5guide].

Users of the `@Testcontainers` annotation must ensure their project
direcly declares a test-scoped dependency on
`org.junit.jupiter:junit-jupiter` in addition to
`org.testcontainers:junit-jupiter`.
See the [JUnit 5 User Guide][junit5guide] for recommended
configurations.

Fixes: testcontainers#5680

[junit5guide]: https://junit.org/junit5/docs/current/user-guide/#running-tests-build "JUnit 5 User Guide: Build Support"
@edysli
Copy link
Contributor Author

edysli commented Oct 11, 2022

I think the release notes should mention that projects using org.testcontainers:junit-jupiter should ensure they declare a direct dependency on org.junit.jupiter:junit-jupiter. How can we do this?

@eddumelendez
Copy link
Member

thanks for submitting the PR @edysli ! If we decide to include it in the next release, we will include a note on the Release Notes.

Examples are failing because of missing the junit dependency :)

@eddumelendez eddumelendez added the type/breaking-api-change Public APIs are being changed label Oct 11, 2022
Add a direct dependency on `org.junit.jupiter:junit-jupiter:5.9.1` to
stop relying on it being transitively provided by
`org.testcontainers:junit-jupiter`.
@edysli
Copy link
Contributor Author

edysli commented Oct 11, 2022

Ironically, this CI failure illustrates exactly what happens to projects who don't declare a dependency on JUnit after the changes in this PR! 😉

/home/runner/work/testcontainers-java/testcontainers-java/examples/neo4j-container/src/test/java/org/testcontainers/containers/Neo4jExampleTest.java:13: error: package org.junit.jupiter.api does not exist
import org.junit.jupiter.api.Test;
                            ^
/home/runner/work/testcontainers-java/testcontainers-java/examples/neo4j-container/src/test/java/org/testcontainers/containers/Neo4jExampleTest.java:34: error: cannot find symbol
    @Test
     ^
  symbol:   class Test
  location: class org.testcontainers.containers.Neo4jExampleTest

@eddumelendez eddumelendez changed the title Stop exposing junit-jupiter-api as transitive dependency Move junit-jupiter-api's dependency configuration to implementation Oct 11, 2022
@edysli
Copy link
Contributor Author

edysli commented Oct 11, 2022

Examples are failing because of missing the junit dependency :)

The remaining failures seem to have been caused by connectivity problems. Or did I miss a dependency somewhere @eddumelendez ?

@eddumelendez
Copy link
Member

checks are passing now

kiview
kiview previously approved these changes Oct 12, 2022
Copy link
Member

@kiview kiview left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @edysli.
I think this is the right approach, so we will need to live with the breaking change. Ideally, it is not breaking for most users, since they should already have a direct dependency to junit-jupiter declared, but we will still need to highlight it in the release notes, as mentioned by @eddumelendez.

@kiview
Copy link
Member

kiview commented Oct 19, 2022

Just FYI @edysli, we plan to eventually merge this PR, since we think it is the right approach. However, because of its breaking nature, we want to include it in the 1.18.0 release (we don't have a strict SemVer policy, but it should not be in a patch release). Therefore, we don't merge it yet.

However, it added the hacktoberfest-accepted label, which counts towards your Hacktoberfest contributions, even without merging. Thanks for your work on this 🙌

@edysli
Copy link
Contributor Author

edysli commented Oct 19, 2022

Thank you so much @kiview ! 😁 I totally understand this has to wait for the appropriate release.

eddumelendez
eddumelendez previously approved these changes Feb 16, 2023
@eddumelendez eddumelendez added this to the next milestone Feb 16, 2023
@eddumelendez eddumelendez dismissed stale reviews from kiview and themself via 922a70b February 16, 2023 17:54
@eddumelendez eddumelendez merged commit 7d1cf6b into testcontainers:main Feb 16, 2023
@eddumelendez
Copy link
Member

Thanks for your contribution, @edysli ! This is now merged in main branch and it will be part of the next release.

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.

[Bug]: No tests found with JUnit 5.9.0
3 participants