Hi,
Thank you for you patience.
On 11/13/2014 11:47 AM, Martin Buchholz wrote:
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)");
ok, then the special case of the AIX bump isn't necessary
---
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
ok, I was thinking that too short and the spawned process would not have
gotten started;
but that's not significant in the test.
---
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.
The exitValue was intended to help with further debugging.
Objects.toString was intended.
But it is as effective to leave the ITSE fall into the unexpected
exception handler.
Webrev updated.
Thanks, Roger
On Thu, Nov 13, 2014 at 7:37 AM, roger riggs <roger.ri...@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.ri...@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.
