On 6/23/14 2:33 AM, Staffan Larsen wrote:
Fancy!
new review: http://cr.openjdk.java.net/~sla/8046883/webrev.01/
<http://cr.openjdk.java.net/%7Esla/8046883/webrev.01/>
test/com/sun/jdi/ProcessAttachTest.java
New test looks very good. There's a couple of really long lines,
but I can't think of a way to break the lines that makes sense
with this new streams coding style.
Thumbs up.
Dan
/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/
<http://cr.openjdk.java.net/%7Esla/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
