Looks good to me. BTW, how did you track down this failure. Did you use some tracing option, or did you add a diagnostic to dump the stray event?
On 5/2/19, 12:48 PM, 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 > >
