RFR: JDK-8215568: Refactor SA clhsdb tests to use ClhsdbLauncher

[Jini George]

Thank you very much for the review, JC. My comments inline.


> I saw this potential issue with one of the test conversions:
> http://cr.openjdk.java.net/~jgeorge/8215568/webrev.00/test/hotspot/jtreg/serviceability/sa/TestPrintMdo.java.udiff.html
>     - It seems like there is a missing "unexpected" strings check here no?
>        - The original test was doing
> -
> -                if (line.contains("missing reason for ")) {
> -                    unexpected = new RuntimeException("Unexpected msg: 
> missing reason for ");
> -                    break;
> -                }
> 
> whereas the new test is not seemingly (though 
> http://cr.openjdk.java.net/~jgeorge/8215568/webrev.00/test/hotspot/jtreg/serviceability/sa/TestClhsdbJstackLock.java.udiff.html 
> does do it so I think this is an oversight?).

Thank you very much for pointing this out! This was an oversight. I have 
added the unexpected strings check.

> 
>     - Also interesting is that the original test was trying to find one 
> of X:
> -                if (line.contains("VirtualCallData")  ||
> -                    line.contains("CounterData")      ||
> -                    line.contains("ReceiverTypeData")) {
> -                    knownProfileDataTypeFound = true;
> -                }
> 
> whereas you are now wanting to find all of them. Is that normal/wanted?

I was being extra cautious when I had written this test case in the 
beginning :-). As far as I have seen, the printmdo output does contain 
all of these. (The test passes with 50 repeated runs across all hs tiers 
with the changes -- so I believe it is OK).

I have the new webrev at:

http://cr.openjdk.java.net/~jgeorge/8215568/webrev.01/

I have additionally modified the copyright year to 2019.

Thank you,
Jini.


> 
> The rest looked good to me, though I wish there were a way to not have 
> to change all the strings to be regex friendly but I fail to see how to 
> do that without writing a runCmdWithoutRegexMatcher, which seems 
> overkill :-)
> Jc
> 
> On Tue, Dec 18, 2018 at 8:55 PM Jini George <jini.george at oracle.com 
> <mailto:jini.george at oracle.com>> wrote:
> 
>     Hello!
> 
>     Requesting reviews for:
> 
>     http://cr.openjdk.java.net/~jgeorge/8215568/webrev.00/
> 
>     BugID: https://bugs.openjdk.java.net/browse/JDK-8215568
> 
>     No new failures with the SA tests (hs-tiers 1-5) with these changes.
>     The
>     changes here include:
> 
>     * Refactoring the SA tests which test clhsdb commands to use
>     ClhsdbLauncher for uniformity and ease of maintainence.
>     * Testing for regular expressions with shouldMatch rather than
>     shouldContain.
>     * Minor cleanups.
> 
>     Thank you,
>     Jini.
> 
> 
> 
> 
> -- 
> 
> Thanks,
> Jc