Re: RFR(S): 8234277:ClhsdbLauncher should enable verbose exceptions and do a better job of detecting SA failures
Hi Chris, The change looks good to me. Best regards, Daniil On 12/4/19, 5:39 PM, "serviceability-dev on behalf of Chris Plummer" wrote: Can I get one more review please? thanks, Chris On 12/3/19 1:10 PM, serguei.spit...@oracle.com wrote: > Hi Chris, > > It looks good. > > Thanks, > Serguei > > On 12/3/19 12:45 PM, Chris Plummer wrote: >> Hello, >> >> Please review the following: >> >> https://bugs.openjdk.java.net/browse/JDK-8234277 >> http://cr.openjdk.java.net/~cjplummer/8234277/webrev.00/ >> >> No longer redirect stderr for the jhsdb/clhsdb process. It results in >> not seeing attach failures in the output, so OutputAnalyer can't >> check for them. >> >> Execute "verbose true" as the first clhsdb command after launching. >> This will result in verboseExceptions being true in >> CommandProcessor.java, so full exception traces will appear in the >> output. This will make debugging future SA test failures a lot easier. >> >> Add an extra check for any DebuggerException. This is mainly for >> detecting that the attached failed. This previously was going >> un-noticed, and instead the test would later fail because it noticed >> some other issue, like missing output, which isn't very informative. >> >> Add checks for other unexpected SA exceptions that are caught and >> printed by CommandProcessor. These will always have an "Error: " >> prefix, making them easy to detect. >> >> Problem list ClhsdbScanOops.java. With the new error checking, it >> will now always fail on windows due to JDK-8230731 and on macos and >> linux due to JDK-8235220. These failures are not "new" per se, but >> are just now being properly detected. >> >> thanks, >> >> Chris >
Re: RFR(S): 8234277:ClhsdbLauncher should enable verbose exceptions and do a better job of detecting SA failures
Thanks! On 12/4/19 6:15 PM, Yasumasa Suenaga wrote: Looks good. Yasumasa On 2019/12/05 10:39, Chris Plummer wrote: Can I get one more review please? thanks, Chris On 12/3/19 1:10 PM, serguei.spit...@oracle.com wrote: Hi Chris, It looks good. Thanks, Serguei On 12/3/19 12:45 PM, Chris Plummer wrote: Hello, Please review the following: https://bugs.openjdk.java.net/browse/JDK-8234277 http://cr.openjdk.java.net/~cjplummer/8234277/webrev.00/ No longer redirect stderr for the jhsdb/clhsdb process. It results in not seeing attach failures in the output, so OutputAnalyer can't check for them. Execute "verbose true" as the first clhsdb command after launching. This will result in verboseExceptions being true in CommandProcessor.java, so full exception traces will appear in the output. This will make debugging future SA test failures a lot easier. Add an extra check for any DebuggerException. This is mainly for detecting that the attached failed. This previously was going un-noticed, and instead the test would later fail because it noticed some other issue, like missing output, which isn't very informative. Add checks for other unexpected SA exceptions that are caught and printed by CommandProcessor. These will always have an "Error: " prefix, making them easy to detect. Problem list ClhsdbScanOops.java. With the new error checking, it will now always fail on windows due to JDK-8230731 and on macos and linux due to JDK-8235220. These failures are not "new" per se, but are just now being properly detected. thanks, Chris
Re: RFR(S): 8234277:ClhsdbLauncher should enable verbose exceptions and do a better job of detecting SA failures
Looks good. Yasumasa On 2019/12/05 10:39, Chris Plummer wrote: Can I get one more review please? thanks, Chris On 12/3/19 1:10 PM, serguei.spit...@oracle.com wrote: Hi Chris, It looks good. Thanks, Serguei On 12/3/19 12:45 PM, Chris Plummer wrote: Hello, Please review the following: https://bugs.openjdk.java.net/browse/JDK-8234277 http://cr.openjdk.java.net/~cjplummer/8234277/webrev.00/ No longer redirect stderr for the jhsdb/clhsdb process. It results in not seeing attach failures in the output, so OutputAnalyer can't check for them. Execute "verbose true" as the first clhsdb command after launching. This will result in verboseExceptions being true in CommandProcessor.java, so full exception traces will appear in the output. This will make debugging future SA test failures a lot easier. Add an extra check for any DebuggerException. This is mainly for detecting that the attached failed. This previously was going un-noticed, and instead the test would later fail because it noticed some other issue, like missing output, which isn't very informative. Add checks for other unexpected SA exceptions that are caught and printed by CommandProcessor. These will always have an "Error: " prefix, making them easy to detect. Problem list ClhsdbScanOops.java. With the new error checking, it will now always fail on windows due to JDK-8230731 and on macos and linux due to JDK-8235220. These failures are not "new" per se, but are just now being properly detected. thanks, Chris
Re: RFR(S): 8234277:ClhsdbLauncher should enable verbose exceptions and do a better job of detecting SA failures
Can I get one more review please? thanks, Chris On 12/3/19 1:10 PM, serguei.spit...@oracle.com wrote: Hi Chris, It looks good. Thanks, Serguei On 12/3/19 12:45 PM, Chris Plummer wrote: Hello, Please review the following: https://bugs.openjdk.java.net/browse/JDK-8234277 http://cr.openjdk.java.net/~cjplummer/8234277/webrev.00/ No longer redirect stderr for the jhsdb/clhsdb process. It results in not seeing attach failures in the output, so OutputAnalyer can't check for them. Execute "verbose true" as the first clhsdb command after launching. This will result in verboseExceptions being true in CommandProcessor.java, so full exception traces will appear in the output. This will make debugging future SA test failures a lot easier. Add an extra check for any DebuggerException. This is mainly for detecting that the attached failed. This previously was going un-noticed, and instead the test would later fail because it noticed some other issue, like missing output, which isn't very informative. Add checks for other unexpected SA exceptions that are caught and printed by CommandProcessor. These will always have an "Error: " prefix, making them easy to detect. Problem list ClhsdbScanOops.java. With the new error checking, it will now always fail on windows due to JDK-8230731 and on macos and linux due to JDK-8235220. These failures are not "new" per se, but are just now being properly detected. thanks, Chris
Re: RFR(S): 8234277:ClhsdbLauncher should enable verbose exceptions and do a better job of detecting SA failures
Thanks Serguei! Chris On 12/3/19 1:10 PM, serguei.spit...@oracle.com wrote: Hi Chris, It looks good. Thanks, Serguei On 12/3/19 12:45 PM, Chris Plummer wrote: Hello, Please review the following: https://bugs.openjdk.java.net/browse/JDK-8234277 http://cr.openjdk.java.net/~cjplummer/8234277/webrev.00/ No longer redirect stderr for the jhsdb/clhsdb process. It results in not seeing attach failures in the output, so OutputAnalyer can't check for them. Execute "verbose true" as the first clhsdb command after launching. This will result in verboseExceptions being true in CommandProcessor.java, so full exception traces will appear in the output. This will make debugging future SA test failures a lot easier. Add an extra check for any DebuggerException. This is mainly for detecting that the attached failed. This previously was going un-noticed, and instead the test would later fail because it noticed some other issue, like missing output, which isn't very informative. Add checks for other unexpected SA exceptions that are caught and printed by CommandProcessor. These will always have an "Error: " prefix, making them easy to detect. Problem list ClhsdbScanOops.java. With the new error checking, it will now always fail on windows due to JDK-8230731 and on macos and linux due to JDK-8235220. These failures are not "new" per se, but are just now being properly detected. thanks, Chris
Re: RFR(S): 8234277:ClhsdbLauncher should enable verbose exceptions and do a better job of detecting SA failures
Hi Chris, It looks good. I'm in favor to always run tests in verbose mode. It is not a good idea in general to optimize on it. Thanks, Serguei On 12/3/19 12:45 PM, Chris Plummer wrote: Hello, Please review the following: https://bugs.openjdk.java.net/browse/JDK-8234277 http://cr.openjdk.java.net/~cjplummer/8234277/webrev.00/ No longer redirect stderr for the jhsdb/clhsdb process. It results in not seeing attach failures in the output, so OutputAnalyer can't check for them. Execute "verbose true" as the first clhsdb command after launching. This will result in verboseExceptions being true in CommandProcessor.java, so full exception traces will appear in the output. This will make debugging future SA test failures a lot easier. Add an extra check for any DebuggerException. This is mainly for detecting that the attached failed. This previously was going un-noticed, and instead the test would later fail because it noticed some other issue, like missing output, which isn't very informative. Add checks for other unexpected SA exceptions that are caught and printed by CommandProcessor. These will always have an "Error: " prefix, making them easy to detect. Problem list ClhsdbScanOops.java. With the new error checking, it will now always fail on windows due to JDK-8230731 and on macos and linux due to JDK-8235220. These failures are not "new" per se, but are just now being properly detected. thanks, Chris
Re: RFR(S): 8234277:ClhsdbLauncher should enable verbose exceptions and do a better job of detecting SA failures
Hi Chris, It looks good. Thanks, Serguei On 12/3/19 12:45 PM, Chris Plummer wrote: Hello, Please review the following: https://bugs.openjdk.java.net/browse/JDK-8234277 http://cr.openjdk.java.net/~cjplummer/8234277/webrev.00/ No longer redirect stderr for the jhsdb/clhsdb process. It results in not seeing attach failures in the output, so OutputAnalyer can't check for them. Execute "verbose true" as the first clhsdb command after launching. This will result in verboseExceptions being true in CommandProcessor.java, so full exception traces will appear in the output. This will make debugging future SA test failures a lot easier. Add an extra check for any DebuggerException. This is mainly for detecting that the attached failed. This previously was going un-noticed, and instead the test would later fail because it noticed some other issue, like missing output, which isn't very informative. Add checks for other unexpected SA exceptions that are caught and printed by CommandProcessor. These will always have an "Error: " prefix, making them easy to detect. Problem list ClhsdbScanOops.java. With the new error checking, it will now always fail on windows due to JDK-8230731 and on macos and linux due to JDK-8235220. These failures are not "new" per se, but are just now being properly detected. thanks, Chris
RFR(S): 8234277:ClhsdbLauncher should enable verbose exceptions and do a better job of detecting SA failures
Hello, Please review the following: https://bugs.openjdk.java.net/browse/JDK-8234277 http://cr.openjdk.java.net/~cjplummer/8234277/webrev.00/ No longer redirect stderr for the jhsdb/clhsdb process. It results in not seeing attach failures in the output, so OutputAnalyer can't check for them. Execute "verbose true" as the first clhsdb command after launching. This will result in verboseExceptions being true in CommandProcessor.java, so full exception traces will appear in the output. This will make debugging future SA test failures a lot easier. Add an extra check for any DebuggerException. This is mainly for detecting that the attached failed. This previously was going un-noticed, and instead the test would later fail because it noticed some other issue, like missing output, which isn't very informative. Add checks for other unexpected SA exceptions that are caught and printed by CommandProcessor. These will always have an "Error: " prefix, making them easy to detect. Problem list ClhsdbScanOops.java. With the new error checking, it will now always fail on windows due to JDK-8230731 and on macos and linux due to JDK-8235220. These failures are not "new" per se, but are just now being properly detected. thanks, Chris
