How to handle (transitively) required modules?

[beatngu13]

The Jupiter API requires transitive org.opentest4j:

https://github.com/junit-team/junit5/blob/r5.8.2/junit-jupiter-api/src/module/org.junit.jupiter.api/module-info.java#L17

We don't declare it in our module-info.java, but use opentest4j classes directly, e.g. here:

junit-pioneer/src/test/java/org/junitpioneer/testkit/PioneerTestKit.java

Line 16 in 0a8d9fc

import org.opentest4j.TestAbortedException;

The modular build is obviously passing, but I wonder if there are best practices when it comes to using transitively declared modules? Should we declared it as required, too?

[beatngu13]

We could also ask ourselves whether it makes sense to include such modules as dependencies. For instance, here is what Maven says:

Although transitive dependencies can implicitly include desired dependencies, it is a good practice to explicitly specify the dependencies your source code uses directly. This best practice proves its value especially when the dependencies of your project change their dependencies.

However, I personally don't have a strong opinion on that and I'm also unsure what's "best" in our context.

[Bukama]

We (@beatngu13 @Michael1993 and me) discussed it at our last meeting and none of us had a strong opinion about it. I personally like to make my dependencies clear and therefore would go the Maven way and declare the dependency in Gradle build file and module-info, as for me there is a difference if I use a class directly (like in this case) or truly indirectly (which i also true at the moment for opentest4j).

[nipafx]

At the moment, Gradle build file and module declaration list all Jupiter artifacts we depend on (API, parameterized, commons, and launcher), but not their transitive dependency OpenTest4J even though we use its classes. This is possible because Gradle puts all transitive dependencies on the compile path and the module system lets us read them because Jupiter uses requires transitive.

I'm generally of the opinion that projects should rely on these indirect mechanisms if and only if they don't use the artifacts (e.g. OpenTest4J) for anything but integration with the direct dependency (e.g. Jupiter).

The problem for us is that this heuristic doesn't take us very far because all we're doing is integration with Jupiter. By that logic, we shouldn't depend on / require anything that Jupiter lets us use for free, which includes commons (which we currently depend on / require). But making that dependency implicit would be weird as we have a few classes that use only commons. Then again, explicitly listing these dependencies is pretty weird as well as we effectively have zero control over them (which is a big goal when handling them explicitly) - we can't use a different version than Jupiter does and if, say, Jupiter drops OpenTest4J, it's not like it makes sense for us to keep using it if Jupiter no longer recognizes the types.

So I see no principle that helps us decide this one way or another. 🥴

[nipafx]

Sure we do TDD, here!

[Michael1993]

Poll says NO. To me this basically sums it up:

we can't use a different version than Jupiter does and if, say, Jupiter drops OpenTest4J, it's not like it makes sense for us to keep using it if Jupiter no longer recognizes the types.

[Bukama]

Most commons on Twitter tends towards this too. So I'll close the issue.

[nipafx]

Uhm, but no means not relying on transitive dependencies, which implies adding OpenTest4J as a dependency and module. But judging by the few conversations the poll sparked, it made it too easy to give an opinion without considering the problem. Proposals were relatively unspecific and easy to "counter" with the respective opposite opinion as expressed above. I will hence ignore it.

I talked to @raphw about this and he had an interesting take on this: Compatibility with (breaking) changes in Jupiter. If Jupiter refactors their module relationships (which is of course highly unlike as that would be a breaking change), every module we explicitly depend on may cause problems (e.g. split packages). From that perspective, keeping the list of explicitly required modules short improves Pioneer's compatibility with a wide range of Jupiter versions.

Unlike the other pros and cons, this isn't just philosophical but can actually impact users and so even though it doesn't describe a particularly likely scenario, it has some weight and so became the tie breaker. We shall hence only directly depend on the following artifacts/modules:

• core API: org.junit.jupiter.api
• additional APIs as needed, e.g. org.junit.jupiter.params
• additional functionality as needed, e.g. org.junit.platform.launcher

That ruleset excludes org.junit.platform.commons, which we've depended on explicitly until now and so I remove it in #646, and org.opentest4j. While it's technically possible to elide org.junit.jupiter.api because org.junit.jupiter.params requires it transitively, that would be a bridge too far.

Disagreements with this decision should be discussed here - let's leave the PR for verifying whether it correctly implements the decision.