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> 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> 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> 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> wrote: >>> >>>> >>>> On 17 jun 2014, at 15:03, Alan Bateman <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 >>> >> >
