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

[David Holmes]

Hi Alexey,

The popframe004 test is now failing in tier 4 - [CI] jdk13-jdk.26

Please file a bug and investigate.

Thanks,
David

On 19/12/2018 1:35 pm, JC Beyler wrote:
> Hi Alex,
> 
> Looks good to me as well :)
> 
> Nice job!
> Jc
> 
> On Tue, Dec 18, 2018 at 6:10 PM <serguei.spitsyn at oracle.com 
> <mailto:serguei.spitsyn at oracle.com>> wrote:
> 
>     Alex,
> 
>     Thank you for the update!
> 
>     It looks good.
>     There is another instance with incorrect spacing:
> 
>     121 totRes=doPopFrame(false, Thread.currentThread());
> 
> 
>     No need in new webrev if you fix the above.
> 
>     Thanks,
>     Serguei
> 
> 
>     On 12/18/18 6:01 PM, Alex Menkov wrote:
>>     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
>>     <mailto: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
>>>
> 
> 
> 
> -- 
> 
> Thanks,
> Jc