review request: 4244896: (process) Provide System.getPid(), System.killProcess(String pid)

[Alan Bateman]

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.