RFR JDK-8215425: vmTestbase/nsk/jvmti/PopFrame should provide more detailed output

[Alex Menkov]

Hi Serguei,

Thank you for the review.
Updated webrev:
http://cr.openjdk.java.net/~amenkov/popframe_cleanup/webrev.02/

On 12/18/2018 16:49, serguei.spitsyn at oracle.com wrote:
> Hi Alex,
> 
> A couple of minor comments.
> 
> http://cr.openjdk.java.net/~amenkov/popframe_cleanup/webrev.01/test/hotspot/jtreg/vmTestbase/nsk/jvmti/PopFrame/popframe002.java.frames.html
> http://cr.openjdk.java.net/~amenkov/popframe_cleanup/webrev.01/test/hotspot/jtreg/vmTestbase/nsk/jvmti/PopFrame/popframe004.java.frames.html
> 
>    While you are at these files, could you, fix several originally ugly 
> indented comments?

Done.

> 
>    Could you, also, fix the incorrect spacing around '=' in the 
> popframe004.java:
> 
> 95 retValue=doPopFrame(true, popFrameClsThr);

Done.

Also fixed comment in popframe004/TestDescription.java (to be in sync 
with comment change in popframe004.java)

> 
> 
>    Could you give an idea about the motivation to remove the following 
> fragment ?:

This is "test case 2" in popframe004 which I mentioned in the review 
request.
The code is never executed (if it is, this means the test has already 
failed) and I don't have an idea what other case can be tested here.

--alex

> 
> 167 if (popframe004.popFdone) { // popping has been done
> 168 if (DEBUG_MODE)
> 169 out.println("popFrameCls (" + this +
> 170 "): enter activeMethod() after popping");
> 171 // test case #2: popping from the current thread
> 172 if (!popframe004.popF2done) {
> 173 popframe004.popF2done = true;
> 174 if (DEBUG_MODE) {
> 175 out.println("popFrameCls (" + this +
> 176 "): going to pop a frame from the current thread...");
> 177 retVal = doPopFrame(3, popFrameClsThr);
> 178 } else
> 179 retVal = doPopFrame(2, popFrameClsThr);
> 180 if (retVal != PASSED)
> 181 popframe004.totRes = FAILED;
> 182 }
> 183 if (DEBUG_MODE)
> 184 out.println("popFrameCls (" + this +
> 185 "): leaving activeMethod()...");
> 186 return;
> 187 }
> 
> 
> Otherwise, it looks good.
> 
> Thanks,
> Serguei
> 
> 
> On 12/18/18 1:37 PM, Alex Menkov wrote:
>> Hi all,
>>
>> please review the fix for
>> https://bugs.openjdk.java.net/browse/JDK-8215425
>> webrev:
>> http://cr.openjdk.java.net/~amenkov/popframe_cleanup/webrev.01/
>>
>> The fix
>> - turns on detailed output for the tests and cleaned related stuff;
>> - for popframe002 deletes output from run() method as it caused 
>> unexpected MethodExit event;
>> - removes "test case 2" in popframe004 test as it's never executed.
>>
>> --alex
>