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@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.
