Looks good Jini.
Thanks, Sharath -----Original Message----- From: Jini George Sent: Friday, December 01, 2017 1:54 PM To: serviceability-dev@openjdk.java.net Subject: Re: RFR: JDK-8191538: SA: tests for clhsdb commands: vmstructsdump, field, symboltable and symbol 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. >> >> >> >>