RFR(L) : 8176176 : fix @modules in jdk_svc tests

[serguei.spitsyn at oracle.com]

Igor,

It was my guess but it is nice to have it explicitly explained.

Thanks!
Serguei



On 3/15/17 22:07, Igor Ignatyev wrote:
> Hi Serguei,
>
> 1s of all, thank you for your review.
>
> since these tests have dependencies on more modules than corresponding TEST.properties file declares, we have to specify @modules tag in them, and because @modules in a test overrides modules from TEST.properties, jdk.jdi module should be mentioned in the tests.
>
> — Igor
>   
>> On Mar 15, 2017, at 9:53 PM, serguei.spitsyn at oracle.com wrote:
>>
>> Hi Igor,
>>
>> This looks good to me.
>> A couple of questions below.
>>
>>
>> http://cr.openjdk.java.net/~iignatyev//8176176/webrev.01/test/com/sun/jdi/InvokeHangTest.java.udiff.html
>>
>> - * @modules jdk.jdi
>>   *  @library /test/lib
>> + * @modules java.management
>> + * jdk.jdi    Should the jdk.jdi be removed from this com/sun/jdi test?
>>
>>
>> http://cr.openjdk.java.net/~iignatyev//8176176/webrev.01/test/com/sun/jdi/RedefineCrossEvent.java.udiff.html
>>
>>   *  @modules java.corba
>>   *           jdk.jdi
>>
>>   Should the jdk.jdi be removed from this com/sun/jdi test?
>>
>>
>> Thanks,
>> Serguei
>>
>>
>> On 3/15/17 11:16, Igor Ignatyev wrote:
>>> Shura,
>>>
>>> Thank you for your review. I have update test/java/lang/management/PlatformLoggingMXBean/TEST.properties[1] and checked that there are no similar things in other TEST.properties files.
>>>
>>> Still looking for a review from a Reviewer.
>>>
>>> [1]
>>>> --- a/test/java/lang/management/PlatformLoggingMXBean/TEST.properties     Mon Mar 13 18:54:58 2017 -0700
>>>> +++ b/test/java/lang/management/PlatformLoggingMXBean/TEST.properties     Wed Mar 15 11:09:06 2017 -0700
>>>> @@ -1,3 +1,2 @@
>>>> -modules = java.management \
>>>> -          java.logging
>>>> +modules = java.logging
>>> Thanks,
>>> — Igor
>>>
>>>> On Mar 15, 2017, at 9:56 AM, Alexandre (Shura) Iline <alexandre.iline at oracle.com> wrote:
>>>>
>>>> Igor,
>>>>
>>>> I have looked through a bunch of tests where @modules is changed - that looks good.
>>>>
>>>> One minor thing I noticed is that, in test/java/lang/management/PlatformLoggingMXBean/TEST.properties you are explicitly calling out java.management. You do not have to do that because “modules” property is concatenated through the hierarchy.  java.management is already listed in test/java/lang/management/TEST.properties.
>>>>
>>>> Shura
>>>>
>>>>> On Mar 13, 2017, at 9:03 PM, Igor Ignatyev <igor.ignatyev at oracle.com> wrote:
>>>>>
>>>>> Shura,
>>>>>
>>>>> Thank you for review, I agree that having separate bugs is more convenient. [1] is new webrev w/ changes only in the files w/ incorrect module dependency declarations.
>>>>>
>>>>> [1] http://cr.openjdk.java.net/~iignatyev//8176176/webrev.01/index.html
>>>>>
>>>>> Thanks,
>>>>> — Igor
>>>>>
>>>>>> On Mar 9, 2017, at 5:02 PM, Alexandre (Shura) Iline <alexandre.iline at oracle.com> wrote:
>>>>>>
>>>>>> Igor,
>>>>>>
>>>>>> I have reviewed some number tests which change the @modules - they are fine.
>>>>>>
>>>>>> You, however, fix more things with this than missing module dependency declaration. There is a redesign of line-number-sensitive tests [1] and other multiple improvements such as in [2]. Would it be more convenient to have that as separate bugs?
>>>>>>
>>>>>> Shura
>>>>>>
>>>>>> [1] http://cr.openjdk.java.net/~iignatyev/8176176/webrev.00/test/com/sun/jdi/ArgumentValuesTest.java.sdiff.html
>>>>>> [2] http://cr.openjdk.java.net/~iignatyev/8176176/webrev.00/test/com/sun/jdi/ArrayLengthDumpTest.sh.sdiff.html
>>>>>>
>>>>>>> On Mar 7, 2017, at 1:07 PM, Igor Ignatyev <igor.ignatyev at oracle.com> wrote:
>>>>>>>
>>>>>>> http://cr.openjdk.java.net/~iignatyev/8176176/webrev.00/index.html
>>>>>>>> 2586 lines changed: 669 ins; 484 del; 1433 mod;
>>>>>>> Hi all,
>>>>>>>
>>>>>>> Could you please review this changeset which fix @modules dependency declaration in jdk_svc tests?
>>>>>>> there are a couple issues w/ modules in jdk_svc tests:
>>>>>>> - some tests do not specify modules which they depend on
>>>>>>> - modules in TEST.properties is not used in cases there all tests (should) have the same @modules directive
>>>>>>> - @modules directive isn't placed according to current convention (before the 1st run directive)
>>>>>>>
>>>>>>> Since this fix has already touched lots of tests, I have decided to use this opportunity and reordered some of jtreg tags as well, so there won’t be two massive updates in the tests.
>>>>>>>
>>>>>>> Some of our tests are line number sensitive, and then I fixed jtreg declaration, they started to fail. It was really hard to find our all line number sensitive tests, so I have unified the way we declare that as a part of this fix. Please let me know if you prefer to have it done separately.
>>>>>>>
>>>>>>> There are two one-liners which, I hope, can simplify review:
>>>>>>> [1] shows only the changes which are not in comments. Besides obvious new added TEST.properties, there are changes in the following line number sensitive tests (which I mentioned before):
>>>>>>> test/com/sun/jdi/ArgumentValuesTest.java
>>>>>>> test/com/sun/jdi/BreakpointTest.java
>>>>>>> test/com/sun/jdi/FetchLocals.java
>>>>>>> test/com/sun/jdi/GetLocalVariables.java
>>>>>>> test/com/sun/jdi/GetSetLocalTest.java
>>>>>>> test/com/sun/jdi/LambdaBreakpointTest.java
>>>>>>> test/com/sun/jdi/LineNumberOnBraceTest.java
>>>>>>> test/com/sun/jdi/PopAndStepTest.java
>>>>>>>
>>>>>>> [2] shows changes in jtreg tags, it can help to see that almost all changes in jtreg tags are either moving of tags which does not affect execution order or @modules changes.
>>>>>>>
>>>>>>> webrev: http://cr.openjdk.java.net/~iignatyev/8176176/webrev.00/index.html
>>>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8176176
>>>>>>> Testing:
>>>>>>> - jdk_svc on linux, windows, mac
>>>>>>> - checked that all tests which could be executed with full jdk before still can be executed with full jdk
>>>>>>>
>>>>>>> Thanks,
>>>>>>> — Igor
>>>>>>>
>>>>>>> [1] $ hg diff -w | grep "^[+-]" | grep -v "^[+-]\s*[*#]"
>>>>>>> [2] $ hg diff -w | grep -e "^\(+++ b\)\|\(--- a\)" -e "^[+-]\s*[*#]\s*@"