Re: [9] RFR (M) 8054888: Runtime: Add Diagnostic Command that prints the class hierarchy
On 2015-02-11 04:13, Chris Plummer wrote: In general I think this looks very good. Simple and well-commented code to follow. I am missing a test, though. Please look at the hotspot/test/serviceability/dcmd set of tests. Added. Your test is based on DcmdUtil.java which was removed last week (see [0]). If you pull new changes from hs-rt/hotspot, you'll see plenty of tests using the new DCMD utility classes in testlibrary. Also, the new tests are divided in different subdirectories depending on the commands, so as your test exercises VM.class_hierarchy it should go in .../dcmd/vm/ as opposed to just .../dcmd/. Thanks, Mikael [0] http://hg.openjdk.java.net/jdk9/hs-rt/hotspot/file/tip/test/serviceability/dcmd
Re: RFR(XXS): 8072472: serviceability/dcmd/framework/* should be quarantined
Thanks for the review. Would you help me push this (exported changeset attached)? Thanks, Mikael On 2015-02-04 17:56, Jaroslav Bachorik wrote: Looks good! -JB- On 4.2.2015 16:13, Mikael Auno wrote: Hi, could I have a review for this small change to ignore three tests. Issue: https://bugs.openjdk.java.net/browse/JDK-8072472 Webrev: http://cr.openjdk.java.net/~miauno/8072472/webrev.00/ Thanks, Mikael # HG changeset patch # User miauno # Date 1423062589 -3600 # Node ID c1552a4dfc143973516a730a0dc87e25ca946eb9 # Parent 79f4205419d2118a7a5a548fd24ca4a82a50460b 8072472: serviceability/dcmd/framework/* should be quarantined Reviewed-by: jbachorik diff --git a/test/serviceability/dcmd/framework/HelpTest.java b/test/serviceability/dcmd/framework/HelpTest.java --- a/test/serviceability/dcmd/framework/HelpTest.java +++ b/test/serviceability/dcmd/framework/HelpTest.java @@ -33,6 +33,7 @@ * @test * @summary Test of diagnostic command help (tests all DCMD executors) * @library /testlibrary + * @ignore 8072440 * @build com.oracle.java.testlibrary.* * @build com.oracle.java.testlibrary.dcmd.* * @run testng/othervm -XX:+UsePerfData HelpTest diff --git a/test/serviceability/dcmd/framework/InvalidCommandTest.java b/test/serviceability/dcmd/framework/InvalidCommandTest.java --- a/test/serviceability/dcmd/framework/InvalidCommandTest.java +++ b/test/serviceability/dcmd/framework/InvalidCommandTest.java @@ -33,6 +33,7 @@ * @test * @summary Test of invalid diagnostic command (tests all DCMD executors) * @library /testlibrary + * @ignore 8072440 * @build com.oracle.java.testlibrary.* * @build com.oracle.java.testlibrary.dcmd.* * @run testng/othervm -XX:+UsePerfData InvalidCommandTest diff --git a/test/serviceability/dcmd/framework/VMVersionTest.java b/test/serviceability/dcmd/framework/VMVersionTest.java --- a/test/serviceability/dcmd/framework/VMVersionTest.java +++ b/test/serviceability/dcmd/framework/VMVersionTest.java @@ -34,6 +34,7 @@ * @test * @summary Test of diagnostic command VM.version (tests all DCMD executors) * @library /testlibrary + * @ignore 8072440 * @build com.oracle.java.testlibrary.* * @build com.oracle.java.testlibrary.dcmd.* * @run testng/othervm -XX:+UsePerfData VMVersionTest
RFR(XXS): 8072472: serviceability/dcmd/framework/* should be quarantined
Hi, could I have a review for this small change to ignore three tests. Issue: https://bugs.openjdk.java.net/browse/JDK-8072472 Webrev: http://cr.openjdk.java.net/~miauno/8072472/webrev.00/ Thanks, Mikael
RFR(XS): 8072401, 8072403, 8072405: Small fixes to newly added DCMD tests
Hi, could I please have some reviews for these very small fixes. Webrev: http://cr.openjdk.java.net/~miauno/8072401_8072403_8072405/webrev.00/ Issues: Some of the newly added DCMD tests fail due to lack of -XX:+UsePerfData https://bugs.openjdk.java.net/browse/JDK-8072401 HeapDumpTest and HeapDumpAllTest fails to find jhat in non-JDK runs https://bugs.openjdk.java.net/browse/JDK-8072403 DCMD tests needs at least compact3 profile https://bugs.openjdk.java.net/browse/JDK-8072405 Thanks, Mikael
Re: RFR(XS): 8072401, 8072403, 8072405: Small fixes to newly added DCMD tests
Thanks for the review. Could you help push this for me? The exported
changsets are attached.
Thanks,
Mikael
On 2015-02-03 16:44, Jaroslav Bachorik wrote:
Looks good.
-JB-
On 3.2.2015 16:27, Mikael Auno wrote:
Hi, could I please have some reviews for these very small fixes.
Webrev:
http://cr.openjdk.java.net/~miauno/8072401_8072403_8072405/webrev.00/
Issues:
Some of the newly added DCMD tests fail due to lack of -XX:+UsePerfData
https://bugs.openjdk.java.net/browse/JDK-8072401
HeapDumpTest and HeapDumpAllTest fails to find jhat in non-JDK runs
https://bugs.openjdk.java.net/browse/JDK-8072403
DCMD tests needs at least compact3 profile
https://bugs.openjdk.java.net/browse/JDK-8072405
Thanks,
Mikael
# HG changeset patch
# User miauno
# Date 1422962800 -3600
# Node ID 01cc42eb107afca5408e30a0a09c251be7976b98
# Parent 190387dac81353a3bc1dddbb328a60f2cb85500a
8072401: [TESTBUG] Some of the newly added DCMD tests fail due to lack of -XX:+UsePerfData
Reviewed-by: jbachorik
diff --git a/test/serviceability/dcmd/framework/HelpTest.java b/test/serviceability/dcmd/framework/HelpTest.java
--- a/test/serviceability/dcmd/framework/HelpTest.java
+++ b/test/serviceability/dcmd/framework/HelpTest.java
@@ -35,7 +35,7 @@
* @library /testlibrary
* @build com.oracle.java.testlibrary.*
* @build com.oracle.java.testlibrary.dcmd.*
- * @run testng HelpTest
+ * @run testng/othervm -XX:+UsePerfData HelpTest
*/
public class HelpTest {
public void run(CommandExecutor executor) {
diff --git a/test/serviceability/dcmd/framework/InvalidCommandTest.java b/test/serviceability/dcmd/framework/InvalidCommandTest.java
--- a/test/serviceability/dcmd/framework/InvalidCommandTest.java
+++ b/test/serviceability/dcmd/framework/InvalidCommandTest.java
@@ -35,7 +35,7 @@
* @library /testlibrary
* @build com.oracle.java.testlibrary.*
* @build com.oracle.java.testlibrary.dcmd.*
- * @run testng InvalidCommandTest
+ * @run testng/othervm -XX:+UsePerfData InvalidCommandTest
*/
public class InvalidCommandTest {
diff --git a/test/serviceability/dcmd/framework/VMVersionTest.java b/test/serviceability/dcmd/framework/VMVersionTest.java
--- a/test/serviceability/dcmd/framework/VMVersionTest.java
+++ b/test/serviceability/dcmd/framework/VMVersionTest.java
@@ -36,7 +36,7 @@
* @library /testlibrary
* @build com.oracle.java.testlibrary.*
* @build com.oracle.java.testlibrary.dcmd.*
- * @run testng VMVersionTest
+ * @run testng/othervm -XX:+UsePerfData VMVersionTest
*/
public class VMVersionTest {
public void run(CommandExecutor executor) {
# HG changeset patch
# User miauno
# Date 1422964153 -3600
# Node ID bca4aed2d75f4025d3d0929235d88155e36a0731
# Parent 01cc42eb107afca5408e30a0a09c251be7976b98
8072403: [TESTBUG] HeapDumpTest and HeapDumpAllTest fails to find jhat in non-JDK runs
Reviewed-by: jbachorik
diff --git a/test/serviceability/dcmd/gc/HeapDumpTest.java b/test/serviceability/dcmd/gc/HeapDumpTest.java
--- a/test/serviceability/dcmd/gc/HeapDumpTest.java
+++ b/test/serviceability/dcmd/gc/HeapDumpTest.java
@@ -51,7 +51,7 @@
}
private void verifyHeapDump(String fileName) {
-String jhat = JDKToolFinder.getTestJDKTool(jhat);
+String jhat = JDKToolFinder.getJDKTool(jhat);
String[] cmd = { jhat, -parseonly, true, fileName };
ProcessBuilder pb = new ProcessBuilder(cmd);
# HG changeset patch
# User miauno
# Date 1422964173 -3600
# Node ID 926a6fbb8d1ab01ef64e93b817f4c60705d5e37f
# Parent bca4aed2d75f4025d3d0929235d88155e36a0731
8072405: [TESTBUG] DCMD tests needs at least compact3 profile
Reviewed-by: jbachorik
diff --git a/test/TEST.groups b/test/TEST.groups
--- a/test/TEST.groups
+++ b/test/TEST.groups
@@ -145,7 +145,8 @@
gc/survivorAlignment \
runtime/InternalApi/ThreadCpuTimesDeadlock.java \
serviceability/threads/TestFalseDeadLock.java \
- compiler/codecache/jmx
+ compiler/codecache/jmx \
+ serviceability/dcmd
# Compact 2 adds full VM tests
compact2 = \
Re: RFR(L): 8071908, 8071909: Port internal Diagnostic Command tests and test framework to jtreg (plus some testlibrary changes)
Thanks everyone for the reviews. This has now been pushed. Mikael On 2015-01-30 21:19, Mikael Auno wrote: Thanks Jaroslav! I'll leave this open for further comments if someone else has them until Monday afternoon (CET). If there are no more comments by then I'll get it pushed at that time. Mikael On 2015-01-30 20:48, Jaroslav Bachorik wrote: Nice! For me it's good to go. -JB- On 30.1.2015 20:32, Mikael Auno wrote: Jaroslav, First of all, thanks for the quick response and sorry for my slow one (your message didn't sort into the mail folder I expected, so didn't see it until now). Secondly, all good comments; all fixed. Updated webrev: http://cr.openjdk.java.net/~miauno/8071908_8071909/webrev.02/ Thanks, Mikael On 2015-01-30 11:18, Jaroslav Bachorik wrote: Hi Mikael, it's great to see this moving forward! Comments follow: * instead of throwing a RuntimeException from within the test classes you could use o.testng.Assert.fail(...) method * all the newly introduced tests are missing @summary * test/serviceability/dcmd/compiler/CodelistTest.java L43 - unused import L68 - an unused, commented out, line * test/testlibrary/com/oracle/java/testlibrary/OutputAnalyzer.java L436-438 - use Arrays.asList() * test/serviceability/dcmd/vm/UptimeTest.java L44 - spurious wakeups may cause the test fail intermittently; should make sure the wait was at least 'someUptime' seconds long -JB- On 30.1.2015 10:44, Mikael Auno wrote: Hi, could I please some reviews for this test port? Issues: https://bugs.openjdk.java.net/browse/JDK-8071908 https://bugs.openjdk.java.net/browse/JDK-8071909 Webrev: http://cr.openjdk.java.net/~miauno/8071908_8071909/webrev.00/ Read on for the rationale on a few questions that might arise. * Why two issues? These changes are mainly a port of the Diagnostic Command (DCMD) tests and corresponding framework/utility classes from an internal (non-open) test framework to jtreg. The reason for the two issues is that the changes depend on a few modifications to testlibrary that are available in jdk/test but not yet in hotspot/test, as well as a small new addition to OutputAnalyzer, that are not specific to main subject (i.e. the DCMD tests). To keep the history as clean and coherent as possible, those changes will go in under JDK-8071909 while the new tests and DCMD-related additions to testlibrary go in under JDK-8071908. * Isn't there already utility classes for calling Diagnostic Commands? The main idea with the new utility classes in testlibrary is to provide a single interface to call Diagnostic Commands from tests, regardless of the transport used (e.g. jcmd or JMX). There are a few tests scattered around jdk/test and hotspot/test today that already utilize Diagnostic Commands in some way, but they either use different utility classes for this in different places or just do it directly in the test. Also, some of these utility classes or tests go through jcmd and some through JMX (most often without any real requirement for one transport over the other in the test). All this means that there are, today, numerous different implementations for calling Diagnostic Commands, and consequently a lot of code duplication. These utility classes are meant to replace all of these implementations, and with a single interface regardless of the transport at that. * You've missed this or that test using one of the existing utility classes! This is by design. In order to keep the change at a more manageable size and to get the core of this change in sooner, we've chosen to do this transition in multiple steps. The first of those steps is what is in this review request; the core utility classes, the tests ported from the internal test framework and the adaption of the tests already in hotspot/test/serviceability/dcmd (since they happened to reside in the directory where we wanted to put the ported tests). When this is integrated and have gone a few rounds through nightly testing, the adaption of other tests in hotspot/test will follow, and after that jdk/test. Thanks, Mikael
Re: RFR(L): 8071908, 8071909: Port internal Diagnostic Command tests and test framework to jtreg (plus some testlibrary changes)
Thanks Jaroslav! I'll leave this open for further comments if someone else has them until Monday afternoon (CET). If there are no more comments by then I'll get it pushed at that time. Mikael On 2015-01-30 20:48, Jaroslav Bachorik wrote: Nice! For me it's good to go. -JB- On 30.1.2015 20:32, Mikael Auno wrote: Jaroslav, First of all, thanks for the quick response and sorry for my slow one (your message didn't sort into the mail folder I expected, so didn't see it until now). Secondly, all good comments; all fixed. Updated webrev: http://cr.openjdk.java.net/~miauno/8071908_8071909/webrev.02/ Thanks, Mikael On 2015-01-30 11:18, Jaroslav Bachorik wrote: Hi Mikael, it's great to see this moving forward! Comments follow: * instead of throwing a RuntimeException from within the test classes you could use o.testng.Assert.fail(...) method * all the newly introduced tests are missing @summary * test/serviceability/dcmd/compiler/CodelistTest.java L43 - unused import L68 - an unused, commented out, line * test/testlibrary/com/oracle/java/testlibrary/OutputAnalyzer.java L436-438 - use Arrays.asList() * test/serviceability/dcmd/vm/UptimeTest.java L44 - spurious wakeups may cause the test fail intermittently; should make sure the wait was at least 'someUptime' seconds long -JB- On 30.1.2015 10:44, Mikael Auno wrote: Hi, could I please some reviews for this test port? Issues: https://bugs.openjdk.java.net/browse/JDK-8071908 https://bugs.openjdk.java.net/browse/JDK-8071909 Webrev: http://cr.openjdk.java.net/~miauno/8071908_8071909/webrev.00/ Read on for the rationale on a few questions that might arise. * Why two issues? These changes are mainly a port of the Diagnostic Command (DCMD) tests and corresponding framework/utility classes from an internal (non-open) test framework to jtreg. The reason for the two issues is that the changes depend on a few modifications to testlibrary that are available in jdk/test but not yet in hotspot/test, as well as a small new addition to OutputAnalyzer, that are not specific to main subject (i.e. the DCMD tests). To keep the history as clean and coherent as possible, those changes will go in under JDK-8071909 while the new tests and DCMD-related additions to testlibrary go in under JDK-8071908. * Isn't there already utility classes for calling Diagnostic Commands? The main idea with the new utility classes in testlibrary is to provide a single interface to call Diagnostic Commands from tests, regardless of the transport used (e.g. jcmd or JMX). There are a few tests scattered around jdk/test and hotspot/test today that already utilize Diagnostic Commands in some way, but they either use different utility classes for this in different places or just do it directly in the test. Also, some of these utility classes or tests go through jcmd and some through JMX (most often without any real requirement for one transport over the other in the test). All this means that there are, today, numerous different implementations for calling Diagnostic Commands, and consequently a lot of code duplication. These utility classes are meant to replace all of these implementations, and with a single interface regardless of the transport at that. * You've missed this or that test using one of the existing utility classes! This is by design. In order to keep the change at a more manageable size and to get the core of this change in sooner, we've chosen to do this transition in multiple steps. The first of those steps is what is in this review request; the core utility classes, the tests ported from the internal test framework and the adaption of the tests already in hotspot/test/serviceability/dcmd (since they happened to reside in the directory where we wanted to put the ported tests). When this is integrated and have gone a few rounds through nightly testing, the adaption of other tests in hotspot/test will follow, and after that jdk/test. Thanks, Mikael
Re: RFR(L): 8071908, 8071909: Port internal Diagnostic Command tests and test framework to jtreg (plus some testlibrary changes)
I found an issue on Windows. DCMD output has Unix newlines even on Windows (and actually mixed Unix and Windows newlines when running through jcmd), so I've updated OutputAnalyzer.asLines() to split on (\\r\\n|\\n|\\r) instead of line.separator. I've also added better debug output in CodeCacheTest.java. New webrev: http://cr.openjdk.java.net/~miauno/8071908_8071909/webrev.01/ Thanks, Mikael On 2015-01-30 10:44, Mikael Auno wrote: Hi, could I please some reviews for this test port? Issues: https://bugs.openjdk.java.net/browse/JDK-8071908 https://bugs.openjdk.java.net/browse/JDK-8071909 Webrev: http://cr.openjdk.java.net/~miauno/8071908_8071909/webrev.00/ Read on for the rationale on a few questions that might arise. * Why two issues? These changes are mainly a port of the Diagnostic Command (DCMD) tests and corresponding framework/utility classes from an internal (non-open) test framework to jtreg. The reason for the two issues is that the changes depend on a few modifications to testlibrary that are available in jdk/test but not yet in hotspot/test, as well as a small new addition to OutputAnalyzer, that are not specific to main subject (i.e. the DCMD tests). To keep the history as clean and coherent as possible, those changes will go in under JDK-8071909 while the new tests and DCMD-related additions to testlibrary go in under JDK-8071908. * Isn't there already utility classes for calling Diagnostic Commands? The main idea with the new utility classes in testlibrary is to provide a single interface to call Diagnostic Commands from tests, regardless of the transport used (e.g. jcmd or JMX). There are a few tests scattered around jdk/test and hotspot/test today that already utilize Diagnostic Commands in some way, but they either use different utility classes for this in different places or just do it directly in the test. Also, some of these utility classes or tests go through jcmd and some through JMX (most often without any real requirement for one transport over the other in the test). All this means that there are, today, numerous different implementations for calling Diagnostic Commands, and consequently a lot of code duplication. These utility classes are meant to replace all of these implementations, and with a single interface regardless of the transport at that. * You've missed this or that test using one of the existing utility classes! This is by design. In order to keep the change at a more manageable size and to get the core of this change in sooner, we've chosen to do this transition in multiple steps. The first of those steps is what is in this review request; the core utility classes, the tests ported from the internal test framework and the adaption of the tests already in hotspot/test/serviceability/dcmd (since they happened to reside in the directory where we wanted to put the ported tests). When this is integrated and have gone a few rounds through nightly testing, the adaption of other tests in hotspot/test will follow, and after that jdk/test. Thanks, Mikael
Re: RFR(L): 8071908, 8071909: Port internal Diagnostic Command tests and test framework to jtreg (plus some testlibrary changes)
Jaroslav, First of all, thanks for the quick response and sorry for my slow one (your message didn't sort into the mail folder I expected, so didn't see it until now). Secondly, all good comments; all fixed. Updated webrev: http://cr.openjdk.java.net/~miauno/8071908_8071909/webrev.02/ Thanks, Mikael On 2015-01-30 11:18, Jaroslav Bachorik wrote: Hi Mikael, it's great to see this moving forward! Comments follow: * instead of throwing a RuntimeException from within the test classes you could use o.testng.Assert.fail(...) method * all the newly introduced tests are missing @summary * test/serviceability/dcmd/compiler/CodelistTest.java L43 - unused import L68 - an unused, commented out, line * test/testlibrary/com/oracle/java/testlibrary/OutputAnalyzer.java L436-438 - use Arrays.asList() * test/serviceability/dcmd/vm/UptimeTest.java L44 - spurious wakeups may cause the test fail intermittently; should make sure the wait was at least 'someUptime' seconds long -JB- On 30.1.2015 10:44, Mikael Auno wrote: Hi, could I please some reviews for this test port? Issues: https://bugs.openjdk.java.net/browse/JDK-8071908 https://bugs.openjdk.java.net/browse/JDK-8071909 Webrev: http://cr.openjdk.java.net/~miauno/8071908_8071909/webrev.00/ Read on for the rationale on a few questions that might arise. * Why two issues? These changes are mainly a port of the Diagnostic Command (DCMD) tests and corresponding framework/utility classes from an internal (non-open) test framework to jtreg. The reason for the two issues is that the changes depend on a few modifications to testlibrary that are available in jdk/test but not yet in hotspot/test, as well as a small new addition to OutputAnalyzer, that are not specific to main subject (i.e. the DCMD tests). To keep the history as clean and coherent as possible, those changes will go in under JDK-8071909 while the new tests and DCMD-related additions to testlibrary go in under JDK-8071908. * Isn't there already utility classes for calling Diagnostic Commands? The main idea with the new utility classes in testlibrary is to provide a single interface to call Diagnostic Commands from tests, regardless of the transport used (e.g. jcmd or JMX). There are a few tests scattered around jdk/test and hotspot/test today that already utilize Diagnostic Commands in some way, but they either use different utility classes for this in different places or just do it directly in the test. Also, some of these utility classes or tests go through jcmd and some through JMX (most often without any real requirement for one transport over the other in the test). All this means that there are, today, numerous different implementations for calling Diagnostic Commands, and consequently a lot of code duplication. These utility classes are meant to replace all of these implementations, and with a single interface regardless of the transport at that. * You've missed this or that test using one of the existing utility classes! This is by design. In order to keep the change at a more manageable size and to get the core of this change in sooner, we've chosen to do this transition in multiple steps. The first of those steps is what is in this review request; the core utility classes, the tests ported from the internal test framework and the adaption of the tests already in hotspot/test/serviceability/dcmd (since they happened to reside in the directory where we wanted to put the ported tests). When this is integrated and have gone a few rounds through nightly testing, the adaption of other tests in hotspot/test will follow, and after that jdk/test. Thanks, Mikael
RFR(L): 8071908, 8071909: Port internal Diagnostic Command tests and test framework to jtreg (plus some testlibrary changes)
Hi, could I please some reviews for this test port? Issues: https://bugs.openjdk.java.net/browse/JDK-8071908 https://bugs.openjdk.java.net/browse/JDK-8071909 Webrev: http://cr.openjdk.java.net/~miauno/8071908_8071909/webrev.00/ Read on for the rationale on a few questions that might arise. * Why two issues? These changes are mainly a port of the Diagnostic Command (DCMD) tests and corresponding framework/utility classes from an internal (non-open) test framework to jtreg. The reason for the two issues is that the changes depend on a few modifications to testlibrary that are available in jdk/test but not yet in hotspot/test, as well as a small new addition to OutputAnalyzer, that are not specific to main subject (i.e. the DCMD tests). To keep the history as clean and coherent as possible, those changes will go in under JDK-8071909 while the new tests and DCMD-related additions to testlibrary go in under JDK-8071908. * Isn't there already utility classes for calling Diagnostic Commands? The main idea with the new utility classes in testlibrary is to provide a single interface to call Diagnostic Commands from tests, regardless of the transport used (e.g. jcmd or JMX). There are a few tests scattered around jdk/test and hotspot/test today that already utilize Diagnostic Commands in some way, but they either use different utility classes for this in different places or just do it directly in the test. Also, some of these utility classes or tests go through jcmd and some through JMX (most often without any real requirement for one transport over the other in the test). All this means that there are, today, numerous different implementations for calling Diagnostic Commands, and consequently a lot of code duplication. These utility classes are meant to replace all of these implementations, and with a single interface regardless of the transport at that. * You've missed this or that test using one of the existing utility classes! This is by design. In order to keep the change at a more manageable size and to get the core of this change in sooner, we've chosen to do this transition in multiple steps. The first of those steps is what is in this review request; the core utility classes, the tests ported from the internal test framework and the adaption of the tests already in hotspot/test/serviceability/dcmd (since they happened to reside in the directory where we wanted to put the ported tests). When this is integrated and have gone a few rounds through nightly testing, the adaption of other tests in hotspot/test will follow, and after that jdk/test. Thanks, Mikael
Re: RFR(XXS): 8068584: Compiler attach tests should be quarantined
Looks good to me. I'm not a reviewer though. Mikael On 2015-01-08 13:45, Mattias Tobiasson wrote: Thanks for the explanation. I have moved the @ignore tags to directly before the first @run tag. webrev: http://cr.openjdk.java.net/~ykantser/8068584/webrev.01/ http://cr.openjdk.java.net/%7Eykantser/8068584/webrev.01/ Mattias On 01/08/2015 11:21 AM, Mikael Auno wrote: On 2015-01-08 11:04, Mattias Tobiasson wrote: Hi, Could I please have a review of this bug quarantine. bug: https://bugs.openjdk.java.net/browse/JDK-8068584 webrev: http://cr.openjdk.java.net/~ykantser/8068584/webrev.00/ Thanks, Mattias Mattias, In the two **/Launcher.java tests, you must place @ignore after the @library line, otherwise there will be a Parse Exception: `@library' must appear before first `@run'. (The error message is, to say the least, a bit unintuitive until you figure out that @ignore is just an alias for @run ignore.) Mikael
RFR(XXS): 8068718: com/sun/jdi/CatchPatternTest.sh should be quarantined
Hi, Could I please have a review of addition to ProblemList. bug: https://bugs.openjdk.java.net/browse/JDK-8068718 webrev: http://cr.openjdk.java.net/~miauno/8068718/webrev.00/ Thanks, Mikael
Re: RFR(XXS): 8068718: com/sun/jdi/CatchPatternTest.sh should be quarantined
Thanks for the review! Mikael On 2015-01-09 13:59, Jaroslav Bachorik wrote: Looks good! -JB- On 9.1.2015 13:54, Mikael Auno wrote: Hi, Could I please have a review of addition to ProblemList. bug: https://bugs.openjdk.java.net/browse/JDK-8068718 webrev: http://cr.openjdk.java.net/~miauno/8068718/webrev.00/ Thanks, Mikael
Re: [9] RFR (M) 8054888: Runtime: Add Diagnostic Command that prints the class hierarchy
On 2015-01-08 00:29, Chris Plummer wrote: At the moment not much testing has been done other than running the DCMD and looking at the output. I'll do more once it's clear the code has settled. I would like to know if there are any existing tests for GC.class_stats and GC.class_histogram (there are none in the test directory). If so, possibly one could serve as the basis for a new test for VM.class_hierarchy. Unfortunately, there are no open tests either of them as of yet. There are, however, internal ones. I can give you the details offline. Mikael
Re: RFR: 8064799: [TESTBUG] JT-Reg Serviceability tests to be run as part of JPRT submit job
On 2014-11-14 09:53, Alan Bateman wrote: On 14/11/2014 08:40, Staffan Larsen wrote: : So the goal here has been to increase the test coverage of hotspot jprt push jobs, but with a limited impact on execution time. This is all to make sure hotspot changes do no break serviceability features. While it would be great to run all tests at all times, we don’t have time for that. Mikael has been doing code coverage analysis to find the subset of test that provides the biggest bang for the buck. Starting with JDI is as good as any place to start. I agree that listing individual tests is not particularly appealing, but I don’t see many other options. We could possibly use @key tags to select the tests but there isn’t much support in makefiles and jprt for that if I recall. We could use sub-folders, but that quickly gets out of hand. We could move the _sanity lists to one place in the file to make it easier to see the rest. My main concern is keeping the test group hierarchy easy to understand and maintain. It has to be easy to identify any tests that aren't run by any of the top-level groups for example. For serviceability then the original idea was to have the jdk_svc group that ran all of the tests, this in turn consisted of 5 sub-groups to cover the various areas. This sub-groups will execute concurrently on different JPRT clients and works reasonably well, albeit with some imbalance in the execution time. The real definitions of the groups end just after the section with the comment Client area groups so if you can move the new sanity groups down to below that with a good comment to explain what they are then it should okay. At some point we need to look at removing completely the Profile based ... groups at the end. They are completely unmaintainable and there are much better ways of doing this now (with @requires). Here's an updated webrev with your proposed changes. http://cr.openjdk.java.net/~miauno/8064799/webrev.01/ Mikael
Re: RFR: 8064799: [TESTBUG] JT-Reg Serviceability tests to be run as part of JPRT submit job
On 2014-11-14 10:55, Alan Bateman wrote: On 14/11/2014 09:32, Mikael Auno wrote: : Here's an updated webrev with your proposed changes. http://cr.openjdk.java.net/~miauno/8064799/webrev.01/ This looks to okay, thanks for taking the concern on board. -Alan Perfect. Thanks for the reviews, all of you. Mikael
RFR: 8064799: [TESTBUG] JT-Reg Serviceability tests to be run as part of JPRT submit job
Hi, Could I please get a review of this addition of SVC tests to JPRT submit jobs. So far, I'm only adding JDI tests as those are the only ones I have completed code coverage analysis on to determine the best subset to add. The other areas will be added too, but I'm adding these now to get the ball rolling asap. I've run these through JPRT once already without failures and have got two more runs in the pipe. I've also looked through the history for these tests and found that they do not have any known instabilities to worry about. Issue: https://bugs.openjdk.java.net/browse/JDK-8064799 Webrev: http://cr.openjdk.java.net/~miauno/8064799/webrev.00/ Thanks, Mikael
Re: RFR: 8064799: [TESTBUG] JT-Reg Serviceability tests to be run as part of JPRT submit job
On 2014-11-13 14:56, Mikael Auno wrote: Hi, Could I please get a review of this addition of SVC tests to JPRT submit jobs. So far, I'm only adding JDI tests as those are the only ones I have completed code coverage analysis on to determine the best subset to add. The other areas will be added too, but I'm adding these now to get the ball rolling asap. I've run these through JPRT once already without failures and have got two more runs in the pipe. I've also looked through the history for these tests and found that they do not have any known instabilities to worry about. Issue: https://bugs.openjdk.java.net/browse/JDK-8064799 Webrev: http://cr.openjdk.java.net/~miauno/8064799/webrev.00/ The additional JPRT runs have completed now and have no failures. Here are also the duration (in seconds) for each test on each platform to in case anyone wonders (com/sun/jdi prefix stripped out): - | | lin_i586-c1 | lin_i586-c2 | lin_x64-c2 | osx_x64-c2 | sol_sparcv9-c2 | sol_x64-c2 | win_i586-c1 | win_i586-c2 | win_x64-c2 | - | .../AcceptTimeout| 1.24 | 1.277 | 1.308 | 1.349 | 1.54 | 1.204 | 2.184 | 2.293 | 2.34 | | .../AccessSpecifierTest | 1.689 | 1.883 | 2.021 | 2.303 | 4.892 | 2.048 | 1.332 | 1.707 | 2.19 | | .../AfterThreadDeathTest | 0.855 | 0.748 | 0.815 | 0.605 | 1.098 | 0.691 | 0.299 | 0.424 | 1.683 | | .../ArrayRangeTest | 0.659 | 0.823 | 0.794 | 0.702 | 1.445 | 0.837 | 0.783 | 0.503 | 1.267 | | .../ConstantPoolInfo | 0.589 | 0.74 | 0.791 | 0.621 | 1.067 | 0.607 | 0.315 | 0.408 | 0.674 | | .../CountFilterTest | 0.588 | 0.638 | 0.729 | 0.617 | 1.068 | 0.618 | 0.3 | 0.502 | 0.674 | | .../EarlyReturnNegativeTest | 0.724 | 0.8 | 0.824 | 0.675 | 1.186 | 0.642 | 0.362 | 0.627 | 0.736 | | .../EarlyReturnTest | 1.218 | 1.164 | 1.295 | 1.207 | 1.962 | 1.307 | 0.72 | 1.189 | 1.242 | | .../FieldWatchpoints | 0.616 | 0.628 | 0.728 | 0.6 | 1.052 | 0.683 | 0.3 | 0.408 | 0.674 | | .../FramesTest | 0.598 | 0.696 | 0.741 | 0.601 | 1.006 | 0.592 | 0.299 | 0.425 | 0.627 | | .../InstanceFilter | 0.604 | 0.677 | 0.696 | 0.587 | 1.005 | 0.608 | 0.284 | 0.393 | 0.69 | | .../InterfaceMethodsTest | 0.706 | 0.83 | 0.837 | 0.69 | 1.193 | 0.762 | 0.362 | 1.032 | 0.752 | | .../InvokeTest | 0.719 | 0.788 | 0.861 | 0.71 | 1.196 | 0.647 | 0.377 | 0.752 | 0.721 | | .../LocalVariableEqual | 0.66 | 0.662 | 0.714 | 0.622 | 1.087 | 0.715 | 0.315 | 0.383 | 0.612 | | .../LocationTest | 0.639 | 0.651 | 0.688 | 0.715 | 1.014 | 0.612 | 0.299 | 0.362 | 0.58 | | .../ModificationWatchpoints | 0.764 | 0.789 | 0.872 | 0.726 | 1.375 | 0.668 | 0.424 | 0.502 | 0.877 | | .../MonitorEventTest | 0.597 | 0.638 | 0.69 | 1.608 | 1.03 | 0.648 | 0.284 | 0.377 | 0.689 | | .../MonitorFrameInfo | 0.622 | 0.652 | 0.731 | 0.596 | 1.014 | 0.592 | 0.299 | 0.456 | 0.612 | | .../NullThreadGroupNameTest | 0.602 | 0.702 | 0.733 | 0.588 | 1.045 | 0.572 | 0.299 | 0.362 | 0.58 | | .../PopAndStepTest | 0.318 | 0.351 | 0.416 | 0.593 | 0.989 | 0.713 | 0.3 | 0.455 | 0.752 | | .../PopAsynchronousTest | 0.718 | 0.869 | 0.8 | 0.654 | 1.063 | 0.619 | 0.519 | 0.581 | 0.737 | | .../ProcessAttachTest| 6.748 | 6.482 | 6.781
Re: RFR(S): JDK-8059037: JdpTest.sh hangs when trying to kill the test VM
On 2014-10-06 13:51, Dmitry Samersoff wrote: On 2014-10-06 15:09, Staffan Larsen wrote: Changes looks good. How much testing have you done with the newly enabled tests? I’m worried that they could be unstable since they have never been run. Only smoke test: Linux jtreg for promoted jdk9 and for the current workspace. Yes, I also suspect that these tests aren't quite stable but I don't see better way than enable it and address failures if they comes. I could help you start a distributed adhoc run if you'd like. Mikael
Re: RFR(S): JDK-8059037: JdpTest.sh hangs when trying to kill the test VM
Sorry for the delay, here are the results: http://vmsqe-app.russia.sun.com/surl/b0 All tests passed on all platforms. That run is not concurrent with any other tests though, so there might still be some issues there, but it's better than nothing. Mikael On 2014-10-06 14:26, Dmitry Samersoff wrote: Mikael, I could help you start a distributed adhoc run if you'd like. Much appreciated! -Dmitry On 2014-10-06 16:00, Mikael Auno wrote: On 2014-10-06 13:51, Dmitry Samersoff wrote: On 2014-10-06 15:09, Staffan Larsen wrote: Changes looks good. How much testing have you done with the newly enabled tests? I’m worried that they could be unstable since they have never been run. Only smoke test: Linux jtreg for promoted jdk9 and for the current workspace. Yes, I also suspect that these tests aren't quite stable but I don't see better way than enable it and address failures if they comes. I could help you start a distributed adhoc run if you'd like. Mikael
Re: RFR 8059034: ProcessTools.startProcess() might leak processes
On 2014-09-25 12:24, Jaroslav Bachorik wrote: On 09/25/2014 12:13 PM, Staffan Larsen wrote: I wonder if the p.waitFor() is needed? What if the process launching expired with a timeout and now we are still waiting for the process to end - wouldn’t that kind of defeat the timeout? In any case, the destroyForcibly() should end the process whether we wait for it or not. It would be wonderful but the javadoc states that the result of destroyForcibly() call depends on the implementation and may actually not force close the process and one should use waitFor() to make sure that the process has in fact died. The javadoc also states though, that the implementation you get when using either ProcessBuilder.start() or Runtime.exec() does in fact forcibly terminate the process. ProcessTools.startProcess() does use ProcessBuilder.start(), so you should be good on that point at least. Mikael I wonder whether JTReg kills the process tree on timeout - in case it does using waitFor() would guarantee that there would be no zombies left. Without using waitFor() and semantics of destroyForcibly() there might be situations when non-functional stuck processes are left behind (not sure how probable, however). -JB- /Staffan On 25 sep 2014, at 11:54, Jaroslav Bachorik jaroslav.bacho...@oracle.com wrote: Please, review the following change to the JDK test library class Issue : https://bugs.openjdk.java.net/browse/JDK-8059034 Webrev: http://cr.openjdk.java.net/~jbachorik/8059034/webrev.00 Currently, the ProcessTools.startProcess() might leave a dangling process behind when a timeout or interrupt happens. The solution is to try and forcibly terminate the forked process when this happens. Thanks, -JB-
Re: RFR(XS): 8058647, 8058649, 8058651, 8058652: ProblemList additions
Thanks for the review Jaroslav. Could you also help me push this (exported changesets attached)? Thanks, Mikael On 2014-09-22 10:30, Jaroslav Bachorik wrote: Reviewed -JB- On 09/18/2014 02:32 PM, Mikael Auno wrote: Hi, Could I please have a review for these small additions to ProblemList.txt in jdk9? They are separate changes (jcheck didn't agree with the guidelines[1] about multiple fixes in a single changeset) with separate bug numbers, but it seems easier to have one review for them all. bug: https://bugs.openjdk.java.net/browse/JDK-8058647 bug: https://bugs.openjdk.java.net/browse/JDK-8058649 bug: https://bugs.openjdk.java.net/browse/JDK-8058651 bug: https://bugs.openjdk.java.net/browse/JDK-8058652 webrev: http://cr.openjdk.java.net/~miauno/8058647_8058649_8058651_8058652/webrev.00/ Thanks, Mikael [1] http://openjdk.java.net/guide/producingChangeset.html#changesetComment # HG changeset patch # User miauno # Date 1411032854 -7200 # Node ID 80c2675985834df64dc385eb914776f089261bf8 # Parent 71c9aa685da57a3a700a067163d9817a35143494 8058647: sun/jvmstat/monitor/MonitoredVm/MonitorVmStartTerminate.java should be quarantined Reviewed-by: jbachorik diff --git a/test/ProblemList.txt b/test/ProblemList.txt --- a/test/ProblemList.txt +++ b/test/ProblemList.txt @@ -298,4 +298,7 @@ # 6456333 sun/tools/jps/TestJpsJarRelative.javageneric-all +# 8057732 +sun/jvmstat/monitor/MonitoredVm/MonitorVmStartTerminate.javageneric-all + # HG changeset patch # User miauno # Date 1411032854 -7200 # Node ID 89b3cbfdf0e8684c6772fb1095b9b07fd1c1bd37 # Parent 80c2675985834df64dc385eb914776f089261bf8 8058649: java/lang/management/ThreadMXBean/FindDeadlocks.java should be quarantined Reviewed-by: jbachorik diff --git a/test/ProblemList.txt b/test/ProblemList.txt --- a/test/ProblemList.txt +++ b/test/ProblemList.txt @@ -130,6 +130,9 @@ # 8056143 java/lang/management/MemoryMXBean/LowMemoryTest.java generic-all +# 8058492 +java/lang/management/ThreadMXBean/FindDeadlocks.java generic-all + # jdk_jmx # HG changeset patch # User miauno # Date 1411032854 -7200 # Node ID 19d857cb4b9d0ce001c732f676e8c98e9e048625 # Parent 89b3cbfdf0e8684c6772fb1095b9b07fd1c1bd37 8058651: com/sun/jdi/RedefinePop.sh should be quarantined Reviewed-by: jbachorik diff --git a/test/ProblemList.txt b/test/ProblemList.txt --- a/test/ProblemList.txt +++ b/test/ProblemList.txt @@ -271,6 +271,9 @@ # 8044419 com/sun/jdi/JdbReadTwiceTest.sh generic-all +# 8058616 +com/sun/jdi/RedefinePop.sh generic-all + # jdk_util # HG changeset patch # User miauno # Date 1411032854 -7200 # Node ID 3f92ca54cd2c87617e345cc7fad26c6a8908e46c # Parent 19d857cb4b9d0ce001c732f676e8c98e9e048625 8058652: java/lang/management/ThreadMXBean/ThreadMXBeanStateTest.java should be quarantined Reviewed-by: jbachorik diff --git a/test/ProblemList.txt b/test/ProblemList.txt --- a/test/ProblemList.txt +++ b/test/ProblemList.txt @@ -133,6 +133,9 @@ # 8058492 java/lang/management/ThreadMXBean/FindDeadlocks.java generic-all +# 8058506 +java/lang/management/ThreadMXBean/ThreadMXBeanStateTest.java generic-all + # jdk_jmx
RFR(XS): 8058647, 8058649, 8058651, 8058652: ProblemList additions
Hi, Could I please have a review for these small additions to ProblemList.txt in jdk9? They are separate changes (jcheck didn't agree with the guidelines[1] about multiple fixes in a single changeset) with separate bug numbers, but it seems easier to have one review for them all. bug: https://bugs.openjdk.java.net/browse/JDK-8058647 bug: https://bugs.openjdk.java.net/browse/JDK-8058649 bug: https://bugs.openjdk.java.net/browse/JDK-8058651 bug: https://bugs.openjdk.java.net/browse/JDK-8058652 webrev: http://cr.openjdk.java.net/~miauno/8058647_8058649_8058651_8058652/webrev.00/ Thanks, Mikael [1] http://openjdk.java.net/guide/producingChangeset.html#changesetComment
RFR(XS): 8057937: javax/management/monitor/GaugeMonitorDeadlockTest.java should be quarantined
Hi, Could I please have a review for this small addition to ProblemList.txt? bug: https://bugs.openjdk.java.net/browse/JDK-8057937 webrev: http://cr.openjdk.java.net/~miauno/8057937/webrev.00/ Thanks, Mikael
Re: RFR(XS): 8057937: javax/management/monitor/GaugeMonitorDeadlockTest.java should be quarantined
Thanks for the reviews! Staffan, could you help me push this (exported changeset attached)? Thanks Mikael On 2014-09-10 11:05, Staffan Larsen wrote: Looks good! Thanks, /Staffan On 10 sep 2014, at 10:44, Mikael Auno mikael.a...@oracle.com wrote: Hi, Could I please have a review for this small addition to ProblemList.txt? bug: https://bugs.openjdk.java.net/browse/JDK-8057937 webrev: http://cr.openjdk.java.net/~miauno/8057937/webrev.00/ Thanks, Mikael # HG changeset patch # User miauno # Date 1410338232 -7200 # Node ID cd04877afd2f085c1254cc31e03254f58987bf98 # Parent e1a4036d8592def43e65dafc9a411d39b176bfa1 8057937: javax/management/monitor/GaugeMonitorDeadlockTest.java should be quarantined Reviewed-by: sla, allwin diff --git a/test/ProblemList.txt b/test/ProblemList.txt --- a/test/ProblemList.txt +++ b/test/ProblemList.txt @@ -139,6 +139,9 @@ com/sun/management/OperatingSystemMXBean/GetSystemCpuLoad.java aix-all javax/management/MBeanServer/OldMBeanServerTest.javaaix-all +# 8050115 +javax/management/monitor/GaugeMonitorDeadlockTest.java generic-all + # jdk_math
Re: RFR(XS): 8044495: Remove test demo/jvmti/mtrace/TraceJFrame.java
Thanks for the reviews.
Staffan, could you help me push this? The exported changeset is attached.
Thanks,
Mikael
On 2014-06-02 23:48, serguei.spit...@oracle.com wrote:
Mikael,
It looks good.
Thank you for doing this!
Thanks,
Serguei
On 6/2/14 9:14 AM, Mikael Auno wrote:
Hi,
Could I please have a review of this small fix, removing the test
TraceJFrame.java.
webrev: http://cr.openjdk.java.net/~miauno/8044495/webrev.00/
bug: https://bugs.openjdk.java.net/browse/JDK-8044495
This test was created a long, long time ago as it reportedly had been
very slow to step into the first JFrame in the early stages of JDK
1.5. Thus, it times how long it takes to create that first JFrame while
tracing, and then seemingly ignores the result and returns PASS. During
its life time, however, we've had lots and lots of failures in this test
due to non-existent or misconfigured X11 displays. As such, we believe
we're better off without it.
Thanks,
Mikael
# HG changeset patch
# User miauno
# Date 1401724748 -7200
# Node ID 9587f9eb4b5aa8a6ff4de34190b0f7f2f9383389
# Parent cb15bc14c26a85874cd6dd97ea17fc76a50e8f18
8044495: Remove test demo/jvmti/mtrace/TraceJFrame.java
Reviewed-by: sla, sspitsyn
diff --git a/test/demo/jvmti/mtrace/JFrameCreateTime.java b/test/demo/jvmti/mtrace/JFrameCreateTime.java
deleted file mode 100644
--- a/test/demo/jvmti/mtrace/JFrameCreateTime.java
+++ /dev/null
@@ -1,59 +0,0 @@
-/*
- * Copyright (c) 2004, 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
- * under the terms of the GNU General Public License version 2 only, as
- * published by the Free Software Foundation.
- *
- * This code is distributed in the hope that it will be useful, but WITHOUT
- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
- * version 2 for more details (a copy is included in the LICENSE file that
- * accompanied this code).
- *
- * You should have received a copy of the GNU General Public License version
- * 2 along with this work; if not, write to the Free Software Foundation,
- * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
- *
- * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
- * or visit www.oracle.com if you need additional information or have any
- * questions.
- */
-
-
-/* JFrameCreateTime:
- *
- * Example swing application that just creates a JFrame object.
- *
- */
-
-/* Early in 1.5 it was reported that doing a step into the first JFrame
- * was very slow (VisualMust debugger people reported this).
- */
-
-import java.awt.GraphicsEnvironment;
-import javax.swing.*;
-
-public class JFrameCreateTime {
-public static void main(String[] args) {
-JFrame f;
-long start, end;
-if (GraphicsEnvironment.isHeadless()) {
-System.out.println(JFrameCreateTime test was skipped due to headless mode);
-} else {
-start = System.currentTimeMillis();
-f = new JFrame(JFrame);
-end = System.currentTimeMillis();
-
-System.out.println(JFrame first creation took + (end - start) + ms);
-
-start = System.currentTimeMillis();
-f = new JFrame(JFrame);
-end = System.currentTimeMillis();
-
-System.out.println(JFrame second creation took + (end - start) + ms);
-System.exit(0);
-}
-}
-}
diff --git a/test/demo/jvmti/mtrace/TraceJFrame.java b/test/demo/jvmti/mtrace/TraceJFrame.java
deleted file mode 100644
--- a/test/demo/jvmti/mtrace/TraceJFrame.java
+++ /dev/null
@@ -1,57 +0,0 @@
-/*
- * Copyright (c) 2004, 2012, 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
- * under the terms of the GNU General Public License version 2 only, as
- * published by the Free Software Foundation.
- *
- * This code is distributed in the hope that it will be useful, but WITHOUT
- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
- * version 2 for more details (a copy is included in the LICENSE file that
- * accompanied this code).
- *
- * You should have received a copy of the GNU General Public License version
- * 2 along with this work; if not, write to the Free Software Foundation,
- * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
- *
- * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
- * or visit www.oracle.com if you need additional information or have any
- * questions.
- */
-
-
-/* @test
- * @bug 500 6299047
- * @summary Test jvmti demo mtrace
- *
- * @compile ../DemoRun.java
- * @compile
Re: RFR(XXS): 8044540: serviceability/sa/jmap-hashcode/Test8028623.java should be quarantined
Thanks for the review. Could you also help me push this to hs-rt? The exported changeset is attached. Mikael On 2014-06-02 19:28, Staffan Larsen wrote: Looks good! Thanks, /Staffan On 2 jun 2014, at 17:29, Mikael Auno mikael.a...@oracle.com wrote: Hi, Could I please have a review of this very small fix. webrev: http://cr.openjdk.java.net/~miauno/8044540/webrev.00/ bug: https://bugs.openjdk.java.net/browse/JDK-8044540 Verified locally. Thanks, Mikael # HG changeset patch # User miauno # Date 1401722475 -7200 # Node ID aa696854a732ed5bd49bd310cfc0d96a86d1d151 # Parent 1ffd0bb18df9e28230c1a5dbb1124ad5b3869863 8044540: serviceability/sa/jmap-hashcode/Test8028623.java should be quarantined Reviewed-by: sla diff --git a/test/serviceability/sa/jmap-hashcode/Test8028623.java b/test/serviceability/sa/jmap-hashcode/Test8028623.java --- a/test/serviceability/sa/jmap-hashcode/Test8028623.java +++ b/test/serviceability/sa/jmap-hashcode/Test8028623.java @@ -26,6 +26,7 @@ * @bug 8028623 * @summary Test hashing of extended characters in Serviceability Agent. * @library /testlibrary + * @ignore 8044416 * @build com.oracle.java.testlibrary.* * @compile -encoding utf8 Test8028623.java * @run main Test8028623
RFR(XXS): 8044540: serviceability/sa/jmap-hashcode/Test8028623.java should be quarantined
Hi, Could I please have a review of this very small fix. webrev: http://cr.openjdk.java.net/~miauno/8044540/webrev.00/ bug: https://bugs.openjdk.java.net/browse/JDK-8044540 Verified locally. Thanks, Mikael
Re: RFR: 8041934 com/sun/jdi/RepStep.java fails on all platforms with assert(_cur_stack_depth == count_frames()) failed: cur_stack_depth out of sync
Note that RepStep was just added to ProblemList due to this issue and will have to be removed from there when the fix is integrated. Mikael On 2014-05-08 08:06, Staffan Larsen wrote: All, This is a fix for an assert in JVMTI that verifies that JVMTI’s internal notion of the number of frames on the stack is correct. When running in -Xcomp mode and enable single-stepping (or method_entry/exit) we will revert all frames on the stack to be run by the interpreter. Only the interpreter can send single-step and method_entry/exit. However, if one of the frames is a native wrapper, that frame will not be reverted (presumably because we don't know how to do that). This will cause us to miss a method_exit event when that native frame is popped. This in turn will mess up the logic in JVMTI that keeps track of the number of frames currently on the stack (see JvmtiThreadState::_cur_stack_depth) and will trigger the assert. The proposed solution is to include a method_exit event in the native wrapper frame if interpreted mode has been enabled. This needs updates to SharedRuntime::generate_native_wrapper() for all platforms. Kudos to Rickard for helping me write the code. webrev: http://cr.openjdk.java.net/~sla/8041934/webrev.00/ bug: https://bugs.openjdk.java.net/browse/JDK-8041934 The fix has been verified by running the failing test in JPRT with -Xcomp. Thanks, /Staffan
Re: RFR 8040748: [TESTBUG] Exclude failing (serviceability) jtreg tests
Done, and thanks for the reviews. Mikael On 2014-05-05 20:24, Staffan Larsen wrote: Mikael, I just the final review for JDK-8031126 so I will be pushing that shortly to jdk9/hs-rt. Would you mind removing that entry from your change (unless you have already pushed it). Thanks, /Staffan On 5 maj 2014, at 16:42, Mikael Auno mikael.a...@oracle.com wrote: Thanks for spotting it. New webrev at http://cr.openjdk.java.net/~miauno/8040748/webrev.03/. Mikael On 2014-05-05 16:18, Staffan Larsen wrote: 8039432 is Resolved and should not be listed. /Staffan On 5 maj 2014, at 15:26, Mikael Auno mikael.a...@oracle.com wrote: I've updated this change to use ProblemList.txt instead of @ignore (since that was only intended for hotspot/test and not jdk/test). I've also updated the data to nightly failures from the last two weeks with issues since then filtered out manually. Hopefully this should address all concerns. New webrev at http://cr.openjdk.java.net/~miauno/8040748/webrev.02/ Thanks, Mikael On 2014-04-25 08:15, Staffan Larsen wrote: For test/com/sun/jdi/JdbReadTwiceTest.sh, you list JDK-8002116, but that bug is resolved so should not be reason to quarantine the test. For test/com/sun/jdi/RepStep.java you list JDK-6471769 JDK-6766320 but both of those are closed so should not be reason to quarantine the test. For test/demo/jvmti/mtrace/TraceJFrame.java you list JDK-8035195 and JDK-8039432, but 8035195 seems to be just an old name for 8039432 (looking it up in JBS takes you to 8039432). So 8035195 should not be listed. For test/sun/tools/jinfo/Basic.sh, there are no open bugs against it (except JDK-6542634 which is a timeout from 2007 and I’m not sure it is valid) so I don’t think it should be quarantined. /Staffan On 24 apr 2014, at 20:01, Mikael Auno mikael.a...@oracle.com wrote: I've now added the keyword quarantine as well as fixed an issue with placement of the @ignore tag (it's not allowed to come before @library). New webrev at http://cr.openjdk.java.net/~miauno/8040748/webrev.01/ Local testing: % jtreg -jdk $JAVA_HOME -k:quarantine -ignore:quiet -verbose:summary /localhome/ws/jdk9-dev/jdk/test/ Test results: no tests selected Report written to /localhome/temp/run/JTreport/html/report.html Results written to /localhome/temp/run/JTwork Thanks, Mikael On 2014-04-22 17:12, Stefan Särne wrote: Hi Mikael, I think we should use a key word to group quarantined tests, which can be used to run the tests as a separate batch. Recommend to add this to all tests: @key quarantine Note that you may have to add the key word as a known key word to the TEST.ROOT file as well. Best regards, Stefan -Original Message- From: Mikael Auno Sent: den 16 april 2014 19:51 To: serviceability-dev@openjdk.java.net Subject: RFR 8040748: [TESTBUG] Exclude failing (serviceability) jtreg tests using @ignore Please, review the following fix adding the @ignore tag to a couple of serviceability tests Issue: https://bugs.openjdk.java.net/browse/JDK-8040748 Webrev: http://cr.openjdk.java.net/~miauno/8040748/webrev.00/ Thanks, Mikael
Re: RFR 8040748: [TESTBUG] Exclude failing (serviceability) jtreg tests
I've updated this change to use ProblemList.txt instead of @ignore (since that was only intended for hotspot/test and not jdk/test). I've also updated the data to nightly failures from the last two weeks with issues since then filtered out manually. Hopefully this should address all concerns. New webrev at http://cr.openjdk.java.net/~miauno/8040748/webrev.02/ Thanks, Mikael On 2014-04-25 08:15, Staffan Larsen wrote: For test/com/sun/jdi/JdbReadTwiceTest.sh, you list JDK-8002116, but that bug is resolved so should not be reason to quarantine the test. For test/com/sun/jdi/RepStep.java you list JDK-6471769 JDK-6766320 but both of those are closed so should not be reason to quarantine the test. For test/demo/jvmti/mtrace/TraceJFrame.java you list JDK-8035195 and JDK-8039432, but 8035195 seems to be just an old name for 8039432 (looking it up in JBS takes you to 8039432). So 8035195 should not be listed. For test/sun/tools/jinfo/Basic.sh, there are no open bugs against it (except JDK-6542634 which is a timeout from 2007 and I’m not sure it is valid) so I don’t think it should be quarantined. /Staffan On 24 apr 2014, at 20:01, Mikael Auno mikael.a...@oracle.com wrote: I've now added the keyword quarantine as well as fixed an issue with placement of the @ignore tag (it's not allowed to come before @library). New webrev at http://cr.openjdk.java.net/~miauno/8040748/webrev.01/ Local testing: % jtreg -jdk $JAVA_HOME -k:quarantine -ignore:quiet -verbose:summary /localhome/ws/jdk9-dev/jdk/test/ Test results: no tests selected Report written to /localhome/temp/run/JTreport/html/report.html Results written to /localhome/temp/run/JTwork Thanks, Mikael On 2014-04-22 17:12, Stefan Särne wrote: Hi Mikael, I think we should use a key word to group quarantined tests, which can be used to run the tests as a separate batch. Recommend to add this to all tests: @key quarantine Note that you may have to add the key word as a known key word to the TEST.ROOT file as well. Best regards, Stefan -Original Message- From: Mikael Auno Sent: den 16 april 2014 19:51 To: serviceability-dev@openjdk.java.net Subject: RFR 8040748: [TESTBUG] Exclude failing (serviceability) jtreg tests using @ignore Please, review the following fix adding the @ignore tag to a couple of serviceability tests Issue: https://bugs.openjdk.java.net/browse/JDK-8040748 Webrev: http://cr.openjdk.java.net/~miauno/8040748/webrev.00/ Thanks, Mikael
Re: RFR 8040748: [TESTBUG] Exclude failing (serviceability) jtreg tests
Thanks for spotting it. New webrev at http://cr.openjdk.java.net/~miauno/8040748/webrev.03/. Mikael On 2014-05-05 16:18, Staffan Larsen wrote: 8039432 is Resolved and should not be listed. /Staffan On 5 maj 2014, at 15:26, Mikael Auno mikael.a...@oracle.com wrote: I've updated this change to use ProblemList.txt instead of @ignore (since that was only intended for hotspot/test and not jdk/test). I've also updated the data to nightly failures from the last two weeks with issues since then filtered out manually. Hopefully this should address all concerns. New webrev at http://cr.openjdk.java.net/~miauno/8040748/webrev.02/ Thanks, Mikael On 2014-04-25 08:15, Staffan Larsen wrote: For test/com/sun/jdi/JdbReadTwiceTest.sh, you list JDK-8002116, but that bug is resolved so should not be reason to quarantine the test. For test/com/sun/jdi/RepStep.java you list JDK-6471769 JDK-6766320 but both of those are closed so should not be reason to quarantine the test. For test/demo/jvmti/mtrace/TraceJFrame.java you list JDK-8035195 and JDK-8039432, but 8035195 seems to be just an old name for 8039432 (looking it up in JBS takes you to 8039432). So 8035195 should not be listed. For test/sun/tools/jinfo/Basic.sh, there are no open bugs against it (except JDK-6542634 which is a timeout from 2007 and I’m not sure it is valid) so I don’t think it should be quarantined. /Staffan On 24 apr 2014, at 20:01, Mikael Auno mikael.a...@oracle.com wrote: I've now added the keyword quarantine as well as fixed an issue with placement of the @ignore tag (it's not allowed to come before @library). New webrev at http://cr.openjdk.java.net/~miauno/8040748/webrev.01/ Local testing: % jtreg -jdk $JAVA_HOME -k:quarantine -ignore:quiet -verbose:summary /localhome/ws/jdk9-dev/jdk/test/ Test results: no tests selected Report written to /localhome/temp/run/JTreport/html/report.html Results written to /localhome/temp/run/JTwork Thanks, Mikael On 2014-04-22 17:12, Stefan Särne wrote: Hi Mikael, I think we should use a key word to group quarantined tests, which can be used to run the tests as a separate batch. Recommend to add this to all tests: @key quarantine Note that you may have to add the key word as a known key word to the TEST.ROOT file as well. Best regards, Stefan -Original Message- From: Mikael Auno Sent: den 16 april 2014 19:51 To: serviceability-dev@openjdk.java.net Subject: RFR 8040748: [TESTBUG] Exclude failing (serviceability) jtreg tests using @ignore Please, review the following fix adding the @ignore tag to a couple of serviceability tests Issue: https://bugs.openjdk.java.net/browse/JDK-8040748 Webrev: http://cr.openjdk.java.net/~miauno/8040748/webrev.00/ Thanks, Mikael
Re: RFR 8040748: [TESTBUG] Exclude failing (serviceability) jtreg tests using @ignore
I've now added the keyword quarantine as well as fixed an issue with placement of the @ignore tag (it's not allowed to come before @library). New webrev at http://cr.openjdk.java.net/~miauno/8040748/webrev.01/ Local testing: % jtreg -jdk $JAVA_HOME -k:quarantine -ignore:quiet -verbose:summary /localhome/ws/jdk9-dev/jdk/test/ Test results: no tests selected Report written to /localhome/temp/run/JTreport/html/report.html Results written to /localhome/temp/run/JTwork Thanks, Mikael On 2014-04-22 17:12, Stefan Särne wrote: Hi Mikael, I think we should use a key word to group quarantined tests, which can be used to run the tests as a separate batch. Recommend to add this to all tests: @key quarantine Note that you may have to add the key word as a known key word to the TEST.ROOT file as well. Best regards, Stefan -Original Message- From: Mikael Auno Sent: den 16 april 2014 19:51 To: serviceability-dev@openjdk.java.net Subject: RFR 8040748: [TESTBUG] Exclude failing (serviceability) jtreg tests using @ignore Please, review the following fix adding the @ignore tag to a couple of serviceability tests Issue: https://bugs.openjdk.java.net/browse/JDK-8040748 Webrev: http://cr.openjdk.java.net/~miauno/8040748/webrev.00/ Thanks, Mikael
RFR 8040748: [TESTBUG] Exclude failing (serviceability) jtreg tests using @ignore
Please, review the following fix adding the @ignore tag to a couple of serviceability tests Issue: https://bugs.openjdk.java.net/browse/JDK-8040748 Webrev: http://cr.openjdk.java.net/~miauno/8040748/webrev.00/ Thanks, Mikael
Re: RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails intermittently
How about defining the class that you want to attach to as a static inner class to the actual test? That would give you only one file, but with two classes, each with its own main method and clear correlation between them. Mikael On 2013-11-20 15:41, Mattias Tobiasson wrote: Hi, Each test requires 2 files, the actual test code and a helper file. The helper file will launch a separate java process (called Application), and then start the actual test. The actual test will then attach to the Application. For example: PermissionTests.sh: Helper file that will launch Application instance and then start PermissionTest.java PermissionTest.java: The actual test code that attaches to the Application. It is the PermissionTests.sh that is started by jtreg (contains the @test-tag). When I port PermissionTest.sh to java I would get 2 files called PermissionTest.java, so some name change is required. I could have kept the old PermissionTest.java unchanged, but then I would need another name for the prevoius PermissionTest.sh. And I wanted a clean Test name for the file containing the @test-tag. I used these names: TestPermission.java: Helper file. TestPermissionImpl.java: Actual test code. I am still new to adding tests for open jdk. I am happy to change the names if they do not follow the naming convention. Mattias - Original Message - From: alan.bate...@oracle.com To: mattias.tobias...@oracle.com Cc: serviceability-dev@openjdk.java.net Sent: Wednesday, November 20, 2013 2:50:52 PM GMT +01:00 Amsterdam / Berlin / Bern / Rome / Stockholm / Vienna Subject: Re: RFR (S): 6461635: [TESTBUG] BasicTests.sh test fails intermittently Out of curiosity, what is the reason for the rename? I ask because we use Basic and similar names in many areas. Also anything in the test tree should be a test. -Alan. On 20/11/2013 13:47, Mattias Tobiasson wrote: Hi, Could you please review this fix. Summary of changes: 1. The real test bug fix is to add flag -Xshare:off when starting the Application instance. Without that flag, the test for ClassFileTransformer in RedefineAgent.java fails intermittently. The flag is added in function startApplication() in RunnerUtil.java. 2. Ported the following bash scripts to java: BasicTests.sh - TestBasic.java PermissionTests.sh - TestPermission.java ProviderTests.sh - TestProvider.java 3. Renamed the java test code to avoid name clash with new java classes ported from bash script: BasicsTest.java - TestBasicImpl.java PermissionTest.java - TestPermissionImpl.java ProviderTest.java - TestProviderImpl.java 4. Added some utility functions to jdk/testlibrary. 5. Moved exit code check from the common utility function in ProcessThread.java to the test JstatdTest.java. The check is moved to the test because other tests in the future may have other expected exit codes. Thanks, Mattias Webrev: http://cr.openjdk.java.net/~miauno/6461635/webrev.00/ Bug: https://bugs.openjdk.java.net/browse/JDK-6461635
Re: RFR 8014506: Test for Jdp (Java Discovery Protocol) feature
The reduction in testsuites in JdpTest.sh is due to those testsuites removed being ported to Java with this change. The last testsuite in JdpTest.sh however, has not been ported yet, therefore JdpTest.sh can't be removed completely. Mikael On 2013-11-11 14:19, Staffan Larsen wrote: Why are there changes to the existing tests? Most look like whitespace changes, but JdpTest.sh changes the “testsuites” that are being run. /Staffan On 08 Nov 2013, at 18:45, Mikael Auno mikael.a...@oracle.com mailto:mikael.a...@oracle.com wrote: There was some unintended whitespace changes in webrev.05 that Alex had missed. I removed them and uploaded webrev.06 for him. http://cr.openjdk.java.net/~miauno/8014506/webrev.06/ Mikael On 2013-11-08 17:02, Alex Schenkman wrote: Hi list, Latest version is up for review here [1]. It fixes Staffan's latest comments (see below), as well as some indentation and tab/spaces nits. Thanks! [1]http://cr.openjdk.java.net/~miauno/8014506/ http://cr.openjdk.java.net/%7Emiauno/8014506/ On 2013-10-30 16:03, Staffan Larsen wrote: Alex, This looks much better! Thanks for spending the time. README: nit: contribuited - contributed JdpTestUtilTest.java: Can you add a comment saying this is a test for the tests - just like PacketTest.java and PortAlreadyInUseTest.java? PortFinder.java: Please remove this and use Utils.getFreePort() since ykantser's fix is now pushed. Thanks, /Staffan On 29 okt 2013, at 15:53, Alex Schenkman alex.schenk...@oracle.com mailto:alex.schenk...@oracle.com mailto:alex.schenk...@oracle.com wrote: Hi Staffan, There is a new webrev here [1], addressing your comments. The Jdp specs have changed a bit, adding three optional fields to the Jdp packet [2]. The JEP-168 [3] is not updated yet, but Dmitry will do it soon. This enhancement request is tracking the changes needed for SQE [4]. See inlined answers for details on your previos comments, please. Thanks! [1] http://cr.openjdk.java.net/~dsamersoff/sponsorship/aschenkman/8014506/webrev.04/ http://cr.openjdk.java.net/%7Edsamersoff/sponsorship/aschenkman/8014506/webrev.04/ [2] https://bugs.openjdk.java.net/browse/JDK-8004213 [3] http://openjdk.java.net/jeps/168 [4] https://bugs.openjdk.java.net/browse/JDK-8027436 On 2013-10-24 10:54, Staffan Larsen wrote: Alex, test/sun/management/jdp/README Thanks for providing a README! References to JDK-7169888 seem incorrect to me. L8: written by appears twice. L11: defautl - default Fixed. test/lib/testlibrary/jdk/testlibrary/PortFinder.java This should be coordinated with the review for 809 which contains the same code in a different testlibrary class: http://cr.openjdk.java.net/~ykantser/809/webrev.01/test/lib/testlibrary/jdk/testlibrary/Utils.java.html http://cr.openjdk.java.net/%7Eykantser/809/webrev.01/test/lib/testlibrary/jdk/testlibrary/Utils.java.html I made a private copy of this function, and have marked it as deprecated. As soon as ykanster's code is merged I can delete my private function and use the library. The reason for this is to get this patch pushed without further delay. test/sun/management/jdp/7169888/JdpClient.java test/sun/management/jdp/7169888/JdpDoSomething.java test/sun/management/jdp/7169888/JdpTest.sh test/sun/management/jdp/7169888/JdpUnitTest.java These were moved to a subdirectory, but these tests have nothing to do with bug 7169888, so why that name? In any case we should avoid naming test directories after bug ids, and instead use a descriptive name. This bug number was wrong. These tests were now moved back to the jdp folder. I have expanded the README file with some details about these tests. Dmitry's shell-based are as follows: test_01 - basic test, check if JDP packet assembler and other parts of JDP is not broken test_02 - test if JDP starts with custom parameters. test_03 - test if jcmd is able to start jdp with custom parameters (disabled) test_04 - test if JDP starts with default parameters test_05 - test if jcmd is able to start jdp with default parameters (disabled) test_03 and test_05 are diabled becuase there is a mismatch between hotsport and jdk repos, according to Dmitry. These cases (jcmd) are not covered by this patch, and are part of the test enhacements [4]. test_01 does partly overlap with the Java-based tests, but we should keep it until we implement the latest enhacements [4]. test/sun/management/jdp/ClientConnection.java No comments. test/sun/management/jdp/DefaultLauncher.java Can @library ../../../lib/testlibrary be replaced by @library /lib/testlibrary ? Fixed. test/sun/management/jdp/OffLauncher.java Can @library ../../../lib/testlibrary be replaced by @library /lib/testlibrary ? Fixed. test/sun/management/jdp/SpecificJdpAddressLauncher.java Can @library ../../../lib/testlibrary be replaced by @library /lib/testlibrary ? Fixed. test/sun/management/jdp/DynamicLauncher.java No comments. Fixed. test/sun/management
Re: RFR 8014506: Test for Jdp (Java Discovery Protocol) feature
There was some unintended whitespace changes in webrev.05 that Alex had missed. I removed them and uploaded webrev.06 for him. http://cr.openjdk.java.net/~miauno/8014506/webrev.06/ Mikael On 2013-11-08 17:02, Alex Schenkman wrote: Hi list, Latest version is up for review here [1]. It fixes Staffan's latest comments (see below), as well as some indentation and tab/spaces nits. Thanks! [1] http://cr.openjdk.java.net/~miauno/8014506/ http://cr.openjdk.java.net/%7Emiauno/8014506/ On 2013-10-30 16:03, Staffan Larsen wrote: Alex, This looks much better! Thanks for spending the time. README: nit: contribuited - contributed JdpTestUtilTest.java: Can you add a comment saying this is a test for the tests - just like PacketTest.java and PortAlreadyInUseTest.java? PortFinder.java: Please remove this and use Utils.getFreePort() since ykantser's fix is now pushed. Thanks, /Staffan On 29 okt 2013, at 15:53, Alex Schenkman alex.schenk...@oracle.com mailto:alex.schenk...@oracle.com wrote: Hi Staffan, There is a new webrev here [1], addressing your comments. The Jdp specs have changed a bit, adding three optional fields to the Jdp packet [2]. The JEP-168 [3] is not updated yet, but Dmitry will do it soon. This enhancement request is tracking the changes needed for SQE [4]. See inlined answers for details on your previos comments, please. Thanks! [1] http://cr.openjdk.java.net/~dsamersoff/sponsorship/aschenkman/8014506/webrev.04/ http://cr.openjdk.java.net/%7Edsamersoff/sponsorship/aschenkman/8014506/webrev.04/ [2] https://bugs.openjdk.java.net/browse/JDK-8004213 [3] http://openjdk.java.net/jeps/168 [4] https://bugs.openjdk.java.net/browse/JDK-8027436 On 2013-10-24 10:54, Staffan Larsen wrote: Alex, test/sun/management/jdp/README Thanks for providing a README! References to JDK-7169888 seem incorrect to me. L8: written by appears twice. L11: defautl - default Fixed. test/lib/testlibrary/jdk/testlibrary/PortFinder.java This should be coordinated with the review for 809 which contains the same code in a different testlibrary class: http://cr.openjdk.java.net/~ykantser/809/webrev.01/test/lib/testlibrary/jdk/testlibrary/Utils.java.html http://cr.openjdk.java.net/%7Eykantser/809/webrev.01/test/lib/testlibrary/jdk/testlibrary/Utils.java.html I made a private copy of this function, and have marked it as deprecated. As soon as ykanster's code is merged I can delete my private function and use the library. The reason for this is to get this patch pushed without further delay. test/sun/management/jdp/7169888/JdpClient.java test/sun/management/jdp/7169888/JdpDoSomething.java test/sun/management/jdp/7169888/JdpTest.sh test/sun/management/jdp/7169888/JdpUnitTest.java These were moved to a subdirectory, but these tests have nothing to do with bug 7169888, so why that name? In any case we should avoid naming test directories after bug ids, and instead use a descriptive name. This bug number was wrong. These tests were now moved back to the jdp folder. I have expanded the README file with some details about these tests. Dmitry's shell-based are as follows: test_01 - basic test, check if JDP packet assembler and other parts of JDP is not broken test_02 - test if JDP starts with custom parameters. test_03 - test if jcmd is able to start jdp with custom parameters (disabled) test_04 - test if JDP starts with default parameters test_05 - test if jcmd is able to start jdp with default parameters (disabled) test_03 and test_05 are diabled becuase there is a mismatch between hotsport and jdk repos, according to Dmitry. These cases (jcmd) are not covered by this patch, and are part of the test enhacements [4]. test_01 does partly overlap with the Java-based tests, but we should keep it until we implement the latest enhacements [4]. test/sun/management/jdp/ClientConnection.java No comments. test/sun/management/jdp/DefaultLauncher.java Can @library ../../../lib/testlibrary be replaced by @library /lib/testlibrary ? Fixed. test/sun/management/jdp/OffLauncher.java Can @library ../../../lib/testlibrary be replaced by @library /lib/testlibrary ? Fixed. test/sun/management/jdp/SpecificJdpAddressLauncher.java Can @library ../../../lib/testlibrary be replaced by @library /lib/testlibrary ? Fixed. test/sun/management/jdp/DynamicLauncher.java No comments. Fixed. test/sun/management/jdp/JdpOffTest.java For the future: it is customary that the file containing the jtreg directives be named xxxTest.java, and supporting files not ending in Test. I have renamed the tests following your recomendation. Jtreg tests have now the test suffix. test/sun/management/jdp/JdpOnTest.java For the future: it is customary that the file containing the jtreg directives be named xxxTest.java, and supporting files not ending in Test. Same as above. test/sun/management/jdp/JdpTest.java No comments.
Re: RFR(S): 7145419 com/sun/jdi/JdbMethodExitTest.sh fails when a background thread is generating events.
On 2013-10-29 15:41, Staffan Larsen wrote: This test fails if there are background threads that run while the test is running. I've modified the test to use the trace commands in jdb with the extra thread parameter. I have assumed that the main thread has thread id 1. trace with thread id behaves a little bit different so a couple of extra cont were needed. webrev: http://cr.openjdk.java.net/~sla/7145419/webrev.00/ Would it be possible to set a breakpoint in main (or some other known location) to determine the thread id (as we do in some of the JDI tests) to make sure we have the right one before continuing with the rest of the test? Mikael
Re: RFR: 8009681: TEST_BUG: MethodExitReturnValuesTest.java fails with when there are unexpected background threads
Thanks for the reviews. Mikael On 2013-10-18 13:09, Peter Allwin wrote: +1 Thanks, /peter On Oct 18, 2013, at 12:56 PM, Staffan Larsen staffan.lar...@oracle.com wrote: Looks good! Thanks, /Staffan On 16 okt 2013, at 14:04, Mikael Auno mikael.a...@oracle.com wrote: This bug got a bit lost from my radar after vacation, but I've picked it again now. I've moved Arrays.asList() as suggested. In further testing of the fix though, I found that the include list is not enough, as one of the expected method exit events is from String.intern(), which might also be called from background threads. To counter this, I added a thread filter to the events. This also has the added benefit of speeding up the test significantly (from 90 seconds to about 5 seconds) in the cases where there are background threads interfering. Also added to this webrev is a fix for MethodEntryExitEvents.java which had exactly the same problem and a similar test design as MethodExitReturnValuesTest.java. The updated webrev is at http://cr.openjdk.java.net/~allwin/auno/8009681/webrev.00/. Thanks, Mikael On 2013-05-28 08:46, Staffan Larsen wrote: Looks good. You could optimize it a bit by not doing the Arrays.asList() on every methodExit event. /Staffan On 17 apr 2013, at 15:03, Mikael Auno mikael.a...@oracle.com wrote: Hi, I'd like some reviews on http://cr.openjdk.java.net/~nloodin/8009681/webrev.01/ for JDK-8009681 (http://bugs.sun.com/view_bug.do?bug_id=8009681). The issue here is that when MethodExitReturnValuesTest hooks into MethodExit events through JDI it uses an exclude list to filter out classes from which it is not interested in these events. This is bound to break over and over again as new features are added to the JDK. I've changed the test to use an include list instead, containing only the handful of classes the test is actually interested in. Thanks, Mikael
Re: RFR: 8009681: TEST_BUG: MethodExitReturnValuesTest.java fails with when there are unexpected background threads
This bug got a bit lost from my radar after vacation, but I've picked it again now. I've moved Arrays.asList() as suggested. In further testing of the fix though, I found that the include list is not enough, as one of the expected method exit events is from String.intern(), which might also be called from background threads. To counter this, I added a thread filter to the events. This also has the added benefit of speeding up the test significantly (from 90 seconds to about 5 seconds) in the cases where there are background threads interfering. Also added to this webrev is a fix for MethodEntryExitEvents.java which had exactly the same problem and a similar test design as MethodExitReturnValuesTest.java. The updated webrev is at http://cr.openjdk.java.net/~allwin/auno/8009681/webrev.00/. Thanks, Mikael On 2013-05-28 08:46, Staffan Larsen wrote: Looks good. You could optimize it a bit by not doing the Arrays.asList() on every methodExit event. /Staffan On 17 apr 2013, at 15:03, Mikael Auno mikael.a...@oracle.com wrote: Hi, I'd like some reviews on http://cr.openjdk.java.net/~nloodin/8009681/webrev.01/ for JDK-8009681 (http://bugs.sun.com/view_bug.do?bug_id=8009681). The issue here is that when MethodExitReturnValuesTest hooks into MethodExit events through JDI it uses an exclude list to filter out classes from which it is not interested in these events. This is bound to break over and over again as new features are added to the JDK. I've changed the test to use an include list instead, containing only the handful of classes the test is actually interested in. Thanks, Mikael
Re: RFR 6470730: Disconnect button leads to wrong popup message
On 2013-05-24 16:13, Nils Loodin wrote: Simple fix not to add a confusing dialog. Bug: https://jbs.oracle.com/bugs/browse/JDK-6470730 Webrev: http://cr.openjdk.java.net/~nloodin/6470730/webrev.00/ Fix looks simple and good to me. However, I don't have my OpenJDK username yet, so this might not carry that much weight. Mikael
RFR: 8009681: TEST_BUG: MethodExitReturnValuesTest.java fails with when there are unexpected background threads
Hi, I'd like some reviews on http://cr.openjdk.java.net/~nloodin/8009681/webrev.01/ for JDK-8009681 (http://bugs.sun.com/view_bug.do?bug_id=8009681). The issue here is that when MethodExitReturnValuesTest hooks into MethodExit events through JDI it uses an exclude list to filter out classes from which it is not interested in these events. This is bound to break over and over again as new features are added to the JDK. I've changed the test to use an include list instead, containing only the handful of classes the test is actually interested in. Thanks, Mikael
