Re: Request for review: 8005618 - TEST_BUG: java/lang/ProcessBuilder/Basic.java failing intermittently
On 15/01/2013 01:31, David Holmes wrote: On 15/01/2013 7:12 AM, Rob McKenna wrote: Simple enough fix but to be honest I'm not sure any value will *always* work for the dead process waitFor(). Our testing infrastructure seems to glide past whatever we consider to be acceptable tolerances. http://cr.openjdk.java.net/~robm/8005618/webrev.01/ http://cr.openjdk.java.net/%7Erobm/8005618/webrev.01/ Using the latch seems reasonable but the existing wait/sleep times do not. Why waitFor(1) if the main thread is going to interrupt you after a sleep(1000) ??? It's testing that Process.waitFor will be interrupted by Thread.interrupt so it requires a thread to block in waitFor. Using sleeps is always going to be problematic as the load on test machines is unpredictable but I think Rob's proposed change does make this test a bit more robust. -Alan.
Re: Request for review: 8005618 - TEST_BUG: java/lang/ProcessBuilder/Basic.java failing intermittently
On 15/01/2013 9:55 PM, Alan Bateman wrote: On 15/01/2013 01:31, David Holmes wrote: On 15/01/2013 7:12 AM, Rob McKenna wrote: Simple enough fix but to be honest I'm not sure any value will *always* work for the dead process waitFor(). Our testing infrastructure seems to glide past whatever we consider to be acceptable tolerances. http://cr.openjdk.java.net/~robm/8005618/webrev.01/ http://cr.openjdk.java.net/%7Erobm/8005618/webrev.01/ Using the latch seems reasonable but the existing wait/sleep times do not. Why waitFor(1) if the main thread is going to interrupt you after a sleep(1000) ??? It's testing that Process.waitFor will be interrupted by Thread.interrupt so it requires a thread to block in waitFor. Using sleeps is always going to be problematic as the load on test machines is unpredictable but I think Rob's proposed change does make this test a bit more robust. Ah I see. I'd missed that aspect of this. Thanks for clarifying. David -Alan.
Request for review: 8005618 - TEST_BUG: java/lang/ProcessBuilder/Basic.java failing intermittently
Simple enough fix but to be honest I'm not sure any value will *always* work for the dead process waitFor(). Our testing infrastructure seems to glide past whatever we consider to be acceptable tolerances. http://cr.openjdk.java.net/~robm/8005618/webrev.01/ http://cr.openjdk.java.net/%7Erobm/8005618/webrev.01/ -Rob
Re: Request for review: 8005618 - TEST_BUG: java/lang/ProcessBuilder/Basic.java failing intermittently
Looks good! Mea culpa for some of those slow and flaky sleeps in this code, although they are tricky to avoid - there is no cross-process latches, for example. On Mon, Jan 14, 2013 at 1:12 PM, Rob McKenna rob.mcke...@oracle.com wrote: Simple enough fix but to be honest I'm not sure any value will *always* work for the dead process waitFor(). Our testing infrastructure seems to glide past whatever we consider to be acceptable tolerances. http://cr.openjdk.java.net/~robm/8005618/webrev.01/ http://cr.openjdk.java.net/%7Erobm/8005618/webrev.01/ -Rob
Re: Request for review: 8005618 - TEST_BUG: java/lang/ProcessBuilder/Basic.java failing intermittently
On 15/01/2013 7:12 AM, Rob McKenna wrote: Simple enough fix but to be honest I'm not sure any value will *always* work for the dead process waitFor(). Our testing infrastructure seems to glide past whatever we consider to be acceptable tolerances. http://cr.openjdk.java.net/~robm/8005618/webrev.01/ http://cr.openjdk.java.net/%7Erobm/8005618/webrev.01/ Using the latch seems reasonable but the existing wait/sleep times do not. Why waitFor(1) if the main thread is going to interrupt you after a sleep(1000) ??? David -Rob
Re: Request for review: 8005618 - TEST_BUG: java/lang/ProcessBuilder/Basic.java failing intermittently
On Mon, Jan 14, 2013 at 5:31 PM, David Holmes david.hol...@oracle.com wrote: On 15/01/2013 7:12 AM, Rob McKenna wrote: Simple enough fix but to be honest I'm not sure any value will *always* work for the dead process waitFor(). Our testing infrastructure seems to glide past whatever we consider to be acceptable tolerances. http://cr.openjdk.java.net/~robm/8005618/webrev.01/ http://cr.openjdk.java.net/%7Erobm/8005618/webrev.01/ Using the latch seems reasonable but the existing wait/sleep times do not. Why waitFor(1) if the main thread is going to interrupt you after a sleep(1000) ??? Actually, in this case it would be even safer to sleep longer, i.e. impossibly long, without any testing performance problem. I am tempted to clean up a bunch of those other sleeps that actually do cause performance problems, as perhaps are you.
