[GitHub] ant pull request: JDK9 modules support for JUnitTask

classic Classic list List threaded Threaded
13 messages Options
Reply | Threaded
Open this post in threaded view
|

[GitHub] ant pull request: JDK9 modules support for JUnitTask

bodewig
GitHub user tzezula opened a pull request:

    https://github.com/apache/ant/pull/18

    JDK9 modules support for JUnitTask

    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](http://mail-archives.apache.org/mod_mbox/ant-dev/201604.mbox/%3CAFE6C849-0622-44D1-9FF7-3A6CA4832F82%40oracle.com%3E) 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!

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/tzezula/ant jigsaw/junit

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/ant/pull/18.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #18
   
----
commit 2161a1383a3d09266351d70343f72a6398eb3a93
Author: Tomas Zezula <[hidden email]>
Date:   2016-04-21T20:06:41Z

    JDK9 modules support for JUnitTask

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] ant pull request: JDK9 modules support for JUnitTask

bodewig
Github user tzezula commented on the pull request:

    https://github.com/apache/ant/pull/18#issuecomment-215201380
 
    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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] ant pull request: JDK9 modules support for JUnitTask

bodewig
In reply to this post by bodewig
Github user bodewig commented on the pull request:

    https://github.com/apache/ant/pull/18#issuecomment-220823223
 
    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
   
    ```java
    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`.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [GitHub] ant pull request: JDK9 modules support for JUnitTask

Rm Beer
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 <[hidden email]>:

> Github user bodewig commented on the pull request:
>
>     https://github.com/apache/ant/pull/18#issuecomment-220823223
>
>     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
>
>     ```java
>     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`.
>
>
> ---
> If your project is set up for it, you can reply to this email and have your
> reply appear on GitHub as well. If your project does not have this feature
> enabled and wishes so, or if the feature is enabled but not working, please
> contact infrastructure at [hidden email] or file a JIRA ticket
> with INFRA.
> ---
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>
Reply | Threaded
Open this post in threaded view
|

[GitHub] ant issue #18: JDK9 modules support for JUnitTask

bodewig
In reply to this post by bodewig
Github user bodewig commented on the issue:

    https://github.com/apache/ant/pull/18
 
    Have you seen my comments of "22 May", @tzezula ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] ant issue #18: JDK9 modules support for JUnitTask

bodewig
In reply to this post by bodewig
Github user tzezula commented on the issue:

    https://github.com/apache/ant/pull/18
 
    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](http://hg.openjdk.java.net/jdk9/dev/jdk/file/2624d54d0103/src/java.base/share/classes/java/lang/reflect/Layer.java). 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: b9183f95e5b543acd60eaea240c6c3fc74bae748
   
    The toLowerCase problem is fixed by: 1fc1ce108693bd52b9cf691b68dffb8f16343a42


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] ant issue #18: JDK9 modules support for JUnitTask

bodewig
In reply to this post by bodewig
Github user tzezula commented on the issue:

    https://github.com/apache/ant/pull/18
 
    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.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] ant issue #18: JDK9 modules support for JUnitTask

bodewig
In reply to this post by bodewig
Github user bodewig commented on the issue:

    https://github.com/apache/ant/pull/18
 
    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?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] ant issue #18: JDK9 modules support for JUnitTask

bodewig
In reply to this post by bodewig
Github user tzezula commented on the issue:

    https://github.com/apache/ant/pull/18
 
    OK, I will rebase to current master.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] ant issue #18: JDK9 modules support for JUnitTask

bodewig
In reply to this post by bodewig
Github user tzezula commented on the issue:

    https://github.com/apache/ant/pull/18
 
    Should be rebased. Let me know in case of any problems.
    Thanks!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] ant pull request #18: JDK9 modules support for JUnitTask

bodewig
In reply to this post by bodewig
Github user asfgit closed the pull request at:

    https://github.com/apache/ant/pull/18


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] ant issue #18: JDK9 modules support for JUnitTask

bodewig
In reply to this post by bodewig
Github user bodewig commented on the issue:

    https://github.com/apache/ant/pull/18
 
    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.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] ant issue #18: JDK9 modules support for JUnitTask

bodewig
In reply to this post by bodewig
Github user tzezula commented on the issue:

    https://github.com/apache/ant/pull/18
 
    Thanks a lot Stefan!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]