Staffan, Looks good for me.
-Dmitry On 2014-07-01 15:44, Staffan Larsen wrote: > I’m still looking for a Review of this test code. > > Thanks, > /Staffan > > On 23 jun 2014, at 10:33, Staffan Larsen <staffan.lar...@oracle.com > <mailto:staffan.lar...@oracle.com>> wrote: > >> Fancy! >> >> new review: http://cr.openjdk.java.net/~sla/8046883/webrev.01/ >> >> /Staffan >> >> On 18 jun 2014, at 13:59, Peter Allwin <peter.all...@oracle.com >> <mailto:peter.all...@oracle.com>> wrote: >> >>> This looks a lot better! >>> >>> (Since we’re using fancy new features we could use streams to find >>> the connector instance) >>> >>> AttachingConnector ac = >>> Bootstrap.virtualMachineManager().attachingConnectors() >>> .stream() >>> .filter(c -> c.name().equals("com.sun.jdi.ProcessAttach")) >>> .findFirst() >>> .orElseThrow(() -> new RuntimeException("Unable to locate >>> ProcessAttachingConnector")); >>> >>> Thanks! >>> /peter >>> >>> On 17 Jun 2014, at 19:46, Staffan Larsen <staffan.lar...@oracle.com >>> <mailto:staffan.lar...@oracle.com>> wrote: >>> >>>> Here is a rewrite of the test in Java instead of a shell script. >>>> Should be easier to maintain. >>>> >>>> webrev: http://cr.openjdk.java.net/~sla/8046883/webrev.00/ >>>> >>>> Thanks, >>>> /Staffan >>>> >>>> On 17 jun 2014, at 15:12, Staffan Larsen <staffan.lar...@oracle.com >>>> <mailto:staffan.lar...@oracle.com>> wrote: >>>> >>>>> >>>>> On 17 jun 2014, at 15:03, Alan Bateman <alan.bate...@oracle.com >>>>> <mailto:alan.bate...@oracle.com>> wrote: >>>>> >>>>>> On 17/06/2014 13:35, Staffan Larsen wrote: >>>>>>> : >>>>>>> >>>>>>> It could be a timing issue, but in the other direction. If cygwin >>>>>>> hasn’t yet started the real windows process when I run ps, then >>>>>>> maybe ps will not list it. But given the “sleep 2” before the ps >>>>>>> invocation, the process should have had time to started. No >>>>>>> guarantees of course. >>>>>>> >>>>>>> Making the sleep shorter will not help as the process we are >>>>>>> starting will not terminate until we tell it to. >>>>>>> >>>>>>> >>>>>> Okay, although what I was suggesting is to use your patch but >>>>>> additionally move the sleep at L79 into the new while loop so that >>>>>> it doesn't spin quickly through the 10 iterations. That would give >>>>>> the test 10 attempts (and 10 seconds) to get the pid. >>>>> >>>>> Ah, I see. I misunderstood your comment. >>>>> >>>>> I started looking at rewriting the test in pure Java instead of the >>>>> shell script. With the new Process.getPid() this looks like the >>>>> best approach. I’ll come back with a new review request soon. >>>>> >>>>> /Staffan >>>> >>> >> > -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * I would love to change the world, but they won't give me the sources.
