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

Set 'junit' as Automatic-Module-Name #1571

Merged
merged 1 commit into from
Nov 25, 2018
Merged

Conversation

sormuras
Copy link
Member

Prior to this commit, junit was derived as the module name from the name of the JAR file. Now, junit is set as an explicit module name, stored in the MANIFEST.MF file, to ensure a smooth ride into the modular age for users of JUnit 4.

Prior to this commit, `junit` was derived as the module name
from the name of the JAR file. Now, `junit` is set as an explicit
module name, stored in the MANIFEST.MF file, to ensure a
smooth ride into the modular age for users of JUnit 4.
Copy link
Member

@kcooney kcooney left a comment

Choose a reason for hiding this comment

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

Perhaps we should start the module name with "org.junit". See http://branchandbound.net/blog/java/2017/12/automatic-module-name/

@sbrannen
Copy link
Member

I definitely think it's a good idea to have this in place in JUnit 4.13.

Regarding what the automatic module name should be, well... that's of course debatable.

If we look to the automatic module names for artifacts from JUnit 5 for inspiration, one would likely assume org.junit as @kcooney suggested.

@marcphilipp marcphilipp added this to the 4.13 milestone Nov 24, 2018
@stefanbirkner
Copy link
Contributor

stefanbirkner commented Nov 24, 2018

If we don't use junit as the module name then the name should be based on org.junit. I think we have to choose between org.junit, org.junit.junit and org.junit.junit4.
Although org.junit is the common prefix of all packages of JUnit 4 classes I would not use it because it is the prefix of the JUnit 5's automatic module names (e.g. org.junit.platform.commons), too.
I favor org.junit.junit4 because it makes it very clear that this is JUnit 4 and that you can use it together with other JUnit 5.

However there is a good reason for using junit. JUnit <= 4.12 usually has the automatic module name junit which is derived from the jar. Explecitly using junit as module name makes upgrades easier for users who already use the module name junit.

@sbrannen
Copy link
Member

I favor org.junit.junit4 because it makes it very clear that this is JUnit 4 and that you can use it together with other JUnit 5.

It's indeed tricky!

Going with junit4 in the name is also a bit misleading since that same module would also effectively contain JUnit 3.8.2.

However there is a good reason for using junit. JUnit <= 4.12 usually has the automatic module name junit which is derived from the jar. Explecitly using junit as module name makes upgrades easier for users who already use the module name junit.

That's a valid point.

Plus, we didn't start using org.junit in Group IDs until JUnit 5.

So maybe it is best to just stick with junit.

I'm torn....... 😇

@marcphilipp
Copy link
Member

Thanks for the lively discussion! Given the options, I think junit would be best.

@kcooney Any objections?

@sormuras
Copy link
Member Author

Pardon for joining late to the discussion -- but it basically resembles my thought process to propose junit in this PR. One of our own examples already uses this module name: https://github.com/junit-team/junit5-samples/blob/master/junit5-modular-world/src/test/black.box/module-info.java#L22

Given, not many others will have such a requires junit; line in their project -- but upgrading from <= 4.12 should not alter the already published (automatic) module name. With locking it down to junit we're now on the save side. And we now "own" the triple:

  1. junit - Maven Group ID
  2. junit - Artifact ID
  3. junit - Java Module Name

Looks consistent.

@marcphilipp
Copy link
Member

I've decided to merge this PR so I can include it in 4.13-beta-1. We could still change it before releasing 4.13.

@marcphilipp marcphilipp merged commit d89a403 into junit-team:master Nov 25, 2018
@marcphilipp
Copy link
Member

Thanks, @sormuras!

@stefanbirkner
Copy link
Contributor

I updated the release notes.

@sormuras sormuras deleted the patch-1 branch November 25, 2018 15:49
@marcphilipp
Copy link
Member

Thanks! 👌

@kcooney
Copy link
Member

kcooney commented Nov 25, 2018

I am confused as to why choosing a different name from the automatic module name would be a problem. If it was why would they recommended that we choose names based on the package(s) of the included classes?

@sormuras
Copy link
Member Author

image

The junit-4.12.jar contains both junit/ and org/ as a top-level directory (ignoring META-INF/). Taking that into account when reading Stephen's Creating a module with a particular name takes ownership of that package name and everything beneath it. leaves us with two choices: either claim junit or org.junit.

We can't meet all requirements/recommendations. For me, it's a close call. junit is 51 % good, and org.junit 49%.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants