Thank you, Serguei, Chris, JC, and Gary, for reviewing this change.
Best regards,
Daniil
On 5/2/19, 6:13 PM, "serguei.spit...@oracle.com" <serguei.spit...@oracle.com>
wrote:
Hi Daniil,
Looks good.
Thank you for the update.
Sorry, I forgot to tell no new webrev is needed if you fix this. :)
Thanks,
Serguei
On 5/2/19 6:09 PM, Daniil Titov wrote:
> Hi Serguei,
>
> Please review a new version of the fix that includes the changes you
suggested.
>
> Webrev: http://cr.openjdk.java.net/~dtitov/8222667/webrev.03
> Bug: https://bugs.openjdk.java.net/browse/JDK-8222667
>
> Thanks!
> --Daniil
>
> On 5/2/19, 5:24 PM, "serguei.spit...@oracle.com"
<serguei.spit...@oracle.com> wrote:
>
> Hi Daniil,
>
> Could you, please, replace 'e' with 'event'?
> I've never liked one-letter identifiers.
> Other than that it looks good to me.
>
> Thanks,
> Serguei
>
> On 5/2/19 9:48 AM, Daniil Titov wrote:
> > Hi Gary,
> >
> > Please review a new version of the webrev that adds the comment
you mentioned.
> >
> > Regarding the reusable filters I think it makes sense to create
them when we will find that they could be reused more than in one place and
this test is a quite specific, it creates ThreadStartRequest and enables it
before restricting it to the test thread only (by calling addThreadFilter())
since it is exactly what the test checks (that calling addThreadFilter() on
enabled thread start request throws InvalidRequestStateException.
> >
> > Webrev: http://cr.openjdk.java.net/~dtitov/8222667/webrev.02
> > Bug: https://bugs.openjdk.java.net/browse/JDK-8222667
> >
> > Thanks!
> > --Danill
> >
> > On 5/2/19, 4:56 AM, "Gary Adams" <gary.ad...@oracle.com> wrote:
> >
> > It would be good to include a comment that the thread start
is being
> > allowed because of graal.
> >
> > Now that we have a series of graal specific alterations in
the tests,
> > it might be useful to provide some reusable filters. $.02
> >
> > On 5/1/19, 9:07 PM, Daniil Titov wrote:
> > > Please review the change that fixes the test that
intermittently fails with Graal on.
> > >
> > > The test tests addThreadFilter() method for
com.sun.jdi.ThreadStartRequest class. It starts a debuggee, creates
ThreadStartRequest, enables it, and calls addThreadFilter(). The test also uses
breakpoints to synchronize with the debuggee, but the problem here is that
while waiting for a breakpoint event, occasionally, when Graal is on, the event
the test receives is a ThreadStartEvent for " HotSpotGraalManagement Bean
Registration" thread, rather than a breakpoint event. The test doesn't expect
it and fails.
> > >
> > > The fix takes this into account and makes the test keep
waiting for a breakpoint event instead of failing.
> > >
> > > Webrev:http://cr.openjdk.java.net/~dtitov/8222667/webrev.01/
> > > Bug:https://bugs.openjdk.java.net/browse/JDK-8222667
> > >
> > > Thanks!
> > > --Daniil
> > >
> > >
> >
> >
> >
> >
> >
>
>
>
>
