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

Alexandre (Shura) Iline alexandre.iline at oracle.com
Wed Mar 15 16:56:23 UTC 2017


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*@"
>> 
> 



More information about the jigsaw-dev mailing list