Hi Jini,

New tests look Ok to me.

Thanks,
Serguei


On 12/1/17 00:23, Jini George wrote:
Had missed removing the unused line:

import jdk.test.lib.Platform;

from ClhsdbVmStructsDump.java (Thanks, Sharath, for pointing this out).

Revised webrev:

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

Thanks,
Jini.

On 12/1/2017 1:40 PM, Jini George wrote:
Thank you, Sharath, for the review. The modified webrev is at:

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

Could I get one more pair of eyes to take a look at this ?

Thanks,
Jini.



On 11/30/2017 11:35 PM, Sharath Ballal wrote:
Hi Jini,

http://cr.openjdk.java.net/~jgeorge/8191538/webrev.00/test/hotspot/jtreg/serviceability/sa/ClhsdbField.java.html and http://cr.openjdk.java.net/~jgeorge/8191538/webrev.00/test/hotspot/jtreg/serviceability/sa/ClhsdbSymbolTable.java.html

If you are not adding any new vmoption then

   48             List<String> vmArgs = new ArrayList<String>();
   49             vmArgs.addAll(Utils.getVmOptions());
   50
   51             theApp = new LingeredApp();
   52             LingeredApp.startApp(vmArgs, theApp);

Can be replaced by

             theApp = LingeredApp.startApp();

Internally LingeredApp.startApp() is adding all the options got from Utils.getVmOptions()

If you remove that, then following lines are also not required
27 import java.util.ArrayList;
   30 import jdk.test.lib.Utils;


http://cr.openjdk.java.net/~jgeorge/8191538/webrev.00/test/hotspot/jtreg/serviceability/sa/ClhsdbVmStructsDump.java.html

This line is not required.
29 import jdk.test.lib.Platform;



Thanks,
Sharath (not a Reviewer)


-----Original Message-----
From: Jini George
Sent: Thursday, November 30, 2017 11:11 AM
To: serviceability-dev@openjdk.java.net
Subject: RFR: JDK-8191538: SA: tests for clhsdb commands: vmstructsdump, field, symboltable and symbol

Hi all,

Would like to request for reviews for:

JBS id: https://bugs.openjdk.java.net/browse/JDK-8191538
Webrev: http://cr.openjdk.java.net/~jgeorge/8191538/webrev.00/

These are SA jtreg tests to test the following clhsdb commands:
* field
* vmstructsdump
* symboltable
* symbol

Thanks,
Jini.





Reply via email to