Re: RFR(XS): 8062938: sun/jvmstat/monitor/MonitoredVm/CR6672135.java: java.lang.IllegalArgumentException: Could not map vmid to user name
Jaroslav, thanks for the review! // Katja On 07/24/2015 05:02 PM, Jaroslav Bachorik wrote: Thumbs up! -JB- On 24.7.2015 16:06, Yekaterina Kantserova wrote: Hi, Could I please have a review of this fix. bug: https://bugs.openjdk.java.net/browse/JDK-8062938 webrev: http://cr.openjdk.java.net/~ykantser/8062938/webrev.00 As David pointed out [1] the test is trying to monitor the first best VM from localHost.activeVms() which can result in various errors (https://bugs.openjdk.java.net/browse/JDK-8064572, https://bugs.openjdk.java.net/browse/JDK-8060736). The solution is to start a test VM and operate on it. In order to test the fix the test has been run on all supported platforms. Thanks, Katja [1] David Holmes https://bugs.openjdk.java.net/secure/ViewProfile.jspa?name=dholmes added a comment - 2014-11-07 12:39 My theory is that this test finds all VMs running on the machine, but some of those are not accessible to it - perhaps even running under a different user.
RFR(XS): 8062938: sun/jvmstat/monitor/MonitoredVm/CR6672135.java: java.lang.IllegalArgumentException: Could not map vmid to user name
Hi, Could I please have a review of this fix. bug: https://bugs.openjdk.java.net/browse/JDK-8062938 webrev: http://cr.openjdk.java.net/~ykantser/8062938/webrev.00 As David pointed out [1] the test is trying to monitor the first best VM from localHost.activeVms() which can result in various errors (https://bugs.openjdk.java.net/browse/JDK-8064572, https://bugs.openjdk.java.net/browse/JDK-8060736). The solution is to start a test VM and operate on it. In order to test the fix the test has been run on all supported platforms. Thanks, Katja [1] David Holmes https://bugs.openjdk.java.net/secure/ViewProfile.jspa?name=dholmes added a comment - 2014-11-07 12:39 My theory is that this test finds all VMs running on the machine, but some of those are not accessible to it - perhaps even running under a different user.
Re: RFR(XS): 8132094: Mark intermittently failuring core-svc tests
Hi, In order to test the fix I ran the svc_tools, jdk_management, jdk_jmx and jdk_jdi tests on all of the core platforms. // Katja On 07/23/2015 03:35 PM, Daniel D. Daugherty wrote: What testing of these changes was done? Dan On 7/22/15 7:50 AM, Yekaterina Kantserova wrote: Hi, Could I please have a review of this fix. bug: https://bugs.openjdk.java.net/browse/JDK-8132094 webrev: http://cr.openjdk.java.net/~ykantser/8132094/webrev.00 Thanks, Katja
Re: RFR(XS): 8132094: Mark intermittently failuring core-svc tests
Jaroslav, Serguei, thanks for the review! // Katja On 07/23/2015 12:09 PM, serguei.spit...@oracle.com wrote: Looks good. Thanks, Serguei On 7/22/15 6:50 AM, Yekaterina Kantserova wrote: Hi, Could I please have a review of this fix. bug: https://bugs.openjdk.java.net/browse/JDK-8132094 webrev: http://cr.openjdk.java.net/~ykantser/8132094/webrev.00 Thanks, Katja
RFR(XS): 8132094: Mark intermittently failuring core-svc tests
Hi, Could I please have a review of this fix. bug: https://bugs.openjdk.java.net/browse/JDK-8132094 webrev: http://cr.openjdk.java.net/~ykantser/8132094/webrev.00 Thanks, Katja
RFR(XXS): 8037957: [TEST_BUG] javax/management/mxbean/LeakTest.java misses MerlinMXBean TigerMXBean in @run build tag
Hi, Could I please have a review of this small fix. bug: https://bugs.openjdk.java.net/browse/JDK-8037957 webrev: http://cr.openjdk.java.net/~ykantser/8037957/webrev.00 Thanks, Katja
RFR(XS): 8075658: Mark intermittently failuring core-svc tests
Hi, Could I please have a review of this fix. bug: https://bugs.openjdk.java.net/browse/JDK-8075658 webrev: http://cr.openjdk.java.net/~ykantser/8075658/webrev.00 Thanks, Katja
Re: RFR(XXS): 8037957: [TEST_BUG] javax/management/mxbean/LeakTest.java misses MerlinMXBean TigerMXBean in @run build tag
Thanks Jaroslav! // Katja On 07/17/2015 02:01 PM, Jaroslav Bachorik wrote: Looks good! -JB- On 17.7.2015 13:59, Yekaterina Kantserova wrote: Hi, Could I please have a review of this small fix. bug: https://bugs.openjdk.java.net/browse/JDK-8037957 webrev: http://cr.openjdk.java.net/~ykantser/8037957/webrev.00 Thanks, Katja
Re: RFR: JDK-8114828: wrong class file error when compiling tests
Looks good! (Not a reviewer) I've switched the issue to be open. // Katja On 07/15/2015 06:57 PM, Alexander Kulyakhtin wrote: Hi, Could you, please, review these simple test-only changes: CR: https://bugs.openjdk.java.net/browse/JDK-8114828 wrong class file error when compiling tests (All the changed files belong to the Open JDK. Since the CR was submitted as confidential, the issue is described in this mail below) Webrev: http://cr.openjdk.java.net/~akulyakh/8114828/index.html Before the change the @library tags in the tests pointed to the directory above the tests repository. With the jtreg b12 and above this is not allowed. Instead, the external.lib.roots property should be used to specify any additional roots. The tests have been modified accordingly, so they now compile successfully. Best regards, Alexander
Re: RFR(S): JDK-8129971 TestStackTrace.java: ArrayIndexOutOfBoundsException thrown by AARCH64ThreadContext.setRegister
Looks good! Thanks! // Katja On 07/15/2015 04:25 PM, Dmitry Samersoff wrote: Katja, done. Webrev updated in-place (press shift-reload) -Dmitry On 2015-07-15 15:16, Yekaterina Kantserova wrote: Dmitry, could you please remove @ignore tag in TestStackTrace.java as well? // Katja On 07/15/2015 01:52 PM, Dmitry Samersoff wrote: Everybody, Please, review the fix: http://cr.openjdk.java.net/~dsamersoff/JDK-8129971/webrev.01/ Added one more register counted in frame context. -Dmitry
Re: RFR(XS): 8076471: Remove hprof agent tests in JDK
David, thanks for the review! Yes, I will. // Katja On 07/15/2015 11:31 AM, David Holmes wrote: Hi Katja, On 15/07/2015 7:22 PM, Yekaterina Kantserova wrote: Hi, I've forgotten to remove serviceability/hprof/cpu002.java test in hotspot repo. Could I please have a review for this change? webrev: http://cr.openjdk.java.net/~ykantser/8076471.hotspot/webrev.00 Reviewed. As this is in a separate repo to the changesets already pushed as part of this bug you can push with the same bug number. But make sure you check with the hs-rt gatekeeper to ensure it is included when the changes are pushed to hs. Otherwise it will need a new CR. Thanks, David Thanks, Katja On 07/10/2015 02:55 PM, Yekaterina Kantserova wrote: Hi, Could I please have a review of this fix. bug: https://bugs.openjdk.java.net/browse/JDK-8076471 webrev: http://cr.openjdk.java.net/~ykantser/8076471/webrev.00 More details can be found in JEP 240: Remove the JVM TI hprof Agent (https://bugs.openjdk.java.net/browse/JDK-8046661). Thanks, Katja
Re: RFR(XS): 8076471: Remove hprof agent tests in JDK
Hi, I've forgotten to remove serviceability/hprof/cpu002.java test in hotspot repo. Could I please have a review for this change? webrev: http://cr.openjdk.java.net/~ykantser/8076471.hotspot/webrev.00 Thanks, Katja On 07/10/2015 02:55 PM, Yekaterina Kantserova wrote: Hi, Could I please have a review of this fix. bug: https://bugs.openjdk.java.net/browse/JDK-8076471 webrev: http://cr.openjdk.java.net/~ykantser/8076471/webrev.00 More details can be found in JEP 240: Remove the JVM TI hprof Agent (https://bugs.openjdk.java.net/browse/JDK-8046661). Thanks, Katja
Re: RFR(XS): 8076471: Remove hprof agent tests in JDK
Hi Serguei, Thanks a lot for looking at it! Yes, I unintentionally removed test/demo/jvmti instead of test/demo/jvmti/hprof :( Will restore the tests asap. // Katja On 07/15/2015 11:58 AM, serguei.spit...@oracle.com wrote: Hi Katya, On 7/15/15 2:22 AM, Yekaterina Kantserova wrote: Hi, I've forgotten to remove serviceability/hprof/cpu002.java test in hotspot repo. Could I please have a review for this change? webrev: http://cr.openjdk.java.net/~ykantser/8076471.hotspot/webrev.00 This looks good. Thanks, Katja On 07/10/2015 02:55 PM, Yekaterina Kantserova wrote: Hi, Could I please have a review of this fix. bug: https://bugs.openjdk.java.net/browse/JDK-8076471 webrev: http://cr.openjdk.java.net/~ykantser/8076471/webrev.00 It looks good but I'm not sure why have you deleted these tests as well: |-- -- -- -- Old http://cr.openjdk.java.net/%7Eykantser/8076471/webrev.00/test/demo/jvmti/minst/MinstExample.java-.html --- - --- | *test/demo/jvmti/minst/MinstExample.java* |-- -- -- -- Old http://cr.openjdk.java.net/%7Eykantser/8076471/webrev.00/test/demo/jvmti/minst/MinstTest.java-.html --- - --- | *test/demo/jvmti/minst/MinstTest.java* |-- -- -- -- Old http://cr.openjdk.java.net/%7Eykantser/8076471/webrev.00/test/demo/jvmti/versionCheck/FailsWhenJvmtiVersionDiffers.java-.html --- - --- | *test/demo/jvmti/versionCheck/FailsWhenJvmtiVersionDiffers.java* |-- -- -- -- Old http://cr.openjdk.java.net/%7Eykantser/8076471/webrev.00/test/demo/jvmti/waiters/WaitersTest.java-.html --- - --- | *test/demo/jvmti/waiters/WaitersTest.java* Did you want to remove all the demo tests, not hprof only? Just want to make sure you did not get rid of them unintentionally. Thanks, Serguei More details can be found in JEP 240: Remove the JVM TI hprof Agent (https://bugs.openjdk.java.net/browse/JDK-8046661). Thanks, Katja
Re: RFR(S): JDK-8129971 TestStackTrace.java: ArrayIndexOutOfBoundsException thrown by AARCH64ThreadContext.setRegister
Dmitry, could you please remove @ignore tag in TestStackTrace.java as well? // Katja On 07/15/2015 01:52 PM, Dmitry Samersoff wrote: Everybody, Please, review the fix: http://cr.openjdk.java.net/~dsamersoff/JDK-8129971/webrev.01/ Added one more register counted in frame context. -Dmitry
RFR(XS): 8131328: Restore demo/jvmti tests
Hi, Could I please have a review of this fix. demo/jvmti tests were unintentionally removed by https://bugs.openjdk.java.net/browse/JDK-8076471. This fix will restore the tests unrelated to hprof. bug: https://bugs.openjdk.java.net/browse/JDK-8131328 webrev: http://cr.openjdk.java.net/~ykantser/8131328/webrev.00 Thanks, Katja
Re: RFR(XXS): 8131035: [TESTBUG] sun/management/HotspotRuntimeMBean/GetTotalSafepointTime.java needs to enable UsePerfData
Thanks David! // Katja On 07/14/2015 06:19 AM, David Holmes wrote: Ship it! :) Thanks, David On 13/07/2015 11:40 PM, Yekaterina Kantserova wrote: Hi, Could I please have a review of this very small fix. bug: https://bugs.openjdk.java.net/browse/JDK-8131035 webrev: http://cr.openjdk.java.net/~ykantser/8131035/webrev.00 Thanks, Katja
Re: RFR(XXS): 8130057: serviceability/sa/TestStackTrace.java should be quarantined
Thanks for the review! // Katja On 07/14/2015 12:13 PM, Erik Gahlin wrote: Looks good Erik Yekaterina Kantserova skrev den 14/07/15 11:44: Hi, Could I please have a review of this very small fix. bug: https://bugs.openjdk.java.net/browse/JDK-8130057 webrev: http://cr.openjdk.java.net/~ykantser/8130057/webrev.00/ Thanks, Katja
Re: RFR(XXS): 8131035: [TESTBUG] sun/management/HotspotRuntimeMBean/GetTotalSafepointTime.java needs to enable UsePerfData
Jaroslav, Thanks for the review! Yes, removing the 'author' tag was intentional. My understanding has been it's good to avoid 'author' tag. // Katja On 07/13/2015 03:51 PM, Jaroslav Bachorik wrote: Looks good! (btw, removing the 'author' tag was intentional?) -JB- On 13.7.2015 15:40, Yekaterina Kantserova wrote: Hi, Could I please have a review of this very small fix. bug: https://bugs.openjdk.java.net/browse/JDK-8131035 webrev: http://cr.openjdk.java.net/~ykantser/8131035/webrev.00 Thanks, Katja
RFR(XXS): 8131035: [TESTBUG] sun/management/HotspotRuntimeMBean/GetTotalSafepointTime.java needs to enable UsePerfData
Hi, Could I please have a review of this very small fix. bug: https://bugs.openjdk.java.net/browse/JDK-8131035 webrev: http://cr.openjdk.java.net/~ykantser/8131035/webrev.00 Thanks, Katja
RFR(XS): 8076471: Remove hprof agent tests in JDK
Hi, Could I please have a review of this fix. bug: https://bugs.openjdk.java.net/browse/JDK-8076471 webrev: http://cr.openjdk.java.net/~ykantser/8076471/webrev.00 More details can be found in JEP 240: Remove the JVM TI hprof Agent (https://bugs.openjdk.java.net/browse/JDK-8046661). Thanks, Katja
RFR(XSS): 8032763: Remove use of sun.misc.Ref from hprof parser in testlibrary
Hi, Could I please have a review of this very small fix. bug: https://bugs.openjdk.java.net/browse/JDK-8032763 webrev: http://cr.openjdk.java.net/~ykantser/8032763/webrev.00 Thanks, Katja
Re: RFR(XSS): 8032763: Remove use of sun.misc.Ref from hprof parser in testlibrary
Jaroslav, Alan, thanks for your reviews! // Katja On 07/09/2015 12:41 PM, Jaroslav Bachorik wrote: Looks good! -JB- On 9.7.2015 12:39, Yekaterina Kantserova wrote: Hi, Could I please have a review of this very small fix. bug: https://bugs.openjdk.java.net/browse/JDK-8032763 webrev: http://cr.openjdk.java.net/~ykantser/8032763/webrev.00 Thanks, Katja
Re: RFR(S): 8085813: The targeted processes in sun/tools tests should be launched with -XX:+UsePerfData flag in order to work on embedded platforms
Serguei, Thank you for the review! // Katja On 06/08/2015 10:05 PM, serguei.spit...@oracle.com wrote: It looks good. Thanks, Serguei On 6/8/15 4:21 AM, Yekaterina Kantserova wrote: Hi, Could I please have a review of this small fix. bug: https://bugs.openjdk.java.net/browse/JDK-8085813 webrev hotspot: http://cr.openjdk.java.net/~ykantser/8085813.hotspot/webrev.00/ webrev jdk: http://cr.openjdk.java.net/~ykantser/8085813.jdk/webrev.00/ Thanks, Katja
RFR(S): 8085813: The targeted processes in sun/tools tests should be launched with -XX:+UsePerfData flag in order to work on embedded platforms
Hi, Could I please have a review of this small fix. bug: https://bugs.openjdk.java.net/browse/JDK-8085813 webrev hotspot: http://cr.openjdk.java.net/~ykantser/8085813.hotspot/webrev.00/ webrev jdk: http://cr.openjdk.java.net/~ykantser/8085813.jdk/webrev.00/ Thanks, Katja
RFR(XS): 8085973: The targeted processes in javax/management tests should be launched with -XX:+UsePerfData flag in order to work on embedded platforms
Hi, Could I please have a review of this small fix. bug: https://bugs.openjdk.java.net/browse/JDK-8085973 webrev: http://cr.openjdk.java.net/~ykantser/8085973/webrev.00/ Thanks, Katja
Re: RFR(S): 8081037: serviceability/sa/ tests time out on Windows
Hi, due to https://bugs.openjdk.java.net/browse/JDK-8081381 I wasn't able to push this fix. The problem is LingeredApp.java contains JDK 9 feature java.lang.Process.getPid() but the test library is compiled with JDK 8 today. This issue is not trivial to solve so I suggest a temporary fix to test/lib/Makefile. webrev root: http://cr.openjdk.java.net/~ykantser/8081037/webrev.01 Thanks, Katja On 05/27/2015 03:02 PM, Yekaterina Kantserova wrote: Hi, Could I please have a review of this fix. bug: https://bugs.openjdk.java.net/browse/JDK-8081037 webrev root: http://cr.openjdk.java.net/~ykantser/8081037/webrev.00 webrev jdk: http://cr.openjdk.java.net/~ykantser/8081037.jdk/webrev.00 webrev hotspot: http://cr.openjdk.java.net/~ykantser/8081037.hotspot/webrev.00 From the bug: The problem is most likely that SA will pause the target process while it is running. In this case, the target process is the same as the process that launched SA. That process is also handling the output from SA over a pipe, but when that pipe fills up the process cannot empty it and the SA process is blocked because it cannot write any more output. Deadlock. The solutions is to start a separate target process. Dmitry Samersoff has already created a test application for such cases so I've decided to move it on the top level library instead of duplicating it. The test application will reside under test/lib/share/classes/jdk/test/lib/apps and the test under test/lib-test/jdk/test/lib/apps. Thanks, Katja
Re: RFR(XS): 8080828: Create sanity test for JDK-8080155
Staffan, thanks for the review!
On 05/21/2015 01:04 PM, Staffan Larsen wrote:
Looks good!
Thanks,
/Staffan
On 21 maj 2015, at 12:55, Yekaterina Kantserova
yekaterina.kantser...@oracle.com wrote:
The new webrev can be found here:
http://cr.openjdk.java.net/~ykantser/8080828/webrev.01
Thanks,
Katja
On 05/21/2015 10:15 AM, Staffan Larsen wrote:
I just realize that you also need to add a check if SA can attach on the
current platform. See the test in sa/jmap-hashcode:
if (!Platform.shouldSAAttach()) {
System.out.println(SA attach not expected to work - test skipped.);
return;
}
Thanks,
/Staffan
On 21 maj 2015, at 10:13, Staffan Larsen staffan.lar...@oracle.com wrote:
Can we add some kind of validation of the output? Something simple, just to
verify that we actually ran what we think we ran.
On 21 maj 2015, at 09:33, Yekaterina Kantserova
yekaterina.kantser...@oracle.com wrote:
Hi,
Could I please have a review of this new test.
bug: https://bugs.openjdk.java.net/browse/JDK-8080828
webrev: http://cr.openjdk.java.net/~ykantser/8080828/webrev.00
This test will trigger the exception reported in
https://bugs.openjdk.java.net/browse/JDK-8080155 if running with -Xcomp.
Thanks,
Katja
8080855: Create sanity test for JDK-8080692
Hi, Could I please have a review of this new test. bug: https://bugs.openjdk.java.net/browse/JDK-8080855 webrev: http://cr.openjdk.java.net/~ykantser/8080855/webrev.00/ This test will trigger the exception reported in https://bugs.openjdk.java.net/browse/JDK-8080692 if running with -Xcomp. Thanks, Katja
RFR(XS): 8080828: Create sanity test for JDK-8080155
Hi, Could I please have a review of this new test. bug: https://bugs.openjdk.java.net/browse/JDK-8080828 webrev: http://cr.openjdk.java.net/~ykantser/8080828/webrev.00 This test will trigger the exception reported in https://bugs.openjdk.java.net/browse/JDK-8080155 if running with -Xcomp. Thanks, Katja
Re: RFR(XS): 8080828: Create sanity test for JDK-8080155
Staffan, thanks fro looking at it! I'll fix according the test your
comments.
// Katja
On 05/21/2015 10:15 AM, Staffan Larsen wrote:
I just realize that you also need to add a check if SA can attach on the
current platform. See the test in sa/jmap-hashcode:
if (!Platform.shouldSAAttach()) {
System.out.println(SA attach not expected to work - test skipped.);
return;
}
Thanks,
/Staffan
On 21 maj 2015, at 10:13, Staffan Larsen staffan.lar...@oracle.com wrote:
Can we add some kind of validation of the output? Something simple, just to
verify that we actually ran what we think we ran.
On 21 maj 2015, at 09:33, Yekaterina Kantserova
yekaterina.kantser...@oracle.com wrote:
Hi,
Could I please have a review of this new test.
bug: https://bugs.openjdk.java.net/browse/JDK-8080828
webrev: http://cr.openjdk.java.net/~ykantser/8080828/webrev.00
This test will trigger the exception reported in
https://bugs.openjdk.java.net/browse/JDK-8080155 if running with -Xcomp.
Thanks,
Katja
Re: RFR(XS): 8080828: Create sanity test for JDK-8080155
The new webrev can be found here:
http://cr.openjdk.java.net/~ykantser/8080828/webrev.01
Thanks,
Katja
On 05/21/2015 10:15 AM, Staffan Larsen wrote:
I just realize that you also need to add a check if SA can attach on the
current platform. See the test in sa/jmap-hashcode:
if (!Platform.shouldSAAttach()) {
System.out.println(SA attach not expected to work - test skipped.);
return;
}
Thanks,
/Staffan
On 21 maj 2015, at 10:13, Staffan Larsen staffan.lar...@oracle.com wrote:
Can we add some kind of validation of the output? Something simple, just to
verify that we actually ran what we think we ran.
On 21 maj 2015, at 09:33, Yekaterina Kantserova
yekaterina.kantser...@oracle.com wrote:
Hi,
Could I please have a review of this new test.
bug: https://bugs.openjdk.java.net/browse/JDK-8080828
webrev: http://cr.openjdk.java.net/~ykantser/8080828/webrev.00
This test will trigger the exception reported in
https://bugs.openjdk.java.net/browse/JDK-8080155 if running with -Xcomp.
Thanks,
Katja
Re: RFR(S): 6755586: Test com/sun/jdi/NoLaunchOptionTest.java may erroneously fail
Staffan, Dmitry, thank you for the reviews! // Katja On 05/07/2015 10:40 PM, Dmitry Samersoff wrote: Looks good for me! -Dmitry On 2015-05-07 18:04, Yekaterina Kantserova wrote: Hi, Could I please have a review of this small fix. bug: https://bugs.openjdk.java.net/browse/JDK-6755586 webrev: http://cr.openjdk.java.net/~ykantser/6755586/webrev.00/ This bug is actually already solved. But I'll use it as an opportunity to re-write the test using test library. There is no need to retrieve a free port since the error occurs before binding to the address. Thanks, Katja
8079200: Fix heapdump tests to validate heapdump after jhat is removed
Hi, Could I please have a review of this fix. bug: https://bugs.openjdk.java.net/browse/JDK-8079200 webrev: http://cr.openjdk.java.net/~ykantser/8079200/webrev.00 The fix makes sure the HprofParser is available for all types of test frameworks, not only JTreg. It will be a part of test-lib.jar. Thanks, Katja
Re: RFR(XS): 8076998: BadHandshakeTest.java fails due to warnings in output
Thanks Serguei! On 05/06/2015 12:48 AM, serguei.spit...@oracle.com wrote: Looks good. Thanks, Serguei On 5/5/15 6:24 AM, Yekaterina Kantserova wrote: Hi, Could I please have a review of this fix. bug: https://bugs.openjdk.java.net/browse/JDK-8076998 webrev: http://cr.openjdk.java.net/~ykantser/8076998/webrev.00/ The test has been checking the first line of output for either 'Listening for transport dt_socket at address:' or 'Address already in use'. But different VM warnings can show up before these lines. The fix is to read output until one of these lines is found. Thanks, Katja
Re: RFR(M): 8078896: Add @modules as needed to the jdk_svc tests
Mandy, Thanks fro your review! Please see my comment inlined. On 05/05/2015 11:00 PM, Mandy Chung wrote: com.sun.management has been moved to jdk.management module. The patch for JDK-8042901 is just integrated in jdk9/dev today. Most, if not all, test/com/sun/management tests need updates to require @modules jdk.management. There are several test/java/lang/management tests dependng on both java.lang.management and com.sun.management. For tests only using java.lang.management API, @modules java.management is adequate. test/com/sun/jdi/InterfaceMethodsTest.java The copyright header contains already 2015. test/com/sun/jdi/InterruptHangTest.java The test has been missing copyrights, I've updated it with the copyright header. Since you have updated the end year of the copyright header in most tests, I spot a couple and so mention it to you. Otherwise looks good. Thanks for doing this. The new webrev can be found here: http://cr.openjdk.java.net/~ykantser/8078896/webrev.02/ About the test selection, one typical aspect of svc tests is to run a j* tool in a child process (e.g. jinfo, jstack, jstat, jstatd,jcmd, jps etc that are in jdk.jcmd module). I would expect all test/sun/tools/jcmd tests should have @modules jdk.jcmd and similiar for other sun/tools/j* tests. Are you thinking to come back in the next round of test selection update? As Alan pointed in his answer to this mail JTreg is not ready yet. I prefer to do it when it will be possible to test this change. Best regards, Katja Mandy
RFR(XS): 8076998: BadHandshakeTest.java fails due to warnings in output
Hi, Could I please have a review of this fix. bug: https://bugs.openjdk.java.net/browse/JDK-8076998 webrev: http://cr.openjdk.java.net/~ykantser/8076998/webrev.00/ The test has been checking the first line of output for either 'Listening for transport dt_socket at address:' or 'Address already in use'. But different VM warnings can show up before these lines. The fix is to read output until one of these lines is found. Thanks, Katja
Re: RFR(M): 8078896: Add @modules as needed to the jdk_svc tests
Alan, Thanks for the review! And for the catch - I'll fix it. // Katja On 05/05/2015 03:30 PM, Alan Bateman wrote: Thanks Katja, this looks good. One thing that we should as part of this is rev the requiredVersion in jdk/test/TEST.ROOT in case people are using older versions of jtreg. I think you did that as part of the patch for the hotspot repo. -Alan.
Re: RFR(XS): 8076998: BadHandshakeTest.java fails due to warnings in output
Thanks! // Katja On 05/05/2015 03:30 PM, Staffan Larsen wrote: Looks good! Thanks, /Staffan On 5 maj 2015, at 15:24, Yekaterina Kantserova yekaterina.kantser...@oracle.com wrote: Hi, Could I please have a review of this fix. bug: https://bugs.openjdk.java.net/browse/JDK-8076998 webrev: http://cr.openjdk.java.net/~ykantser/8076998/webrev.00/ The test has been checking the first line of output for either 'Listening for transport dt_socket at address:' or 'Address already in use'. But different VM warnings can show up before these lines. The fix is to read output until one of these lines is found. Thanks, Katja
RFR(M): 8078896: Add @modules as needed to the jdk_svc tests
Hi, Could I please have a review of this fix. bug: https://bugs.openjdk.java.net/browse/JDK-8078896 webrev: http://cr.openjdk.java.net/~ykantser/8078896 The push will be pushed to jdk9/dev. Thanks, Katja Alan's clarification from same change in hotspot (RFR: JDK-8075586: add @modules as needed to the open hotspot tests) == Just to put more context on this patch and similar patches that will be proposed for tests in other repos. The new @modules tag can be used for test selection (e.g. no point running a test that make use of types in module java.management if the runtime under test does not have the management module), and additionally to declare a dependency on JDK-internal APIs. If my test uses sun.misc.Unsafe for example then the runtime under test needs to export that API for the test to compile and run. Clearly the latter requires the module system in JDK 9 so consider this part as just preparing the tests for when that day comes. One other thing to say to say is that Alex has tooling to examine the test tree and create or change the @modules tag as needed. The intention is that this should run periodically to refresh the @modules tags. In this period before the module system then we can't expect everyone that is adding or changing tests to have internalized the module graph [1] and furthermore know which APIs are exported or not. In other words, the intention is not to needlessly burden anyone that adds or changes tests. Clearly making use of a new jtreg tag means everyone with a local copy of jtreg should grab a recent build. Finally, I hope in time that the TEST.groups files can be cleaned up to remove the needs_* groups, this is important for the group definitions in the jdk repo mostly (but there are some in the hotspot TEST.groups too). -Alan
RFR(XXS): 8079120: serviceability/dcmd/gc/HeapDumpAllTest.java: compilation failed
Hi, Could I please have a review of this very small fix. bug: https://bugs.openjdk.java.net/browse/JDK-8079120 webrev: http://cr.openjdk.java.net/~ykantser/8079120/webrev.00/ Thanks, Katja
Re: RFR(M): 8059047: Extract parser/validator from jhat for use in tests
Staffan, Thanks for the review! // Katja On 04/28/2015 02:05 PM, Staffan Larsen wrote: Looks good! Thanks, /Staffan On 24 apr 2015, at 16:17, Yekaterina Kantserova yekaterina.kantser...@oracle.com mailto:yekaterina.kantser...@oracle.com wrote: All suggestions have been implemented. Please find new webrevs here: webrev root: http://cr.openjdk.java.net/~ykantser/8059047/webrev.02 webrev jdk: http://cr.openjdk.java.net/~ykantser/8059047.jdk/webrev.02 webrev hotspot: http://cr.openjdk.java.net/~ykantser/8059047.hotspot/webrev.01 // Katja On 04/24/2015 12:10 PM, Staffan Larsen wrote: On 24 apr 2015, at 11:34, Yekaterina Kantserova yekaterina.kantser...@oracle.com mailto:yekaterina.kantser...@oracle.com wrote: Hi, Here comes the updated version. bug: https://bugs.openjdk.java.net/browse/JDK-8059047 webrev root: http://cr.openjdk.java.net/~ykantser/8059047/webrev.01/ http://cr.openjdk.java.net/%7Eykantser/8059047/webrev.01/ webrev jdk: http://cr.openjdk.java.net/~ykantser/8059047.jdk/webrev.01/ http://cr.openjdk.java.net/%7Eykantser/8059047.jdk/webrev.01/ test/com/sun/management/HotSpotDiagnosticMXBean/DumpHeap.java: In main() I think you should change the deleteOnExit() to just delete(). There is no reason to wait with it. Perhaps we should also verify that the file does not already exists before we try to write to it. If it exists, we can delete it. webrev hotspot: http://cr.openjdk.java.net/~ykantser/8059047.hotspot/webrev.00/ http://cr.openjdk.java.net/%7Eykantser/8059047.hotspot/webrev.00/ test/serviceability/dcmd/gc/HeapDumpTest.java: Same thing: call delete() instead of deleteOnExit(). One comment about changes in hotspot part. The refactored version of serviceability/dcmd/gc/HeapDumpTest.java doesn't contain check: 70 /* 71 * Some hprof dumps of all objects contain constantPoolOop references that cannot be resolved, so we ignore 72 * failures about resolving constantPoolOop fields using a negative lookahead 73 */ 74 output.shouldNotMatch(.*WARNING(?!.*Failed to resolve object.*constantPoolOop.*).*); It depends on that the current version of jdk.test.lib.hprof parser simply write down warnings to stdout. As a result the test needs to invent own logic to parse it. I suggest instead to improve jdk.test.lib.hprof parser as a separate RFE. The parser will collect such information and provide a new method for getting it, e.g. jdk.test.lib.hprof.model.Snapshot.getWarnings(). The serviceability/dcmd/gc/HeapDumpTest.java will be changed accordingly when RFE is implemented. Sounds right. A quick, hacky solution is to redirect System.out to a stream or file while running the parser… /Staffan Thanks, Katja On 04/22/2015 03:09 PM, Staffan Larsen wrote: On 22 apr 2015, at 11:17, Yekaterina Kantserovayekaterina.kantser...@oracle.com mailto:yekaterina.kantser...@oracle.com wrote: Hi, Could I please have a review of this fix. bug:https://bugs.openjdk.java.net/browse/JDK-8059047 http://bugs.openjdk.java.net/browse/JDK-8059047 webrev:http://cr.openjdk.java.net/~ykantser/8059047/webrev.00/ http://cr.openjdk.java.net/%7Eykantser/8059047/webrev.00/ This fix is a part of JEP 241: Remove the jhat Tool (https://bugs.openjdk.java.net/browse/JDK-8059039). I suggest to put parser/validator into common test library since the functionality can be useful not only for SVC tools tests but even for some future GC tests. The old jhat packages have been moved as follows: com.sun.tools.hat.internal.model - jdk.test.lib.hprof.model com.sun.tools.hat.internal.parser - jdk.test.lib.hprof.parser com.sun.tools.hat.internal.util - jdk.test.lib.hprof.util The source has not been changed except Copyrights year. Thanks, Katja
Re: RFR(M): 8059047: Extract parser/validator from jhat for use in tests
Hi, Here comes the updated version. bug: https://bugs.openjdk.java.net/browse/JDK-8059047 webrev root: http://cr.openjdk.java.net/~ykantser/8059047/webrev.01/ webrev jdk: http://cr.openjdk.java.net/~ykantser/8059047.jdk/webrev.01/ webrev hotspot: http://cr.openjdk.java.net/~ykantser/8059047.hotspot/webrev.00/ One comment about changes in hotspot part. The refactored version of serviceability/dcmd/gc/HeapDumpTest.java doesn't contain check: 70 /* 71 * Some hprof dumps of all objects contain constantPoolOop references that cannot be resolved, so we ignore 72 * failures about resolving constantPoolOop fields using a negative lookahead 73 */ 74 output.shouldNotMatch(.*WARNING(?!.*Failed to resolve object.*constantPoolOop.*).*); It depends on that the current version of jdk.test.lib.hprof parser simply write down warnings to stdout. As a result the test needs to invent own logic to parse it. I suggest instead to improve jdk.test.lib.hprof parser as a separate RFE. The parser will collect such information and provide a new method for getting it, e.g. jdk.test.lib.hprof.model.Snapshot.getWarnings(). The serviceability/dcmd/gc/HeapDumpTest.java will be changed accordingly when RFE is implemented. Thanks, Katja On 04/22/2015 03:09 PM, Staffan Larsen wrote: On 22 apr 2015, at 11:17, Yekaterina Kantserovayekaterina.kantser...@oracle.com wrote: Hi, Could I please have a review of this fix. bug:https://bugs.openjdk.java.net/browse/JDK-8059047 webrev:http://cr.openjdk.java.net/~ykantser/8059047/webrev.00/ This fix is a part of JEP 241: Remove the jhat Tool (https://bugs.openjdk.java.net/browse/JDK-8059039). I suggest to put parser/validator into common test library since the functionality can be useful not only for SVC tools tests but even for some future GC tests. The old jhat packages have been moved as follows: com.sun.tools.hat.internal.model - jdk.test.lib.hprof.model com.sun.tools.hat.internal.parser - jdk.test.lib.hprof.parser com.sun.tools.hat.internal.util - jdk.test.lib.hprof.util The source has not been changed except Copyrights year. Thanks, Katja
Re: RFR(M): 8059047: Extract parser/validator from jhat for use in tests
All suggestions have been implemented. Please find new webrevs here: webrev root: http://cr.openjdk.java.net/~ykantser/8059047/webrev.02 webrev jdk: http://cr.openjdk.java.net/~ykantser/8059047.jdk/webrev.02 webrev hotspot: http://cr.openjdk.java.net/~ykantser/8059047.hotspot/webrev.01 // Katja On 04/24/2015 12:10 PM, Staffan Larsen wrote: On 24 apr 2015, at 11:34, Yekaterina Kantserova yekaterina.kantser...@oracle.com mailto:yekaterina.kantser...@oracle.com wrote: Hi, Here comes the updated version. bug: https://bugs.openjdk.java.net/browse/JDK-8059047 webrev root: http://cr.openjdk.java.net/~ykantser/8059047/webrev.01/ http://cr.openjdk.java.net/%7Eykantser/8059047/webrev.01/ webrev jdk: http://cr.openjdk.java.net/~ykantser/8059047.jdk/webrev.01/ http://cr.openjdk.java.net/%7Eykantser/8059047.jdk/webrev.01/ test/com/sun/management/HotSpotDiagnosticMXBean/DumpHeap.java: In main() I think you should change the deleteOnExit() to just delete(). There is no reason to wait with it. Perhaps we should also verify that the file does not already exists before we try to write to it. If it exists, we can delete it. webrev hotspot: http://cr.openjdk.java.net/~ykantser/8059047.hotspot/webrev.00/ http://cr.openjdk.java.net/%7Eykantser/8059047.hotspot/webrev.00/ test/serviceability/dcmd/gc/HeapDumpTest.java: Same thing: call delete() instead of deleteOnExit(). One comment about changes in hotspot part. The refactored version of serviceability/dcmd/gc/HeapDumpTest.java doesn't contain check: 70 /* 71 * Some hprof dumps of all objects contain constantPoolOop references that cannot be resolved, so we ignore 72 * failures about resolving constantPoolOop fields using a negative lookahead 73 */ 74 output.shouldNotMatch(.*WARNING(?!.*Failed to resolve object.*constantPoolOop.*).*); It depends on that the current version of jdk.test.lib.hprof parser simply write down warnings to stdout. As a result the test needs to invent own logic to parse it. I suggest instead to improve jdk.test.lib.hprof parser as a separate RFE. The parser will collect such information and provide a new method for getting it, e.g. jdk.test.lib.hprof.model.Snapshot.getWarnings(). The serviceability/dcmd/gc/HeapDumpTest.java will be changed accordingly when RFE is implemented. Sounds right. A quick, hacky solution is to redirect System.out to a stream or file while running the parser… /Staffan Thanks, Katja On 04/22/2015 03:09 PM, Staffan Larsen wrote: On 22 apr 2015, at 11:17, Yekaterina Kantserovayekaterina.kantser...@oracle.com mailto:yekaterina.kantser...@oracle.com wrote: Hi, Could I please have a review of this fix. bug:https://bugs.openjdk.java.net/browse/JDK-8059047 http://bugs.openjdk.java.net/browse/JDK-8059047 webrev:http://cr.openjdk.java.net/~ykantser/8059047/webrev.00/ http://cr.openjdk.java.net/%7Eykantser/8059047/webrev.00/ This fix is a part of JEP 241: Remove the jhat Tool (https://bugs.openjdk.java.net/browse/JDK-8059039). I suggest to put parser/validator into common test library since the functionality can be useful not only for SVC tools tests but even for some future GC tests. The old jhat packages have been moved as follows: com.sun.tools.hat.internal.model - jdk.test.lib.hprof.model com.sun.tools.hat.internal.parser - jdk.test.lib.hprof.parser com.sun.tools.hat.internal.util - jdk.test.lib.hprof.util The source has not been changed except Copyrights year. Thanks, Katja
Re: RFR(M): 8059047: Extract parser/validator from jhat for use in tests
Thanks for the catch Staffan! I'll go through all of them and send out a new webrev. // Katja - Original Message - From: staffan.lar...@oracle.com To: staffan.lar...@oracle.com Cc: yekaterina.kantser...@oracle.com, serviceability-dev@openjdk.java.net, hotspot-...@openjdk.java.net Sent: Wednesday, April 22, 2015 3:09:35 PM GMT +01:00 Amsterdam / Berlin / Bern / Rome / Stockholm / Vienna Subject: Re: RFR(M): 8059047: Extract parser/validator from jhat for use in tests Oh, and: serviceability/dcmd/gc/HeapDumpTest.java On 22 apr 2015, at 15:06, Staffan Larsen staffan.lar...@oracle.com wrote: I also found this test that uses jhat and needs an update: com/sun/management/HotSpotDiagnosticMXBean/DumpHeap.sh /Staffan On 22 apr 2015, at 15:05, Staffan Larsen staffan.lar...@oracle.com wrote: I think you are missing a @build jdk.test.lib.hprof..*”. /Staffan On 22 apr 2015, at 14:25, Yekaterina Kantserova yekaterina.kantser...@oracle.com wrote: Staffan, thank you for the review! This issue needs a change in jdk as well. http://cr.openjdk.java.net/~ykantser/8059047.jdk/webrev.00 - BasicJMapTest.java will use HprofParser to verify hprof dumps created by the test. // Katja On 04/22/2015 01:20 PM, Staffan Larsen wrote: Looks good! Thanks, /Staffan On 22 apr 2015, at 11:17, Yekaterina Kantserova yekaterina.kantser...@oracle.com wrote: Hi, Could I please have a review of this fix. bug: https://bugs.openjdk.java.net/browse/JDK-8059047 webrev: http://cr.openjdk.java.net/~ykantser/8059047/webrev.00/ This fix is a part of JEP 241: Remove the jhat Tool (https://bugs.openjdk.java.net/browse/JDK-8059039). I suggest to put parser/validator into common test library since the functionality can be useful not only for SVC tools tests but even for some future GC tests. The old jhat packages have been moved as follows: com.sun.tools.hat.internal.model - jdk.test.lib.hprof.model com.sun.tools.hat.internal.parser - jdk.test.lib.hprof.parser com.sun.tools.hat.internal.util - jdk.test.lib.hprof.util The source has not been changed except Copyrights year. Thanks, Katja
RFR(S): 8076524: Remove jhat tests and help library from JDK
Hi, Could I please have a review of this fix. bug: https://bugs.openjdk.java.net/browse/JDK-8076524 webrev: http://cr.openjdk.java.net/~ykantser/8076524/webrev.00 This fix is a part of JEP 241: Remove the jhat Tool (https://bugs.openjdk.java.net/browse/JDK-8059039). Thanks, Katja
Re: RFR(S): 8076524: Remove jhat tests and help library from JDK
Hi Alan, Thank you for looking at it! When JEP was created the work has been divided in product and test parts according to the JEP guidelines. That's the reason I can come up with. To test part belongs even https://bugs.openjdk.java.net/browse/JDK-8059047. That's would be great if you can take a look at it as well. // Katja On 04/22/2015 01:09 PM, Alan Bateman wrote: On 22/04/2015 11:56, Yekaterina Kantserova wrote: Hi, Could I please have a review of this fix. bug: https://bugs.openjdk.java.net/browse/JDK-8076524 webrev: http://cr.openjdk.java.net/~ykantser/8076524/webrev.00 This fix is a part of JEP 241: Remove the jhat Tool (https://bugs.openjdk.java.net/browse/JDK-8059039). This looks okay to me although I would have expected these tests to be removed as part of the change to remove jhat (no issue with it being split into multiple changes, just wondering why it is being done this way). -Alan
RFR(M): 8059047: Extract parser/validator from jhat for use in tests
Hi, Could I please have a review of this fix. bug: https://bugs.openjdk.java.net/browse/JDK-8059047 webrev: http://cr.openjdk.java.net/~ykantser/8059047/webrev.00/ This fix is a part of JEP 241: Remove the jhat Tool (https://bugs.openjdk.java.net/browse/JDK-8059039). I suggest to put parser/validator into common test library since the functionality can be useful not only for SVC tools tests but even for some future GC tests. The old jhat packages have been moved as follows: com.sun.tools.hat.internal.model - jdk.test.lib.hprof.model com.sun.tools.hat.internal.parser - jdk.test.lib.hprof.parser com.sun.tools.hat.internal.util - jdk.test.lib.hprof.util The source has not been changed except Copyrights year. Thanks, Katja
Re: RFR(M): 8059047: Extract parser/validator from jhat for use in tests
Staffan, thank you for the review! This issue needs a change in jdk as well. http://cr.openjdk.java.net/~ykantser/8059047.jdk/webrev.00 - BasicJMapTest.java will use HprofParser to verify hprof dumps created by the test. // Katja On 04/22/2015 01:20 PM, Staffan Larsen wrote: Looks good! Thanks, /Staffan On 22 apr 2015, at 11:17, Yekaterina Kantserova yekaterina.kantser...@oracle.com wrote: Hi, Could I please have a review of this fix. bug: https://bugs.openjdk.java.net/browse/JDK-8059047 webrev: http://cr.openjdk.java.net/~ykantser/8059047/webrev.00/ This fix is a part of JEP 241: Remove the jhat Tool (https://bugs.openjdk.java.net/browse/JDK-8059039). I suggest to put parser/validator into common test library since the functionality can be useful not only for SVC tools tests but even for some future GC tests. The old jhat packages have been moved as follows: com.sun.tools.hat.internal.model - jdk.test.lib.hprof.model com.sun.tools.hat.internal.parser - jdk.test.lib.hprof.parser com.sun.tools.hat.internal.util - jdk.test.lib.hprof.util The source has not been changed except Copyrights year. Thanks, Katja
Re: JDK 9 RFR of JDK-8077952: sun/management/jmxremote/bootstrap/RmiSslBootstrapTest.sh should be quarantined
Hi Joe, I would also suggest to put the test on the ProblemList, which is the current practice for jdk repo. Best regards, Katja - Original Message - From: alan.bate...@oracle.com To: joe.da...@oracle.com, serviceability-dev@openjdk.java.net Sent: Monday, April 20, 2015 7:44:30 PM GMT +01:00 Amsterdam / Berlin / Bern / Rome / Stockholm / Vienna Subject: Re: JDK 9 RFR of JDK-8077952: sun/management/jmxremote/bootstrap/RmiSslBootstrapTest.sh should be quarantined On 20/04/2015 17:49, joe darcy wrote: Hello, As the test sun/management/jmxremote/bootstrap/RmiSslBootstrapTest.sh has been failing for several days across platforms, I think the time has come to mark the test as ignored until the full fix for related bug JDK-8077924: sun/management/jmxremote/bootstrap/RmiSslBootstrapTest.sh start failing after JDK-8076221 can be developed. This looks okay but we mostly use the exclude list (jdk/test/ProblemList.txt) these days in the jdk repo (I realize @ignore is still used in the hotspot repo). -Alan.
RFR(XS): 8077423: jstatd is not terminated even though it cannot contact or bind to RMI Registry
Hi, Could I please have a review of this fix. bug: https://bugs.openjdk.java.net/browse/JDK-8077423 webrev: http://cr.openjdk.java.net/~ykantser/8077423/webrev.00/ A couple of comments about changes in sun/tools/jstatd/JstatdTest.java. If the suggested fix will be accepted there will be no need of waitForJstatdRMI(), because jstad will either be started properly or will terminate. A check has been added that there is no RMi Registry already running on default port. Otherwise the test will fail fast. A sanity check for -nr option has been added as well. http://docs.oracle.com/javase/7/docs/technotes/tools/share/jstatd.html -nr Do not attempt to create an internal RMI registry within the jstatd process when an existing RMI registry is not found. Thanks, Katja
Re: RFR(XXS): 8077611: com/sun/jdi/ConnectedVMs.java should be unquarantined
Staffan, thanks! On 04/15/2015 01:03 PM, Staffan Larsen wrote: Looks good! Thanks, /Staffan On 14 apr 2015, at 11:44, Yekaterina Kantserova yekaterina.kantser...@oracle.com wrote: Hi, Could I please have a review of this very small fix. bug: https://bugs.openjdk.java.net/browse/JDK-8077611 webrev: http://cr.openjdk.java.net/~ykantser/8077611/webrev.00 Thanks, Katja
RFR(XXS): 8077611: com/sun/jdi/ConnectedVMs.java should be unquarantined
Hi, Could I please have a review of this very small fix. bug: https://bugs.openjdk.java.net/browse/JDK-8077611 webrev: http://cr.openjdk.java.net/~ykantser/8077611/webrev.00 Thanks, Katja
Re: RFR(XS): 8027668: sun/tools/jstatd/TestJstatdPort.java: java.net.ConnectException: Connection refused: connect
Hi, The updated webrev can be found here: http://cr.openjdk.java.net/~ykantser/8027668/webrev.01/ Thanks, Katja On 04/02/2015 02:44 PM, Jaroslav Bachorik wrote: Hi Katja, On 2.4.2015 14:16, Yekaterina Kantserova wrote: Hi, Could I please have a review of this fix. bug: https://bugs.openjdk.java.net/browse/JDK-8027668 webrev: http://cr.openjdk.java.net/~ykantser/8027668/webrev.00/ In src/jdk.jvmstat/share/classes/sun/tools/jstatd/Jstatd.java I would suggest adding System.out.flush() after printing jstatd is started on ... to make sure the output is actually committed to stdout. Please, update the copyright years. -JB- The tests can still contain a race. There is a possibility the jstad is not really started though there is a pid for the process. The suggestion is to let jstatd notify it's started and for the test wait until this notification. Thanks, Katja
Re: RFR(XS): 8027668: sun/tools/jstatd/TestJstatdPort.java: java.net.ConnectException: Connection refused: connect
Jaroslav, Staffan, Thanks for the reviews! // Katja On 04/07/2015 03:55 PM, Jaroslav Bachorik wrote: Looks fine! You might want to convert the 'jstatd started (bound to' string to a constant to make it easier to track any changes in the future but it is up to you. I am fine with the changes anyway. -JB- On 7.4.2015 14:52, Yekaterina Kantserova wrote: Hi, The updated webrev can be found here: http://cr.openjdk.java.net/~ykantser/8027668/webrev.01/ Thanks, Katja On 04/02/2015 02:44 PM, Jaroslav Bachorik wrote: Hi Katja, On 2.4.2015 14:16, Yekaterina Kantserova wrote: Hi, Could I please have a review of this fix. bug: https://bugs.openjdk.java.net/browse/JDK-8027668 webrev: http://cr.openjdk.java.net/~ykantser/8027668/webrev.00/ In src/jdk.jvmstat/share/classes/sun/tools/jstatd/Jstatd.java I would suggest adding System.out.flush() after printing jstatd is started on ... to make sure the output is actually committed to stdout. Please, update the copyright years. -JB- The tests can still contain a race. There is a possibility the jstad is not really started though there is a pid for the process. The suggestion is to let jstatd notify it's started and for the test wait until this notification. Thanks, Katja
Re: RFR(XS): 8027668: sun/tools/jstatd/TestJstatdPort.java: java.net.ConnectException: Connection refused: connect
Jaroslav, thanks, will do! // Katja On 04/02/2015 02:44 PM, Jaroslav Bachorik wrote: Hi Katja, On 2.4.2015 14:16, Yekaterina Kantserova wrote: Hi, Could I please have a review of this fix. bug: https://bugs.openjdk.java.net/browse/JDK-8027668 webrev: http://cr.openjdk.java.net/~ykantser/8027668/webrev.00/ In src/jdk.jvmstat/share/classes/sun/tools/jstatd/Jstatd.java I would suggest adding System.out.flush() after printing jstatd is started on ... to make sure the output is actually committed to stdout. Please, update the copyright years. -JB- The tests can still contain a race. There is a possibility the jstad is not really started though there is a pid for the process. The suggestion is to let jstatd notify it's started and for the test wait until this notification. Thanks, Katja
RFR(XS): 8027668: sun/tools/jstatd/TestJstatdPort.java: java.net.ConnectException: Connection refused: connect
Hi, Could I please have a review of this fix. bug: https://bugs.openjdk.java.net/browse/JDK-8027668 webrev: http://cr.openjdk.java.net/~ykantser/8027668/webrev.00/ The tests can still contain a race. There is a possibility the jstad is not really started though there is a pid for the process. The suggestion is to let jstatd notify it's started and for the test wait until this notification. Thanks, Katja
Re: RFR(XS): 8027668: sun/tools/jstatd/TestJstatdPort.java: java.net.ConnectException: Connection refused: connect
name is a combination of: rmi name = Name to which the remote RMI object is bound in the RMI registry port = Port number where the RMI registry is expected to be found jstatd is bound to ...? // Katja What does the name.toString() look like? Should the message be “jstatd listening on …”? On 2 apr 2015, at 14:56, Yekaterina Kantserova yekaterina.kantser...@oracle.com wrote: Jaroslav, thanks, will do! // Katja On 04/02/2015 02:44 PM, Jaroslav Bachorik wrote: Hi Katja, On 2.4.2015 14:16, Yekaterina Kantserova wrote: Hi, Could I please have a review of this fix. bug: https://bugs.openjdk.java.net/browse/JDK-8027668 webrev: http://cr.openjdk.java.net/~ykantser/8027668/webrev.00/ In src/jdk.jvmstat/share/classes/sun/tools/jstatd/Jstatd.java I would suggest adding System.out.flush() after printing jstatd is started on ... to make sure the output is actually committed to stdout. Please, update the copyright years. -JB- The tests can still contain a race. There is a possibility the jstad is not really started though there is a pid for the process. The suggestion is to let jstatd notify it's started and for the test wait until this notification. Thanks, Katja
Re: RFR 8054890: Serviceability: New diagnostic commands 'VM.set_flag' and 'JVMTI.data_dump'
The tests looks good! Thank you very much for fixing. // Katja On 03/23/2015 12:41 PM, Staffan Larsen wrote: Looks good! Thanks, /Staffan On 23 mar 2015, at 11:55, Jaroslav Bachorik jaroslav.bacho...@oracle.com wrote: On 23.3.2015 08:50, Staffan Larsen wrote: diagnosticCommand.cpp: - Should SetVMFlagDCmd really be inside #if INCLUDE_SERVICES” ? Probably not. On the other hand, the JVMTIDataDumpDCmd registration should probably be guarded by #if INCLUDE_JVMTI - L227-234: strange indentation Fixed. Updated webrev (+ removing the extraneous #include services/attachListener.hpp in diagnosticCommand.hpp) : http://cr.openjdk.java.net/~jbachorik/8054890/webrev.01 -JB- /Staffan On 19 mar 2015, at 10:59, Jaroslav Bachorik jaroslav.bacho...@oracle.com wrote: Please, review the following change Issue : https://bugs.openjdk.java.net/browse/JDK-8054890 Webrev: http://cr.openjdk.java.net/~jbachorik/8054890/webrev.00 This patch is about adding 2 new diagnostic commands - VM.set_flag and JVMTI.data_dump. VM.set_flag allows to set any writeable flag. It takes the flag name and the flag value in textual form. The mutability of the flag and the value format checks are forwarded to the shared vm management code. JVMTI.data_dump will send the data dump request to JVMTI. Both of these commands are covered by the corresponding tests. Thanks, -JB-
Re: RFR: JDK-8075586: add @modules as needed to the open hotspot tests
Mandy, Thank you very much for the catch! The updated webrev can be found here: http://cr.openjdk.java.net/~ykantser/8075586/webrev.00/ Best regards, Katja On 03/26/2015 12:11 AM, Mandy Chung wrote: Alexandar, Shura, The dependency analysis is not up-to-date that sun.tools.jar has been moved to jdk.jartool module in jdk9 b55. It has been in jdk9/dev since 3/6. I have pointed out multiple times previously that jdk.dev/sun.tools.jar is wrong in the jdk side of change. Below includes an example. Mandy --- old/test/runtime/RedefineTests/RedefineAnnotations.java 2015-03-25 16:24:41.462038538 +0300 +++ new/test/runtime/RedefineTests/RedefineAnnotations.java 2015-03-25 16:24:41.386038539 +0300 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2014, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2014, 2015, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -25,6 +25,9 @@ * @test * @library /testlibrary * @summary Test that type annotations are retained after a retransform + * @modules java.base/jdk.internal.org.objectweb.asm + * java.instrument + * jdk.dev/sun.tools.jar * @run main RedefineAnnotations buildagent * @run main/othervm -javaagent:redefineagent.jar RedefineAnnotations */ On 3/25/15 7:38 AM, Alexander Kulyakhtin wrote: Hi Please, find the updated review for the bulk @modules change at the link below. We have fixed the copyrights and the files mentioned in the mail from Lois. http://cr.openjdk.java.net/~eistepan/~akulyakhtin/8075586/index.html Best regards, Alex - Original Message - From: lois.fol...@oracle.com To: yekaterina.kantser...@oracle.com Cc: serviceability-dev@openjdk.java.net, staffan.lar...@oracle.com, hotspot-...@openjdk.java.net, alexander.kulyakh...@oracle.com, alexandre.il...@oracle.com Sent: Tuesday, March 24, 2015 3:57:54 PM GMT +04:00 Abu Dhabi / Muscat Subject: Re: RFR: JDK-8075586: add @modules as needed to the open hotspot tests This looks good, thank you for making these changes! A couple of comments that I don't feel need another webrev but should be fixed before pushing. - copyrights on all the tests need to be updated - the following tests have a blank comment line before the new @modules line that could be removed test/gc/metaspace/TestMetaspacePerfCounters.java test/runtime/contended/Basic.java test/compiler/jsr292/CreatesInterfaceDotEqualsCallInfo.java test/compiler/cpuflags/RestoreMXCSR.java test/compiler/debug/VerifyAdapterSharing.java Thanks, Lois On 3/24/2015 8:09 AM, Yekaterina Kantserova wrote: Notifying hotspot-dev as well. // Katja On 03/24/2015 11:48 AM, Alexander Kulyakhtin wrote: Could the reviewers, please, have a look at the proposed changes below? In addition, we are going to make a change to the TEST.ROOT file as indicated by Staffan in the mail below. Do you think the changes (plus the one-line change to the TEST.ROOT) can be pushed into the jdk? Best regards, Alex - Original Message - From: staffan.lar...@oracle.com To: alexander.kulyakh...@oracle.com Cc: serviceability-dev@openjdk.java.net, alexandre.il...@oracle.com Sent: Friday, March 20, 2015 7:39:10 PM GMT +04:00 Abu Dhabi / Muscat Subject: Re: RFR: JDK-8075586: add @modules as needed to the open hotspot tests I haven’t looked at the changes in detail, but please change the requiredVersion in TEST.ROOT to 4.1 b11 as part of this change. Thanks, /Staffan On 20 mar 2015, at 13:16, Alexander Kulyakhtin alexander.kulyakh...@oracle.com wrote: Hi, Could you, please, review the fix below. CR: https://bugs.openjdk.java.net/browse/JDK-8075586 webrev: http://cr.openjdk.java.net/~tpivovarova/akulyakh/8075586/webrev.00/ The fix adds @modules keyword to the existing hotspot tests, as needed, so that the tests can access the required API when the new modular architecture is in place. Best regards, Alex
RFR(XXS): 8075818: serviceability/threads/TestFalseDeadLock.java should be unquarantined
Hi, Could I please have a review of these 2 very small fixes. bug: https://bugs.openjdk.java.net/browse/JDK-8075818 webrev: http://cr.openjdk.java.net/~ykantser/8075818/webrev.00/ bug: https://bugs.openjdk.java.net/browse/JDK-8075820 webrev: http://cr.openjdk.java.net/~ykantser/8075820/webrev.00/ Thanks, Katja
Re: RFR(XXS): 8075818: serviceability/threads/TestFalseDeadLock.java should be unquarantined
I will. Thanks for the review! // Katja - Original Message - From: jaroslav.bacho...@oracle.com To: serviceability-dev@openjdk.java.net Sent: Wednesday, March 25, 2015 11:19:31 AM GMT +01:00 Amsterdam / Berlin / Bern / Rome / Stockholm / Vienna Subject: Re: RFR(XXS): 8075818: serviceability/threads/TestFalseDeadLock.java should be unquarantined Hi Katja, this looks good. Just update the copyright years before pushing. No need to re-review. -JB- On 25.3.2015 11:05, Yekaterina Kantserova wrote: Hi, Could I please have a review of these 2 very small fixes. bug: https://bugs.openjdk.java.net/browse/JDK-8075818 webrev: http://cr.openjdk.java.net/~ykantser/8075818/webrev.00/ bug: https://bugs.openjdk.java.net/browse/JDK-8075820 webrev: http://cr.openjdk.java.net/~ykantser/8075820/webrev.00/ Thanks, Katja
Re: RFR(XXS): 8075818: serviceability/threads/TestFalseDeadLock.java should be unquarantined
Serguei, thanks for the review! // Katja - Original Message - From: serguei.spit...@oracle.com To: yekaterina.kantser...@oracle.com, serviceability-dev@openjdk.java.net Sent: Wednesday, March 25, 2015 12:35:30 PM GMT +01:00 Amsterdam / Berlin / Bern / Rome / Stockholm / Vienna Subject: Re: RFR(XXS): 8075818: serviceability/threads/TestFalseDeadLock.java should be unquarantined Hi Katya, It looks good. Thanks, Serguei On 3/25/15 3:05 AM, Yekaterina Kantserova wrote: Hi, Could I please have a review of these 2 very small fixes. bug: https://bugs.openjdk.java.net/browse/JDK-8075818 webrev: http://cr.openjdk.java.net/~ykantser/8075818/webrev.00/ bug: https://bugs.openjdk.java.net/browse/JDK-8075820 webrev: http://cr.openjdk.java.net/~ykantser/8075820/webrev.00/ Thanks, Katja
Re: RFR: JDK-8075586: add @modules as needed to the open hotspot tests
Notifying hotspot-dev as well. // Katja On 03/24/2015 11:48 AM, Alexander Kulyakhtin wrote: Could the reviewers, please, have a look at the proposed changes below? In addition, we are going to make a change to the TEST.ROOT file as indicated by Staffan in the mail below. Do you think the changes (plus the one-line change to the TEST.ROOT) can be pushed into the jdk? Best regards, Alex - Original Message - From: staffan.lar...@oracle.com To: alexander.kulyakh...@oracle.com Cc: serviceability-dev@openjdk.java.net, alexandre.il...@oracle.com Sent: Friday, March 20, 2015 7:39:10 PM GMT +04:00 Abu Dhabi / Muscat Subject: Re: RFR: JDK-8075586: add @modules as needed to the open hotspot tests I haven’t looked at the changes in detail, but please change the requiredVersion in TEST.ROOT to 4.1 b11 as part of this change. Thanks, /Staffan On 20 mar 2015, at 13:16, Alexander Kulyakhtin alexander.kulyakh...@oracle.com wrote: Hi, Could you, please, review the fix below. CR: https://bugs.openjdk.java.net/browse/JDK-8075586 webrev: http://cr.openjdk.java.net/~tpivovarova/akulyakh/8075586/webrev.00/ The fix adds @modules keyword to the existing hotspot tests, as needed, so that the tests can access the required API when the new modular architecture is in place. Best regards, Alex
Re: RFR(XXS): 8064923: [TESTBUG] jps doesn't display anything on embedded platforms and it causes some tests to fail
Erik, Jaroslav, thanks for your reviews! // Katja On 03/18/2015 06:25 PM, Erik Gahlin wrote: Looks good. Erik Yekaterina Kantserova skrev den 18/03/15 17:37: Hi, Could I please have a review of this very small fix. bug: https://bugs.openjdk.java.net/browse/JDK-8064923 webrev: http://cr.openjdk.java.net/~ykantser/8064923/webrev.00/ Thanks, Katja
RFR(XXS): 8064923: [TESTBUG] jps doesn't display anything on embedded platforms and it causes some tests to fail
Hi, Could I please have a review of this very small fix. bug: https://bugs.openjdk.java.net/browse/JDK-8064923 webrev: http://cr.openjdk.java.net/~ykantser/8064923/webrev.00/ Thanks, Katja
Re: RFR(S): 8073794: jdk/test/com/sun/jdi/BadHandshakeTest.java should retry if tcp port is taken
Thanks Jaroslav! // Katja On 03/13/2015 07:00 PM, Jaroslav Bachorik wrote: Hi Katja, Thumbs up! -JB- On 13.3.2015 15:23, Yekaterina Kantserova wrote: Hi, Could I please have a review of this fix. bug: https://bugs.openjdk.java.net/browse/JDK-8073794 webrev: http://cr.openjdk.java.net/~ykantser/8073794/webrev.00/ Thanks, Katja
RFR(S): 8073794: jdk/test/com/sun/jdi/BadHandshakeTest.java should retry if tcp port is taken
Hi, Could I please have a review of this fix. bug: https://bugs.openjdk.java.net/browse/JDK-8073794 webrev: http://cr.openjdk.java.net/~ykantser/8073794/webrev.00/ Thanks, Katja
Re: [ping] Re: RFR 8074041: sun/management/jmxremote/startstop/JMXStartStopTest.java fails with InvocationTargetException
Hi Jaroslav, That's exactly the functionality I've been looking for! Not only it will solveJDK-8074041 the ProcessImpl will help me to solve https://bugs.openjdk.java.net/browse/JDK-8073794 since I'll be able to analyse out and err streams outside startProcess() function. Best regards, Katja (not a reviwer) On 03/11/2015 09:35 AM, Jaroslav Bachorik wrote: On 5.3.2015 11:37, Jaroslav Bachorik wrote: Please, review the following change Issue : https://bugs.openjdk.java.net/browse/JDK-8074041 Webrev: http://cr.openjdk.java.net/~jbachorik/8074041/webrev.00 This test fails very intermittently still and the failure is very hard to reproduce. Based on the symptoms the probable cause seems to be the asynchronous processing of a launched process stdout and stderr. The test is using ProcessTools.startProcess(...) to start the target process and process its output (stdout/stderr) to mark the occurrence of the expected string. When the target process has finished the test will check the mark to assert the correctness. However, it may happen that the assertion occurs in the time interval between the target process finishing (process.waitFor() returns) and the asynchronous stderr/stdout processors had the chance to dispatch the target process output to the processor checking for the occurrence of the expected test. The proposed fix is to wrap the Process instance into a wrapper enhancing the waitFor() method with the wait for the stdout/stderr processor being finished before proceeding. I ran the test in a tight loop for 1000 times after applying this patch without any failure. Thanks, -JB-
Re: 8072856: Eliminate ProcessTools.getProcessId dependency on sun.management.VMManagement
Thanks everybody for the reviews! The fix is pushed. // Katja - Original Message - From: mandy.ch...@oracle.com To: daniel.fu...@oracle.com, yekaterina.kantser...@oracle.com, serviceability-dev@openjdk.java.net, alan.bate...@oracle.com Sent: Wednesday, February 11, 2015 6:06:18 PM GMT +01:00 Amsterdam / Berlin / Bern / Rome / Stockholm / Vienna Subject: Re: 8072856: Eliminate ProcessTools.getProcessId dependency on sun.management.VMManagement +1 This is temporary. When the Process API is available, we can convert to use the new public API. Mandy On 2/11/2015 8:57 AM, Daniel Fuchs wrote: Looks good to me too. JEP 102 Process Updates will provide an easier way to get the current process PID but we don't have that yet :-) best regards, -- daniel On 11/02/15 17:42, Yekaterina Kantserova wrote: Hi, Could I please have a review of this small fix. bug: https://bugs.openjdk.java.net/browse/JDK-8072856 webrev: http://cr.openjdk.java.net/~ykantser/8072856/webrev.00/ Thanks, Katja
8072856: Eliminate ProcessTools.getProcessId dependency on sun.management.VMManagement
Hi, Could I please have a review of this small fix. bug: https://bugs.openjdk.java.net/browse/JDK-8072856 webrev: http://cr.openjdk.java.net/~ykantser/8072856/webrev.00/ Thanks, Katja
RFR(XXS): 8071784: serviceability/attach/AttachWithStalePidFile.java should be quarantined
Hi, Could I please have a review of this very small fix. bug: https://bugs.openjdk.java.net/browse/JDK-8071784 webrev: http://cr.openjdk.java.net/~ykantser/8071784/webrev.00/ Thanks, Katja
Re: RFR: JDK-8071464: Clear up SVC jdk/test/* JRE layout dependencies other than those on tools.jar
Alex, I'll push. // Katja On 01/29/2015 01:21 PM, Alexander Kulyakhtin wrote: Hi Katja, Stafan I've corrected the alignment issue, per Stafan's review, and attached the patch. Could it be possible to push the patch? Thank you very much for your help. Best regards, Alex - Original Message - From: staffan.lar...@oracle.com To: alexander.kulyakh...@oracle.com Cc: serviceability-dev@openjdk.java.net Sent: Thursday, January 29, 2015 11:02:12 AM GMT +04:00 Abu Dhabi / Muscat Subject: Re: RFR: JDK-8071464: Clear up SVC jdk/test/* JRE layout dependencies other than those on tools.jar Looks good! nit: test/com/sun/jdi/ShellScaffold.sh:947, the alignment for 2” got broken. Thanks, /Staffan On 28 jan 2015, at 17:24, Alexander Kulyakhtin alexander.kulyakh...@oracle.com wrote: Hi, Could you, please, review the fix below. bug: https://bugs.openjdk.java.net/browse/JDK-8071464 webrev: http://cr.openjdk.java.net/~eistepan/~akulyakhtin/8071464/webrev.01/ References to the jre layout are removed from ShellScaffold.sh and DemoRun.java. Other files mentioned in the CR have not been changed as they don't contain any JRE-related artifacts anymore. Thanks, Alex
Re: RFR(XXS): 8071545: Tests are still excluded while the appropriate bug has been fixed
Thanks, Serguei! On 01/27/2015 09:33 AM, serguei.spit...@oracle.com wrote: Hi Katya, It looks good. Thanks, Serguei On 1/27/15 12:28 AM, Yekaterina Kantserova wrote: Hi, Could I please have a review of this very small fix. bug: https://bugs.openjdk.java.net/browse/JDK-8071545 webrev: http://cr.openjdk.java.net/~ykantser/8071545/webrev.00/ Thanks, Katja
RFR(XXS): 8071582: com/sun/jdi/GetLocalVariables4Test.sh should be quarantined
Hi, Could I please have a review of this very small fix. bug: https://bugs.openjdk.java.net/browse/JDK-8071582 webrev: http://cr.openjdk.java.net/~ykantser/8071582/webrev.00/ Thanks, Katja
RFR(XXS): 8071545: Tests are still excluded while the appropriate bug has been fixed
Hi, Could I please have a review of this very small fix. bug: https://bugs.openjdk.java.net/browse/JDK-8071545 webrev: http://cr.openjdk.java.net/~ykantser/8071545/webrev.00/ Thanks, Katja
RFR(XXS): 8071324: com/sun/jdi/ConnectedVMs.java should be quarantined
Hi, Could I please have a review of this very small fix. bug: https://bugs.openjdk.java.net/browse/JDK-8071324 webrev: http://cr.openjdk.java.net/~ykantser/8071324/webrev.00/ Thanks, Katja
Re: RFR(S): 8044419: TEST_BUG: com/sun/jdi/JdbReadTwiceTest.sh fails when run under root
Hi,
New webrev can be found here
http://cr.openjdk.java.net/~ykantser/8044419/webrev.03/
The fix has been tested on all platforms except embedded.
Thanks,
Katja
On 01/21/2015 12:56 PM, Dmitry Samersoff wrote:
Mattias,
1. mkFiles at ll. 215 above is reluctant
2. if you wish to store id output and grep the file it's better to do
something like:
id $HOME/jdb.ini
chmod a-r $HOME/jdb.ini
grep -q 'uid=0(' $HOME/jdb.ini 2 /dev/null
case $? in
0)
echo Can't make file unreadable running as root
;;
1)
echo Can't make file unreadable for some other reason
;;
2)
if [ -f $HOME/jdb.ini ]
then
echo OK. the file is unreadable
else
echo Can't create a file
fi
;;
esac
-Dmitry
On 2015-01-21 13:05, Mattias Tobiasson wrote:
Hi,
Changes in this version:
1. Replaced the unnecessary grep from unreadable file with id | grep ...
2. Log that permission denied error message is expected.
webrev: http://cr.openjdk.java.net/~ykantser/8044419/webrev.02/
bug: https://bugs.openjdk.java.net/browse/JDK-8044419
Thanks,
Mattias
On 01/20/2015 12:25 PM, Mattias Tobiasson wrote:
Thanks for the suggestion. Your suggestion is a better way to check if
the user is root.
But if we only use that check, then we do not verify that the file is
really unreadable.
I do not know if there are any other conditions, besides running as
root, that can fail to make a file unreadable.
I think it feels safer to really try to read the unreadable file. Then
we will get the error message.
I could add a log that says the error message is expected.
And I can change the second grep to your suggestion.
Mattias
On 01/19/2015 04:13 PM, Dmitry Samersoff wrote:
Mattias,
After chmod a-r grep will display unpleasant
permission denied error for non root user
so it's better just do:
if id | grep -q 'uid=0('
then
Do root staff
else
Do non-root staff
fi
-Dmitry
On 2015-01-19 16:24, Mattias Tobiasson wrote:
Hi,
Could I please have a review of this test bug fix.
Test expects some files to be unreadable. That does not work when
running as root.
The fix is to ignore the parts for unreadable files when running as
root.
bug: https://bugs.openjdk.java.net/browse/JDK-8044419
webrev: http://cr.openjdk.java.net/~miauno/8044419/webrev.01
Tested as non-root on all platforms except embedded.
Tested as root on linux.
Thanks,
Mattias
Re: RFR(S): 8044419: TEST_BUG: com/sun/jdi/JdbReadTwiceTest.sh fails when run under root
Dmitry, Staffan, thanks for your reviews!
// Katja
On 01/23/2015 04:43 PM, Staffan Larsen wrote:
Looks good!
Thanks,
/Staffan
On 23 jan 2015, at 09:51, Yekaterina Kantserova
yekaterina.kantser...@oracle.com wrote:
Hi,
New webrev can be found here
http://cr.openjdk.java.net/~ykantser/8044419/webrev.03/
The fix has been tested on all platforms except embedded.
Thanks,
Katja
On 01/21/2015 12:56 PM, Dmitry Samersoff wrote:
Mattias,
1. mkFiles at ll. 215 above is reluctant
2. if you wish to store id output and grep the file it's better to do
something like:
id $HOME/jdb.ini
chmod a-r $HOME/jdb.ini
grep -q 'uid=0(' $HOME/jdb.ini 2 /dev/null
case $? in
0)
echo Can't make file unreadable running as root
;;
1)
echo Can't make file unreadable for some other reason
;;
2)
if [ -f $HOME/jdb.ini ]
then
echo OK. the file is unreadable
else
echo Can't create a file
fi
;;
esac
-Dmitry
On 2015-01-21 13:05, Mattias Tobiasson wrote:
Hi,
Changes in this version:
1. Replaced the unnecessary grep from unreadable file with id | grep ...
2. Log that permission denied error message is expected.
webrev: http://cr.openjdk.java.net/~ykantser/8044419/webrev.02/
bug: https://bugs.openjdk.java.net/browse/JDK-8044419
Thanks,
Mattias
On 01/20/2015 12:25 PM, Mattias Tobiasson wrote:
Thanks for the suggestion. Your suggestion is a better way to check if
the user is root.
But if we only use that check, then we do not verify that the file is
really unreadable.
I do not know if there are any other conditions, besides running as
root, that can fail to make a file unreadable.
I think it feels safer to really try to read the unreadable file. Then
we will get the error message.
I could add a log that says the error message is expected.
And I can change the second grep to your suggestion.
Mattias
On 01/19/2015 04:13 PM, Dmitry Samersoff wrote:
Mattias,
After chmod a-r grep will display unpleasant
permission denied error for non root user
so it's better just do:
if id | grep -q 'uid=0('
then
Do root staff
else
Do non-root staff
fi
-Dmitry
On 2015-01-19 16:24, Mattias Tobiasson wrote:
Hi,
Could I please have a review of this test bug fix.
Test expects some files to be unreadable. That does not work when
running as root.
The fix is to ignore the parts for unreadable files when running as
root.
bug: https://bugs.openjdk.java.net/browse/JDK-8044419
webrev: http://cr.openjdk.java.net/~miauno/8044419/webrev.01
Tested as non-root on all platforms except embedded.
Tested as root on linux.
Thanks,
Mattias
Re: RFR: JDK-8067945: SVC jdk/test/* should be cleaned from JRE layout dependency (corrected per the review findings)
Hi Alex, Comments bellow should be removed. You don't need to make a webrev for it, only the changes are included in the final patch. test/com/sun/jdi/connect/spi/JdiLoadedByCustomLoader.java 39 // create files from given arguments and tools.jar test/com/sun/tools/attach/BasicTests.java 81 // Need to add jdk/lib/tools.jar to classpath. Best regards, Katja (not a reviewer) On 01/19/2015 05:00 PM, Alexander Kulyakhtin wrote: Hi, Could you, please, review the fix below. To adress the previous review findings any referenes to test.jdk have been removed. bug: https://bugs.openjdk.java.net/browse/JDK-8067945 webrev: http://cr.openjdk.java.net/~eistepan/~akulyakhtin/8067945/webrev.01/ References to tools.jar are removed from the tests as jdk9 drops tools.jar Thanks, Alex
Re: RFR: JDK-8067945: SVC jdk/test/* should be cleaned from JRE layout dependency (corrected per the review findings)
Looks god to me (not a reviewer). // Katja On 01/21/2015 11:23 AM, Alexander Kulyakhtin wrote: Hi Katia, Please, find attached the jdk.patch containing the changes per your findings. The patch has been made by running the webrev tool. Best regards, Alex - Original Message - From: yekaterina.kantser...@oracle.com To: alexander.kulyakh...@oracle.com, serviceability-dev@openjdk.java.net Sent: Wednesday, January 21, 2015 12:11:58 PM GMT +04:00 Abu Dhabi / Muscat Subject: Re: RFR: JDK-8067945: SVC jdk/test/* should be cleaned from JRE layout dependency (corrected per the review findings) Hi Alex, Comments bellow should be removed. You don't need to make a webrev for it, only the changes are included in the final patch. test/com/sun/jdi/connect/spi/JdiLoadedByCustomLoader.java 39 // create files from given arguments and tools.jar test/com/sun/tools/attach/BasicTests.java 81 // Need to add jdk/lib/tools.jar to classpath. Best regards, Katja (not a reviewer) On 01/19/2015 05:00 PM, Alexander Kulyakhtin wrote: Hi, Could you, please, review the fix below. To adress the previous review findings any referenes to test.jdk have been removed. bug: https://bugs.openjdk.java.net/browse/JDK-8067945 webrev: http://cr.openjdk.java.net/~eistepan/~akulyakhtin/8067945/webrev.01/ References to tools.jar are removed from the tests as jdk9 drops tools.jar Thanks, Alex
RFR(XXS): 8069296: java/lang/management/MemoryMXBean/LowMemoryTest.java should be quarantined
Hi, Could I please have a review of this very small fix. bug: https://bugs.openjdk.java.net/browse/JDK-8069296 webrev: http://cr.openjdk.java.net/~ykantser/8069296/webrev.00/ Thanks, Katja
Re: RFR(XXS): 8069296: java/lang/management/MemoryMXBean/LowMemoryTest.java should be quarantined
Thanks, Staffan! On 01/21/2015 03:04 PM, Staffan Larsen wrote: Looks good! Thanks, /Staffan On 21 jan 2015, at 13:25, Yekaterina Kantserova yekaterina.kantser...@oracle.com wrote: Hi, Could I please have a review of this very small fix. bug: https://bugs.openjdk.java.net/browse/JDK-8069296 webrev: http://cr.openjdk.java.net/~ykantser/8069296/webrev.00/ Thanks, Katja
Re: RFR(S): 6977426: sun/tools tests can intermittently fail to find app's Java pid
Hi, Thanks for all reviews! The new webrev can be found here: http://cr.openjdk.java.net/~ykantser/6977426/webrev.01/ // Katja On 12/16/2014 05:41 PM, Jaroslav Bachorik wrote: Hi Katja, This request should probably go to core-libs as well. Other than that I have just a few nits: test/java/util/logging/TestLoggerWeakRefLeak.java L57: if (Logger.contains(args[0])) - Logger.equals(args[0]) ? L127: // in LogManager.loggers that *arebeing* properly managed - *are being* callLoggerAndCount() and callAnonymousLoggerAndCount() are using the 'count' variable to always return 100. Could they be made to return a constant instead? Cheers, -JB- On 12/16/2014 04:57 PM, Yekaterina Kantserova wrote: Hi, Could I please have a review of this fix. bug: https://bugs.openjdk.java.net/browse/JDK-6977426 webrev: http://cr.openjdk.java.net/~ykantser/6977426/webrev.00/ java/util/logging/LoggerWeakRefLeak.sh and java/util/logging/AnonLoggerWeakRefLeak.sh are merged and rewritten in java. sun/tools/common/CommonTests.sh is removed because it doesn't test the product but sun/tool/common library functionality. Thanks, Katja
Re: RFR(S): 6977426: sun/tools tests can intermittently fail to find app's Java pid
Adding Erik. On 12/17/2014 10:10 AM, Yekaterina Kantserova wrote: Hi, Thanks for all reviews! The new webrev can be found here: http://cr.openjdk.java.net/~ykantser/6977426/webrev.01/ // Katja On 12/16/2014 05:41 PM, Jaroslav Bachorik wrote: Hi Katja, This request should probably go to core-libs as well. Other than that I have just a few nits: test/java/util/logging/TestLoggerWeakRefLeak.java L57: if (Logger.contains(args[0])) - Logger.equals(args[0]) ? L127: // in LogManager.loggers that *arebeing* properly managed - *are being* callLoggerAndCount() and callAnonymousLoggerAndCount() are using the 'count' variable to always return 100. Could they be made to return a constant instead? Cheers, -JB- On 12/16/2014 04:57 PM, Yekaterina Kantserova wrote: Hi, Could I please have a review of this fix. bug: https://bugs.openjdk.java.net/browse/JDK-6977426 webrev: http://cr.openjdk.java.net/~ykantser/6977426/webrev.00/ java/util/logging/LoggerWeakRefLeak.sh and java/util/logging/AnonLoggerWeakRefLeak.sh are merged and rewritten in java. sun/tools/common/CommonTests.sh is removed because it doesn't test the product but sun/tool/common library functionality. Thanks, Katja
Re: RFR(S): 6977426: sun/tools tests can intermittently fail to find app's Java pid
Daniel,
It's really funny to get such a feedback!
(1) The output from test right now looks like:
--System.out:(7/183)--
call count = 1000
instance count: 1001
call count = 2000
instance count: 1001
Finishing early due to non-increasing instance count
increasing count: 1
decreasing or the same count: 1
so printing the value of 'count' is already in there: 'call count ='.
(2) In the case we want to have an extra condition I think it should be
'count 2000'. Otherwise if the end time is passed the 'count 1000'
will result in having just one iteration (we will left the loop on 1100)
and test will fail on assert.
The test timeout value should be increased as well. Right now it's 120 +
30 sec.
* @run main/timeout=150 TestLoggerWeakRefLeak Logger
If the test will not be completed in 120 s the extra 30 s will not make
the trick and JTreg will interrupt it. What do you think will be enough?
// Katja
On 12/17/2014 10:55 AM, Daniel Fuchs wrote:
Hi Katja,
Sorry for not thinking of that when I replied earlier.
Your new test is so much more readable than the old shell
script :-)
These are minor suggestions but they might help analysis
if the test fails:
On 17/12/14 10:10, Yekaterina Kantserova wrote:
Hi,
Thanks for all reviews!
The new webrev can be found here:
http://cr.openjdk.java.net/~ykantser/6977426/webrev.01/
Not that I believe it's very likely to happen, but I wonder
if the condition to exit the while loop:
79 while (now (startTime + (LOOP_TIME_SEC * 1000))) {
should also make sure that count 1000 - and continue if not.
(Thinking of passed timeout related issues when tests where run
with -Xcomp on fastdebug builds during integration nightlies)
Possibly printing the value of 'count' before the assert
at line 103 could help with debugging too, if the test actually
fails (even though the combination of increasingCount
+ decreasingCount would give us a hint)
best regards,
-- daniel
// Katja
On 12/16/2014 05:41 PM, Jaroslav Bachorik wrote:
Hi Katja,
This request should probably go to core-libs as well.
Other than that I have just a few nits:
test/java/util/logging/TestLoggerWeakRefLeak.java
L57: if (Logger.contains(args[0])) - Logger.equals(args[0]) ?
L127: // in LogManager.loggers that *arebeing* properly managed -
*are being*
callLoggerAndCount() and callAnonymousLoggerAndCount() are using the
'count' variable to always return 100. Could they be made to return a
constant instead?
Cheers,
-JB-
On 12/16/2014 04:57 PM, Yekaterina Kantserova wrote:
Hi,
Could I please have a review of this fix.
bug: https://bugs.openjdk.java.net/browse/JDK-6977426
webrev: http://cr.openjdk.java.net/~ykantser/6977426/webrev.00/
java/util/logging/LoggerWeakRefLeak.sh and
java/util/logging/AnonLoggerWeakRefLeak.sh are merged and rewritten in
java. sun/tools/common/CommonTests.sh is removed because it doesn't
test
the product but sun/tool/common library functionality.
Thanks,
Katja
Re: RFR(S): 6977426: sun/tools tests can intermittently fail to find app's Java pid
Hopefully the last version :)
Erik has recommended to skip the whole timeout concept. Instead the test
will loop until a decreasing count is found. Otherwise the test will
timeout. The default JTreg time out is 5 minutes so it would be enough
even for slower configurations.
The new webrev can be found here:
http://cr.openjdk.java.net/~ykantser/6977426/webrev.02/
// Katja
On 12/17/2014 12:33 PM, Daniel Fuchs wrote:
On 17/12/14 12:05, Yekaterina Kantserova wrote:
Daniel,
It's really funny to get such a feedback!
(1) The output from test right now looks like:
--System.out:(7/183)--
call count = 1000
instance count: 1001
call count = 2000
instance count: 1001
Finishing early due to non-increasing instance count
increasing count: 1
decreasing or the same count: 1
so printing the value of 'count' is already in there: 'call count ='.
I meant to print the final value for count - but OK - having the
'call count' printed every 1000 count should be enough :-)
(2) In the case we want to have an extra condition I think it should be
'count 2000'. Otherwise if the end time is passed the 'count 1000'
will result in having just one iteration (we will left the loop on 1100)
and test will fail on assert.
right. my mistake. you need at least two histograms.
The test timeout value should be increased as well. Right now it's 120 +
30 sec.
* @run main/timeout=150 TestLoggerWeakRefLeak Logger
If the test will not be completed in 120 s the extra 30 s will not make
the trick and JTreg will interrupt it. What do you think will be enough?
I am not sure - but I guess what you have now should be good enough
for starter. In the problematic configurations there often is
an additional timeout factor - so in such case the real jtreg timeout
would I believe be something like 150*4 anyway - which should hopefully
leave time enough for the while loop to take two histograms. Simply
adding the extra condition to keep looping until we have at least two
should do the trick.
best regards, and thanks again for taking care of 6977426!
-- daniel
// Katja
On 12/17/2014 10:55 AM, Daniel Fuchs wrote:
Hi Katja,
Sorry for not thinking of that when I replied earlier.
Your new test is so much more readable than the old shell
script :-)
These are minor suggestions but they might help analysis
if the test fails:
On 17/12/14 10:10, Yekaterina Kantserova wrote:
Hi,
Thanks for all reviews!
The new webrev can be found here:
http://cr.openjdk.java.net/~ykantser/6977426/webrev.01/
Not that I believe it's very likely to happen, but I wonder
if the condition to exit the while loop:
79 while (now (startTime + (LOOP_TIME_SEC * 1000))) {
should also make sure that count 1000 - and continue if not.
(Thinking of passed timeout related issues when tests where run
with -Xcomp on fastdebug builds during integration nightlies)
Possibly printing the value of 'count' before the assert
at line 103 could help with debugging too, if the test actually
fails (even though the combination of increasingCount
+ decreasingCount would give us a hint)
best regards,
-- daniel
// Katja
On 12/16/2014 05:41 PM, Jaroslav Bachorik wrote:
Hi Katja,
This request should probably go to core-libs as well.
Other than that I have just a few nits:
test/java/util/logging/TestLoggerWeakRefLeak.java
L57: if (Logger.contains(args[0])) - Logger.equals(args[0]) ?
L127: // in LogManager.loggers that *arebeing* properly managed -
*are being*
callLoggerAndCount() and callAnonymousLoggerAndCount() are using the
'count' variable to always return 100. Could they be made to return a
constant instead?
Cheers,
-JB-
On 12/16/2014 04:57 PM, Yekaterina Kantserova wrote:
Hi,
Could I please have a review of this fix.
bug: https://bugs.openjdk.java.net/browse/JDK-6977426
webrev: http://cr.openjdk.java.net/~ykantser/6977426/webrev.00/
java/util/logging/LoggerWeakRefLeak.sh and
java/util/logging/AnonLoggerWeakRefLeak.sh are merged and
rewritten in
java. sun/tools/common/CommonTests.sh is removed because it doesn't
test
the product but sun/tool/common library functionality.
Thanks,
Katja
RFR(S): 6977426: sun/tools tests can intermittently fail to find app's Java pid
Hi, Could I please have a review of this fix. bug: https://bugs.openjdk.java.net/browse/JDK-6977426 webrev: http://cr.openjdk.java.net/~ykantser/6977426/webrev.00/ java/util/logging/LoggerWeakRefLeak.sh and java/util/logging/AnonLoggerWeakRefLeak.sh are merged and rewritten in java. sun/tools/common/CommonTests.sh is removed because it doesn't test the product but sun/tool/common library functionality. Thanks, Katja
Re: RFR(S): 6977426: sun/tools tests can intermittently fail to find app's Java pid
Hi Daniel, Thank you for reviewing the fix! That's what 'vm.heapHisto(-live)' does - it will request a full GC. Best regards, Katja On 12/16/2014 06:37 PM, Daniel Fuchs wrote: Hi Katja, Nice too see some more shell scripts go away! I wonder - shouldn't the test force a System.gc() at some point before taking the histogram? If not what guarantee do we have that the various weak references will be purged? best regards, -- daniel On 16/12/14 16:57, Yekaterina Kantserova wrote: Hi, Could I please have a review of this fix. bug: https://bugs.openjdk.java.net/browse/JDK-6977426 webrev: http://cr.openjdk.java.net/~ykantser/6977426/webrev.00/ java/util/logging/LoggerWeakRefLeak.sh and java/util/logging/AnonLoggerWeakRefLeak.sh are merged and rewritten in java. sun/tools/common/CommonTests.sh is removed because it doesn't test the product but sun/tool/common library functionality. Thanks, Katja
Re: RFR(XS): 8044591: [TESTBUG] com/sun/management/GarbageCollectorMXBean/GarbageCollectionNotificationp[Content]Test.java fail when -XX:+ExplicitGCInvokesConcurrent
The new webrev can be found here: http://cr.openjdk.java.net/~ykantser/8044591/webrev.02/ // Katja On 12/02/2014 09:12 PM, Yekaterina Kantserova wrote: Right! Thank you for the catch! // Katja - Original Message - From: staffan.lar...@oracle.com To: yekaterina.kantser...@oracle.com Cc: serviceability-dev@openjdk.java.net Sent: Tuesday, December 2, 2014 8:54:53 PM GMT +01:00 Amsterdam / Berlin / Bern / Rome / Stockholm / Vienna Subject: Re: RFR(XS): 8044591: [TESTBUG] com/sun/management/GarbageCollectorMXBean/GarbageCollectionNotificationp[Content]Test.java fail when -XX:+ExplicitGCInvokesConcurrent Looks good, but the test also needs to be removed from ProblemList.txt. /Staffan On 2 dec 2014, at 20:22, Yekaterina Kantserova yekaterina.kantser...@oracle.com wrote: Hi, Could I please have a review of this small fix. bug: https://bugs.openjdk.java.net/browse/JDK-8044591 webrev: http://cr.openjdk.java.net/~ykantser/8044591/webrev.00/ Thanks, Katja
Re: RFR(XS): 8044591: [TESTBUG] com/sun/management/GarbageCollectorMXBean/GarbageCollectionNotificationp[Content]Test.java fail when -XX:+ExplicitGCInvokesConcurrent
Staffan, Fredric, thanks for your reviews! // Katja On 12/03/2014 11:04 AM, Frederic Parain wrote: Looks good to me, Thank you for fixing it. Fred On 12/02/2014 08:22 PM, Yekaterina Kantserova wrote: Hi, Could I please have a review of this small fix. bug: https://bugs.openjdk.java.net/browse/JDK-8044591 webrev: http://cr.openjdk.java.net/~ykantser/8044591/webrev.00/ Thanks, Katja
RFR(XS): 8044591: [TESTBUG] com/sun/management/GarbageCollectorMXBean/GarbageCollectionNotificationp[Content]Test.java fail when -XX:+ExplicitGCInvokesConcurrent
Hi, Could I please have a review of this small fix. bug: https://bugs.openjdk.java.net/browse/JDK-8044591 webrev: http://cr.openjdk.java.net/~ykantser/8044591/webrev.00/ Thanks, Katja
Re: RFR(XS): 8044591: [TESTBUG] com/sun/management/GarbageCollectorMXBean/GarbageCollectionNotificationp[Content]Test.java fail when -XX:+ExplicitGCInvokesConcurrent
Right! Thank you for the catch! // Katja - Original Message - From: staffan.lar...@oracle.com To: yekaterina.kantser...@oracle.com Cc: serviceability-dev@openjdk.java.net Sent: Tuesday, December 2, 2014 8:54:53 PM GMT +01:00 Amsterdam / Berlin / Bern / Rome / Stockholm / Vienna Subject: Re: RFR(XS): 8044591: [TESTBUG] com/sun/management/GarbageCollectorMXBean/GarbageCollectionNotificationp[Content]Test.java fail when -XX:+ExplicitGCInvokesConcurrent Looks good, but the test also needs to be removed from ProblemList.txt. /Staffan On 2 dec 2014, at 20:22, Yekaterina Kantserova yekaterina.kantser...@oracle.com wrote: Hi, Could I please have a review of this small fix. bug: https://bugs.openjdk.java.net/browse/JDK-8044591 webrev: http://cr.openjdk.java.net/~ykantser/8044591/webrev.00/ Thanks, Katja
RFR(XS): 8066106: sun/tools/jps/TestJpsClass.java failed to remove stale attach pid file
Hi, Could I please have a review of this fix. Since VM warnings go to stderr the suggested fix is to check only stdout for expected output and ignore warnings in stderr. bug: https://bugs.openjdk.java.net/browse/JDK-8066106 webrev: http://cr.openjdk.java.net/~ykantser/8066106/webrev.00/ The tests have been run and passed on all basic platforms. Thanks, Katj
Re: RFR(XS): 8066106: sun/tools/jps/TestJpsClass.java failed to remove stale attach pid file
Hi Jaroslav, 1. Thank you for the catch! 2. Right, the test inspects jps output buffer (stdout and stderr) and expects the only jps related output appears there. But stderr may contain VM warnings. In description even Exception in thread main java.lang.RuntimeException: The line 'Java HotSpot(TM) Server VM warning: failed to remove stale attach pid file at /tmp/.java_pid13932' does not match pattern '^\\d+\\s+.*': expected true, was false is mentioned which is the true cause of the failure. The other one is the secondary failure. You can find the new webrev here: http://cr.openjdk.java.net/~ykantser/8066106/webrev.01/ Thanks, Katja On 11/28/2014 02:40 PM, Jaroslav Bachorik wrote: Hi Katja, test/sun/tools/jps/JpsHelper.java typo @ L183 - [E|e]xeption - [E|e]xception Just to be sure: the java.lang.RuntimeException: Expected to get exit value of [0] exception mentioned in the issue description is actually caused by JpsBase failing because of the stale pid file error message in the jps stderr, right? -JB- On 11/28/2014 10:47 AM, Yekaterina Kantserova wrote: Hi, Could I please have a review of this fix. Since VM warnings go to stderr the suggested fix is to check only stdout for expected output and ignore warnings in stderr. bug: https://bugs.openjdk.java.net/browse/JDK-8066106 webrev: http://cr.openjdk.java.net/~ykantser/8066106/webrev.00/ The tests have been run and passed on all basic platforms. Thanks, Katj
Re: RFR(XS): 8066106: sun/tools/jps/TestJpsClass.java failed to remove stale attach pid file
Thanks! On 11/28/2014 04:04 PM, Jaroslav Bachorik wrote: On 11/28/2014 03:51 PM, Yekaterina Kantserova wrote: Hi Jaroslav, 1. Thank you for the catch! 2. Right, the test inspects jps output buffer (stdout and stderr) and expects the only jps related output appears there. But stderr may contain VM warnings. In description even Exception in thread main java.lang.RuntimeException: The line 'Java HotSpot(TM) Server VM warning: failed to remove stale attach pid file at /tmp/.java_pid13932' does not match pattern '^\\d+\\s+.*': expected true, was false is mentioned which is the true cause of the failure. The other one is the secondary failure. You can find the new webrev here: http://cr.openjdk.java.net/~ykantser/8066106/webrev.01/ Ok. Reviewed. -JB- Thanks, Katja On 11/28/2014 02:40 PM, Jaroslav Bachorik wrote: Hi Katja, test/sun/tools/jps/JpsHelper.java typo @ L183 - [E|e]xeption - [E|e]xception Just to be sure: the java.lang.RuntimeException: Expected to get exit value of [0] exception mentioned in the issue description is actually caused by JpsBase failing because of the stale pid file error message in the jps stderr, right? -JB- On 11/28/2014 10:47 AM, Yekaterina Kantserova wrote: Hi, Could I please have a review of this fix. Since VM warnings go to stderr the suggested fix is to check only stdout for expected output and ignore warnings in stderr. bug: https://bugs.openjdk.java.net/browse/JDK-8066106 webrev: http://cr.openjdk.java.net/~ykantser/8066106/webrev.00/ The tests have been run and passed on all basic platforms. Thanks, Katj
RFR(S): 6542634: TEST BUG: MISC_REGRESSION tests need to have minimum timeouts examined
Hi, Could I please have a review of this fix. In this fix I take an opportunity to refactor sun/tools/jinfo/Basic.sh and to add more tests for jinfo utility. sun/tools/jinfo/Basic.sh is a last unstable test among tests listed inhttps://bugs.openjdk.java.net/browse/JDK-6542634 that's why I've chosen to use this bug number instead of creating a new one. bug: https://bugs.openjdk.java.net/browse/JDK-6542634 webrev: http://cr.openjdk.java.net/~ykantser/6542634/webrev.00/ The tests have been run and passed on all basic platforms. Thanks, Katja
Re: RFR(S): 6542634: TEST BUG: MISC_REGRESSION tests need to have minimum timeouts examined
Hi Jaroslav,
Thanks for the quick review!
test/sun/tools/jinfo/JInfoRunningProcessFlagTest.java is added to the
ProblemList because it contains this case:
90 private static void testInvalidFlag() throws Exception {
91 OutputAnalyzer output = JInfoHelper.jinfo(-flag, monkey);
92 assertNotEquals(output.getExitValue(), 0, A non-zero exit code should
be returned for invalid flag);
93 }
which will fail due to https://bugs.openjdk.java.net/browse/JDK-6734748.
I've been thinking about to break it out into a separate class, but then
decided to group it together with other flag-tests. What would you
recommend?
The change for L68 will be in the next review.
// Katja
On 11/18/2014 12:20 PM, Jaroslav Bachorik wrote:
Hi Katja,
test/ProblemList.txt - you are adding seemingly unrelated issue here
test/sun/tools/jinfo/JInfoHelper.java
L68 - processBuilder.command(...) could be moved to L67 and do new
ProcessBuilder(launcher.getCommand()) - it communicates the purpose
better
Cheers,
-JB-
On 11/18/2014 12:07 PM, Yekaterina Kantserova wrote:
Hi,
Could I please have a review of this fix.
In this fix I take an opportunity to refactor sun/tools/jinfo/Basic.sh
and to add more tests for jinfo utility. sun/tools/jinfo/Basic.sh is a
last unstable test among tests listed
inhttps://bugs.openjdk.java.net/browse/JDK-6542634 that's why I've
chosen to use this bug number instead of creating a new one.
bug: https://bugs.openjdk.java.net/browse/JDK-6542634
webrev: http://cr.openjdk.java.net/~ykantser/6542634/webrev.00/
The tests have been run and passed on all basic platforms.
Thanks,
Katja
Re: RFR(S): 6542634: TEST BUG: MISC_REGRESSION tests need to have minimum timeouts examined
Jaroslav, Erik, thanks!
The new webrev can be found here
http://cr.openjdk.java.net/~ykantser/6542634/webrev.01/
// Katja
On 11/18/2014 03:36 PM, Jaroslav Bachorik wrote:
On 11/18/2014 02:05 PM, Yekaterina Kantserova wrote:
Hi Jaroslav,
Thanks for the quick review!
test/sun/tools/jinfo/JInfoRunningProcessFlagTest.java is added to the
ProblemList because it contains this case:
90 private static void testInvalidFlag() throws Exception {
91 OutputAnalyzer output = JInfoHelper.jinfo(-flag,
monkey);
92 assertNotEquals(output.getExitValue(), 0, A non-zero
exit code should be returned for invalid flag);
93 }
which will fail due to https://bugs.openjdk.java.net/browse/JDK-6734748.
I've been thinking about to break it out into a separate class, but then
decided to group it together with other flag-tests. What would you
recommend?
Ok. This sounds reasonable. Let's keep it this way.
-JB-
The change for L68 will be in the next review.
// Katja
On 11/18/2014 12:20 PM, Jaroslav Bachorik wrote:
Hi Katja,
test/ProblemList.txt - you are adding seemingly unrelated issue here
test/sun/tools/jinfo/JInfoHelper.java
L68 - processBuilder.command(...) could be moved to L67 and do new
ProcessBuilder(launcher.getCommand()) - it communicates the purpose
better
Cheers,
-JB-
On 11/18/2014 12:07 PM, Yekaterina Kantserova wrote:
Hi,
Could I please have a review of this fix.
In this fix I take an opportunity to refactor sun/tools/jinfo/Basic.sh
and to add more tests for jinfo utility. sun/tools/jinfo/Basic.sh is a
last unstable test among tests listed
inhttps://bugs.openjdk.java.net/browse/JDK-6542634 that's why I've
chosen to use this bug number instead of creating a new one.
bug: https://bugs.openjdk.java.net/browse/JDK-6542634
webrev: http://cr.openjdk.java.net/~ykantser/6542634/webrev.00/
The tests have been run and passed on all basic platforms.
Thanks,
Katja
Re: RFR: 8062536: [TESTBUG] Conflicting GC combinations in jdk tests
Hi Evgeniya,
As David has pointed out these jps tests are not testing gc. The
-XX:+UseParallelGC is just an arbitrary chosen test flag. There should
not be any conflicts either since these tests are running in driver mode:
...
* @run driver TestJpsJar
...
which means no flags from above are accepted.
Thanks,
Katja
On 11/06/2014 11:05 AM, Evgeniya Stepanova wrote:
Hi David,
tag added because tests contain string
cmd.addAll(JpsHelper.getVmArgs());
and JpsHelper defines
...
public static final String[] VM_ARGS = {-Xmx512m, -XX:+UseParallelGC};
...
public static ListString getVmArgs() throws IOException {
if (testVmArgs == null) {
testVmArgs = new ArrayList();
testVmArgs.addAll(Arrays.asList(VM_ARGS));
testVmArgs.add(-XX:Flags= +
getVmFlagsFile().getAbsolutePath());
}
return testVmArgs;
}
Tests itself wouldn't fail if we use another GC, tag added for
cleanup-if we use for example SerialGC we must be sure that tests
passed with this GC, not with another one. Now you will assume that
concrete test passed with Serial GC, but it run only with Parallel GC.
Plus there is no any sense to run test twice in TC (with different GC,
since it use only Parallel)
Thanks,
Evgeniya Stepanova
On 06.11.2014 6:20, David Holmes wrote:
Hi Evgeniya,
On 6/11/2014 1:33 AM, Evgeniya Stepanova wrote:
Hi,
Please review changes for 8062536, the OpenJDK/jdk part of the
JDK-8019361
bug: https://bugs.openjdk.java.net/browse/JDK-8062536
fix: http://cr.openjdk.java.net/~eistepan/8062536/webrev.00/
Problem: Some tests explicitly set GC and fail when another GC is set
outside
I don't see why you have done this for the
test/sun/tools/jps/TestJps*.java
tests. They don't set any GC flags.
Solution: Such tests marked with the jtreg tag requires to skip test
if there is a conflict
Just wondering: Does a skipped test get a .jtr file showing it was
skipped; or does it only appear in the higher-level jtreg log?
Thanks,
David
Tested locally with different GC flags (-XX:+UseG1GC,
-XX:+UseParallelGC, -XX:+UseSerialGC, -XX:+UseConcMarkSweep and without
any GC flag). Tests are being excluded as expected. No tests failed
because of the conflict.
Thanks,
Evgeniya Stepanova
//
--
/Evgeniya Stepanova/
Re: RFR: 8062536: [TESTBUG] Conflicting GC combinations in jdk tests
If it's confusing the tests are using -XX:+UseParallelGC we can change
them to use some other neutral flag.
Plus there is no any sense to run test twice in TC (with different GC,
since it use only Parallel)
I agree it's meaningless to run them with all possible GC combinations
because these tests are GC neutral.
// Katja
On 11/06/2014 11:27 AM, Yekaterina Kantserova wrote:
Hi Evgeniya,
As David has pointed out these jps tests are not testing gc. The
-XX:+UseParallelGC is just an arbitrary chosen test flag. There should
not be any conflicts either since these tests are running in driver mode:
...
* @run driver TestJpsJar
...
which means no flags from above are accepted.
Thanks,
Katja
On 11/06/2014 11:05 AM, Evgeniya Stepanova wrote:
Hi David,
tag added because tests contain string
cmd.addAll(JpsHelper.getVmArgs());
and JpsHelper defines
...
public static final String[] VM_ARGS = {-Xmx512m,
-XX:+UseParallelGC};
...
public static ListString getVmArgs() throws IOException {
if (testVmArgs == null) {
testVmArgs = new ArrayList();
testVmArgs.addAll(Arrays.asList(VM_ARGS));
testVmArgs.add(-XX:Flags= +
getVmFlagsFile().getAbsolutePath());
}
return testVmArgs;
}
Tests itself wouldn't fail if we use another GC, tag added for
cleanup-if we use for example SerialGC we must be sure that tests
passed with this GC, not with another one. Now you will assume that
concrete test passed with Serial GC, but it run only with Parallel
GC. Plus there is no any sense to run test twice in TC (with
different GC, since it use only Parallel)
Thanks,
Evgeniya Stepanova
On 06.11.2014 6:20, David Holmes wrote:
Hi Evgeniya,
On 6/11/2014 1:33 AM, Evgeniya Stepanova wrote:
Hi,
Please review changes for 8062536, the OpenJDK/jdk part of the
JDK-8019361
bug: https://bugs.openjdk.java.net/browse/JDK-8062536
fix: http://cr.openjdk.java.net/~eistepan/8062536/webrev.00/
Problem: Some tests explicitly set GC and fail when another GC is set
outside
I don't see why you have done this for the
test/sun/tools/jps/TestJps*.java
tests. They don't set any GC flags.
Solution: Such tests marked with the jtreg tag requires to skip test
if there is a conflict
Just wondering: Does a skipped test get a .jtr file showing it was
skipped; or does it only appear in the higher-level jtreg log?
Thanks,
David
Tested locally with different GC flags (-XX:+UseG1GC,
-XX:+UseParallelGC, -XX:+UseSerialGC, -XX:+UseConcMarkSweep and
without
any GC flag). Tests are being excluded as expected. No tests failed
because of the conflict.
Thanks,
Evgeniya Stepanova
//
--
/Evgeniya Stepanova/
