Looks good.
Chris
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
>
>
