On 24/06/2012 13:57, Rob McKenna wrote:
Hi folks,
5th, and hopefully final review has been posted to:
http://cr.openjdk.java.net/~robm/4244896/webrev.05/
<http://cr.openjdk.java.net/%7Erobm/4244896/webrev.05/>
Let me know if there are any comments or concerns, and thanks a lot
for the help so far.
-Rob
Overall I think this has worked out well.
I agree with David on the needless calls to System.nanoTime in
Process.waitFor although it's not a major issue given that the
implementation in the JDK overrides this method.
The test additions, both in coverage and implementation, are much
cleaned now, and better test coverage too. A lot of the discussion here
has been on the default implementation of waitFor that Process provides
but it's not covered by the tests. I didn't notice this before but it
would be good to include a basic test for this, just so that code is
exercise. The simplest may be to create a concrete implementation of
Process that delegates to a Process instance returned by
ProcessBuilder.start. Assuming you don't override the waitFor
implementation then it means you will be able to exercise the default
waitFor rather than the platform implementation.
-Alan.
