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