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

[Jini George]

Thank you, Serguei. I have the modified webrev after addressing the 
comments at:

http://cr.openjdk.java.net/~jgeorge/8192985/webrev.02/

jstackOutput could be null if there are attach permission issues.

Thanks,
Jini.


On 12/13/2017 4:44 AM, serguei.spitsyn at oracle.com wrote:
> 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.
>>>
>