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

Reply via email to