Make ArchUnit JUnit 5 support Java Modules compatible#827
Merged
Conversation
Since we use the `MANIFEST` entry `Automatic-Module-Name` the JUnit 5 modules are principally ready to be used on the modulepath. However, we automatically expose **all** packages declared, which means that the modules `junit5-api`, `junit5-engine` and `junit5-engine-api` all export the same package `com.tngtech.archunit.junit`, which is illegal for Java Modules. As a first step we move all classes contained in `junit5-engine-api` to a separate package to resolve part of the conflict. Unfortunately, this demands that we make some methods public that did not have to be before. Signed-off-by: Peter Gafert <peter.gafert@tngtech.com>
Since we use the `MANIFEST` entry `Automatic-Module-Name` the JUnit 5 modules are principally ready to be used on the modulepath. However, we automatically expose **all** packages declared, which means that the modules `junit5-api` and `junit5-engine` all export the same package `com.tngtech.archunit.junit`, which is illegal for Java Modules. We move all implementation details to a subpackage `internal` and only keep API toplevel. This way the module `junit5-engine` does not expose the root package `com.tngtech.archunit.junit` anymore, because all the contained classes are `internal`. While this separation works nicely and cleanly for `junit5`, the separation for `junit4` is a lot less pretty. Because the architecture of JUnit 4 makes it impossible to hide implementation details in a clean way, e.g. by demanding to annotate a `Runner` implementation, which should be pure hidden implementation detail. I added some separation by introducing `ArchUnitRunnerInternal` so we do not need to make all classes public, but have one (obviously not mean for outside usage) public gateway into the `internal` part. Signed-off-by: Peter Gafert <peter.gafert@tngtech.com>
e3096ca to
606b737
Compare
Unfortunately it is not trivial to test that the final artifacts work in a Java modules project. To keep the effort low we just add a test that catches the most probable error: The dependencies change and suddenly two artifacts comprising the JUnit 5 support export the same package. To simplify this even further we can just assume that each JUnit 5 artifact exports exactly one package and by asserting that this is exactly the expected package we effectively prevent collisions. If we use automatic module names we have to assume that artifacts will export all of their packages on the module path. Signed-off-by: Peter Gafert <peter.gafert@tngtech.com>
8a8d599 to
ff3bc43
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
So far, even though we added
Automatic-Module-Nameto theMANIFESTto make it compatible with Java Modules, ArchUnit's JUnit 5 support could not be used on the modulepath. The reason is that all three modulesarchunit-junit5-api,archunit-junit5-engineandarchunit-junit5-engine-apiexport the same packagecom.tngtech.archunit.junit.We now fix this by splitting the three modules into separate packages. This is a little tricky since there is also code that is shared with ArchUnit JUnit 4 support and we want to keep as many internals package private as possible. I tried to find a good compromise between backwards compatibility and public surface, the result is
c.t.archunit.junitc.t.archunit.junit.internal(this causes no problem, because engine dependencies were always only transparent runtime dependencies viaServiceLoader, never compile time dependencies)c.t.archunit.junit.engine_api. This is a breaking change, however theengine-apiis a very specific artifact that is only needed for frameworks to integrate with JUnit Platform. So it should only affect a very limited number of users.Unfortunately the JUnit 4 design makes it very hard to cleanly split API and implementation. I.e. by making the
Runnertype public API (having to explicitly declare it as@RunWith(..)) tends to cause a chain reaction pulling more and more into the public scope. In particular if now the classes that are used internally (likeClassCache) don't reside in the same package anymore. I tried to find some compromise by creating an internal delegate that is loaded via Reflection. This way we can keep the things inside ofinternalconceiled with package-private scope. Hopefully this will not cause any problem in the long term.Note that it will still not possible to put both JUnit 4 and JUnit 5 support on the modulepath together, but I also don't see any reason why anybody should want to do this, since they serve exactly the same purpose and JUnit 5 support is just more future proof.
Resolves: #206