Hi Gary,
updated webrev:
http://cr.openjdk.java.net/~amenkov/sh2java/timeout/webrev.02/
see comments inline.
On 09/17/2018 11:57, Gary Adams wrote:
Should sleepTime also be adjusted?
No.
sleepTime is delay before we read jdb output (i.e. we don't read jdb
output on every update, but check for new data once a second)
Should sleepTime and timeout be scoped to
just waitForPrompt?
They are defined at the block where all constants are defined.
I made the constants final.
On 9/17/18, 2:26 PM, Gary Adams wrote:
Is the log decoration typical?
98 jdb.log("=======================================");
This is to make clearer the exact point error occurs.
(I initially tried just log "Exception thrown during test execution: " +
e.getMessage(), but it's too inconspicuous)
Is the Utils.adjustTimeout() applied consistently?
e.g. is the timeout passed to waitFor() already adjusted?
waitFor timeouts are adjusted.
If you are promoting log() to be publicly visible, then it
should be used for the other cases of println() in Jdb and JdbTest.
fixed.
--alex
On 9/17/18, 1:58 PM, JC Beyler wrote:
Hi Alex,
Looks good to me,
Jc
On Mon, Sep 17, 2018 at 10:49 AM Alex Menkov
<alexey.men...@oracle.com <mailto:alexey.men...@oracle.com>> wrote:
Hi all,
please review small fix:
http://cr.openjdk.java.net/~amenkov/sh2java/timeout/webrev.01/
<http://cr.openjdk.java.net/%7Eamenkov/sh2java/timeout/webrev.01/>
It fixes
https://bugs.openjdk.java.net/browse/JDK-8210725
- accordingly the logs of the failing tests, they work as
expected, but
sometimes (busy environment?) there is no reply from jdb for 60
seconds,
so tests throw exception.
The timeout can be removed at all (so timeouts would be handled by
general jtreg timeout mechanism), but prefer keep smaller timeout
for
jdb reply, but apply timefactor to the value.
Also the change fixes
https://bugs.openjdk.java.net/browse/JDK-8210748
- in the case of error additional logging is done to indicate
certain
point where the error occurs.
--alex
--
Thanks,
Jc
