Hi Olivier,
Ok, reviewed.
Thank you for the update!
Serguei
On 7/31/15 9:21 AM, olivier.lagn...@oracle.com wrote:
Hi Serguey,
Thanks for reviewing. Please see comments inlined.
On 31/07/2015 01:50, serguei.spit...@oracle.com wrote:
Hi Olivier,
I agree with the comments from Jaroslav.
A couple of minor comments.
+ if ( exitCode != 0 ) {
Unneeded extra spaces
Ok. fixed.
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ throw new RuntimeException("Parent process interrupted with exception :
\n " + e);
+ }
+
Why a call to interrupt() is needed?
Sorry. This is a mistake. Removing it.
I'd suggest to add a call e.printStackTrace().
In case of such failure JTREG/JPRT already prints the stack strace.
So currently the output will look like this in case of failure :
"
java.lang.RuntimeException: Parent process interrupted with exception :
java.lang.InterruptedException :
at LowMemoryTest.traceTest(LowMemoryTest.java:143)
at LowMemoryTest.main(LowMemoryTest.java:82)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at
sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:504)
at
com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:92)
at java.lang.Thread.run(Thread.java:746)
"
Which I find more readable.
Otherwise, it looks good.
Thanks,
Serguei
On 7/30/15 9:55 AM, olivier.lagn...@oracle.com wrote:
Hi,
Could you please review this fix:
bug: https://bugs.openjdk.java.net/browse/JDK-8131784
webrev: http://cr.openjdk.java.net/~olagneau/8131784/webrev.00/
We add tracing information to help diagnosing master bug problem
(https://bugs.openjdk.java.net/browse/JDK-8130339),
and we will monitor the quarantined runs to check for any failure
and get more diagnosis info.
Since the test already provides some information which is not
collected when it fails with timeout,
the idea is to collect subprocess I/O (stdout, stderr) by using
ProcessTools.startProcess rather
that OutputAnalyzer.executeProcess. The subprocess I/Os will be
collected while it's running
rather than waiting the subprocess termination.
A few additional infos have also been added to get more information.
Note that this "tracing" change should be temporary and will be
removed when a fix for 8130339 has been found.
Thanks,
Olivier.
