I guess I was expecting to see only tests that use internal API updated but we do have some that do not being updated.
grep "@modules" jdk.patch | grep -v sun | more | wc 119 455 3341 + * @modules java.datatransfer + * @modules java.compiler + * @modules java.desktop And this simple grep is not catching the "jdk.compiler" tag usage nor printing out the multi-line cases such as test/java/beans/PropertyEditor/TestBooleanClassJava.java + * @modules java.compiler + * java.desktop + * jdk.compiler I can understand some of these - you are only putting "java.desktop" into the test properties files. But why do I still see so many cases where java.desktop is being listed without internal API usage ? grep "java.desktop" jdk.patch | grep -v sun | wc 96 264 2445 Trying to answer my own question it appears to be that the @modules" tag is "all or nothing" .. and that so java.desktop needs to be listed when some other module is listed. Is that correct ? If so then I'll give this an "OK" but really this is the sort of reason that you ought to have written here in the code review request so I don't have to do a bunch of research and guessing just to review your change. If you have sufficient reviewers *and* the answer to my questions are as I suppose, then you can push to jdk9-client, and preferably before 3 am PDT Tuesday (about 12 1/2 hours from now) so that we can get this into the next PIT cycle. -phil On 06/22/2015 08:44 AM, Alexander Kulyakhtin wrote:
Hi, Could you, please, review the test-only changes for the JDK-8076468 CR: JDK-8076468 "Add @modules to the tests in jdk_desktop test group" Webrev: http://cr.openjdk.java.net/~akulyakh/8076468/webrev.05/ @modules keywords have been added to the jtreg tests so that the tests can be selected as needed and have access to the restricted API when run with jake. The changes have been done by a script based on the jdeps output for every jtreg test. Best regards, Alexander