I have no further comments/questions.
On 9/20/18 11:52 AM, JC Beyler wrote:
Hi Alex,
Looks good to me still :)
Jc
On Wed, Sep 19, 2018 at 5:21 PM Alex Menkov <alexey.men...@oracle.com
<mailto:alexey.men...@oracle.com>> wrote:
Hi all,
Ping.
The bug has been labeled "timewaster" as it caused failures quite
often.
--alex
On 09/17/2018 12:57, Alex Menkov wrote:
> Hi Gary,
>
> updated webrev:
> http://cr.openjdk.java.net/~amenkov/sh2java/timeout/webrev.02/
<http://cr.openjdk.java.net/%7Eamenkov/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>
<mailto: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/>
>>>>
<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
>>>
>>
--
Thanks,
Jc
