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 
<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.il...@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.ignat...@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*@"
> 

Reply via email to