RFR: JDK-8192985: SA: Test cases for the clhsdb 'inspect', 'scanoops' and 'printas' commands

[serguei.spitsyn at oracle.com]

Hi Jini,

Some minor comments:
http://cr.openjdk.java.net/~jgeorge/8192985/webrev.01/test/hotspot/jtreg/serviceability/sa/ClhsdbInspect.java.html

I'd suggest to unsplit some lines (check if same applies to other two 
tests):

   56             String jstackOutput =
   57                 test.run(theApp.getPid(), cmds, null, null);
   ...

   78                             addressString =
   79                                 (token.replace("<", "")).replace(">", "");

   No need for brackets around the token.replace.

Need a space after 'for':

   70                 for(String key: tokensMap.keySet()) {



Q: In what condition the jstackOutput can be null?

      59             if (jstackOutput != null) {

    A suggestion is to use the same approach as in the ClhsdbScanOops test:

   60             if (universeOutput == null) {
   61                 // Output could be null due to attach permission issues
   62                 // and if we are skipping this.
   63                 LingeredApp.stopApp(theApp);
   64                 return;
   65             }


Otherwise, the fix looks good to me.

Thanks,
Serguei



On 12/12/17 02:38, Jini George wrote:
> Thank you, Sharath. I have a modified webrev at:
>
> http://cr.openjdk.java.net/~jgeorge/8192985/webrev.01/
>
> Could a Reviewer also please take a look at it ?
>
> Thanks,
> Jini.
>
> On 12/11/2017 3:41 PM, Sharath Ballal wrote:
>> Hi Jini,
>> Looks Good. Some nits:
>>
>> http://cr.openjdk.java.net/~jgeorge/8192985/webrev.00/test/hotspot/jtreg/serviceability/sa/ClhsdbInspect.java.html 
>>
>>
>> Since you are not passing any new VM options, the following lines
>>
>>     28         import jdk.test.lib.Utils;
>>    47             List<String> vmArgs = new ArrayList<String>();
>>    48             vmArgs.addAll(Utils.getVmOptions());
>>    49
>>    50             theApp = new LingeredAppWithLock();
>>    51             LingeredApp.startApp(vmArgs, theApp);
>>
>> Can be replaced by
>>
>> theApp = new LingeredAppWithLock();
>> LingeredApp.startApp(null, theApp);
>>
>> http://cr.openjdk.java.net/~jgeorge/8192985/webrev.00/test/hotspot/jtreg/serviceability/sa/ClhsdbScanOops.java.html 
>>
>>
>> 41     public static void testWithGcType
>> If you are not planning on this method being called from elsewhere, 
>> you can make it private.
>>
>> Thanks,
>> Sharath
>>
>>
>> -----Original Message-----
>> From: Jini George
>> Sent: Friday, December 08, 2017 12:33 PM
>> To: serviceability-dev at openjdk.java.net
>> Subject: RFR: JDK-8192985: SA: Test cases for the clhsdb 'inspect', 
>> 'scanoops' and 'printas' commands
>>
>> Hello,
>>
>> Requesting reviews for:
>>
>> JBS Id: https://bugs.openjdk.java.net/browse/JDK-8192985
>> Webrev: http://cr.openjdk.java.net/~jgeorge/8192985/webrev.00/
>>
>> These are the new test cases for the following clhsdb commands:
>> 1. inspect
>> 2. scanoops
>> 3. printas
>>
>> These tests have been verified through the Mach5 and jprt systems.
>>
>> Thanks,
>> Jini.
>>