RFR 9 8043477: java/lang/ProcessBuilder/Basic.java failed with: java.lang.AssertionError: Some tests failed

[Martin Buchholz]

You should delete your import

  36 import java.lang.IllegalStateException;

---

I would bump up the time you're willing to wait here, by at least 10x.

2277             if ((end - start) > 200000000L * (AIX.is() ? 2 : 1))
2278                 fail("Test failed: waitFor took too long (" +
(end - start) + "ns)");

---

When you are surely waiting, it's best to optimize by waiting as
little as possible (although jtreg concurrency helps)

2301             p.waitFor(1, TimeUnit.SECONDS);

I would set the timeout to only 10 millis here, and check that the
operation took at least 10 millis and no more than your long timeout
(you've settled on 7 seconds?)
as in jsr166 tck tests

---

2315                 exitValue = p.exitValue();
2316             } catch (IllegalThreadStateException ise) {
2317                 // no exitValue
2318             }

If ITSE exception is thrown by exitValue, then the test should always
fail but that is not currently the case.
If the test does fail, it currently throws uninformative NPE in
bjects.requireNonNull(exitValue), and we can do better.

On Thu, Nov 13, 2014 at 7:37 AM, roger riggs <roger.riggs at oracle.com> wrote:
> Please re-review,
>
> Corrected the webrev, and a few changes to consistently use the same
> units, thanks for the review:
>
> The timeout time is extended to wait up to 7 seconds
> and the initial waitFor should last at least 1 second.
>
> http://cr.openjdk.java.net/~rriggs/webrev-basic-debug-8043477/
>
> Roger
>
>
>
>
> On 11/12/2014 10:42 PM, Martin Buchholz wrote:
>>
>> On Wed, Nov 12, 2014 at 6:25 PM, roger riggs <roger.riggs at oracle.com>
>> wrote:
>>>
>>> Hi Martin,
>>>
>>> I think that sleep is the version that spawns a JavaChild process and
>>> has its own comment interpreter. See line 309: Am I getting the sleeps
>>> mixed
>>> up?
>>
>> Ah, good point - yes, you're right.  Although we could always bump up
>> the sleep of the JavaChild from 10 if we wanted to - it should be
>> comfortably bigger than any individual test case's need.
>
>