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