JDK9 modules support for JUnitTask

[tzezula]

Changes:

1. Added modulepath and upgrademodulepath elements.
2. When modulepath or upgrademodulepath is given VM fork is required.
3. JUnit library required by Ant is searched both on classpath and modulepath.

As seen in JUnitTask + JDK9 question thread there are many ways how to write and execute unit test in the JDK9 involving several java options (-addmods, -Xpatch, -XaddExports, -XaddReads).

The JunitTask can

1. Keep the responsibility on user to correctly specify these options. The disadvantage of these solution is the complexity of VM options.
2. Automatically add the VM options. The disadvantage of these solution is that it’s impossible to cover all scenarios and there needs to be a way how to disable it. Also the options may change in time, for example if junit becomes a named module.

Currently the patch contains the 1st. solution.
Thanks for comments!

[tzezula]

I've forwarded the pull request to jigsaw-dev mailing list for comments.
http://mail.openjdk.java.net/pipermail/jigsaw-dev/2016-April/007515.html

[bodewig]

Thanks so much Tomáš and sorry for taking so long to get back to you.

I may be reading the code wrong, but do we end up with the modules being on the CLASSPATH as well as the module path if JUnit ist't part of the CLASSPATH? Can this do harm?

The code

return loader.getResource("junit.framework.Test") != null;

looks curious to me. I recall applying a fix to Ant's scriptdef subsystem because we've been told you wouldn't be able to load classes as resources anymore (we used ClassName.class rather than ClassName, though) - has this changed?

And one nit, Ant has been bitten by locale sensitive bugs before, please use Locale.ENGLISH when using toLowerCase.

[asfbot]

Rm Beer on dev@ant.apache.org replies:
sorry but not understand, i have privilege like developer for ant? i can
view all design and development.

2016-05-22 6:41 GMT-03:00 bodewig git@git.apache.org:
ck to you.
ur
e
se

[bodewig]

Have you seen my comments of "22 May", @tzezula ?

[tzezula]

Sorry I've overlooked the comment.

Regarding the JUnit lookup. When the JUnit is not found on the classpath it's searched on the modulepath. In fact the module path is used for 2 things here. First it's passed to the forked external process running the test(s). The presence of modulepath requires fork as there is no way how to change the module path of existing VM except of running the JUnit in a custom
Layer. The second thing the module path is used to is to lookup the JUnit needed by the JunitTask itself. In this case the JUnit is not handled as module but loaded by AntClassLoader as a jar into the unnamed module containing the Ant. I hope it should be OK.

The code: loader.getResource("junit.framework.Test") is clearly wrong, it should be either loader.getResource("junit/framework/Test.class") or loader.loadClass("junit.framework.Test") as done in createMirror. The loader.getResource("junit/framework/Test.class") works as both the JUnit and Ant are part of the unnamed module.
Fixed by: b9183f9

The toLowerCase problem is fixed by: 1fc1ce1

[tzezula]

I also improved the black-box test example in JUnit manual. The addExport to ALL-UNNAMED module containing the Ant is needed when Ant's test runner tries to invoke method on the test class.

[bodewig]

Thanks, looks good to me. Unfortunately git am fails to apply the first patch to JUnitTaskTest.java:27 - any chance you could rebase your patch on the current master branch?

[tzezula]

OK, I will rebase to current master.

[tzezula]

Should be rebased. Let me know in case of any problems.
Thanks!

[bodewig]

Unfortunately git am still didn't work, I've fallen back to applying patches manually (so your original commits didn't make it through).

Thanks a lot.

[tzezula]

Thanks a lot Stefan!