[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR JDK-8215425: vmTestbase/nsk/jvmti/PopFrame should provide more detailed output
From:       David Holmes <david.holmes () oracle ! com>
Date:       2018-12-20 6:44:01
Message-ID: 63d7c1e7-ca48-1300-7e6e-bff9267b07cc () oracle ! com
[Download RAW message or body]

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@oracle.com 
> <mailto:serguei.spitsyn@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@oracle.com
> > <mailto:serguei.spitsyn@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


[prev in list] [next in list] [prev in thread] [next in thread]