RFR - JDK-8209517: com/sun/jdi/BreakpointWithFullGC.java fails with timeout

[Alex Menkov]

Hi Dan,

Thank you for the review.
See replies inline.

On 08/15/2018 17:25, Daniel D. Daugherty wrote:
> On 8/15/18 6:42 PM, Alex Menkov wrote:
>> Hi all,
>>
>> please review a fix for
>> https://bugs.openjdk.java.net/browse/JDK-8209517
>> webrev:
>> http://cr.openjdk.java.net/~amenkov/sh2java/step1_regression/webrev/
> 
> test/jdk/com/sun/jdi/BreakpointWithFullGC.java
>      No comments.
> 
> test/jdk/com/sun/jdi/lib/jdb/Jdb.java
>      L101:                     30, TimeUnit.SECONDS);
>          I'm assuming this is a 30 second timeout for getting the
>          initial "Listening for transport..." message. Keep in mind
>          that this may need to scale with timeoutFactor. I have seen
>          runs with -Xcomp take several minutes here...
> 
>      L168:                 waitFor(10, TimeUnit.SECONDS);
>      L178:                 debuggee.waitFor(10, TimeUnit.SECONDS);
>          10 seconds may not be enough on a loaded machine with
>          lots of parallel processes. Keep in mind that this
>          may need to scale with timeoutFactor.

Good point.
I'll think how to handle this properly and will update timeouts in one 
of the subsequent fixes. I'd prefer to push this fix as soon as I get 
2nd approval (as it produces noise in CI).

> 
> test/jdk/com/sun/jdi/lib/jdb/JdbCommand.java
>      No comments.
> 
> test/jdk/com/sun/jdi/lib/jdb/JdbTest.java
>      No comments.
> 
> Looks good to me. Is BreakpointWithFullGC.java the only test
> that uses the com/sun/jdi/lib/jdb/*.java library code? That
> would surprise me, but I haven't been in this test code for
> a very long time.

com/sun/jdi/lib/jdb classes were introduced to be used by tests 
converted from com/sun/jdi/*.sh 
(https://bugs.openjdk.java.net/browse/JDK-8201652)
For now 2 tests are converted:
test/jdk/com/sun/jdi/BreakpointWithFullGC.java
test/jdk/com/sun/jdi/ArrayLengthDumpTest.java

ArrayLengthDumpTest.java does not require to be updated (works fine with 
updated lib/jdb)

--alex

> 
> Thumbs up!
> 
> Dan
> 
> 
>>
>> Cause of the BreakpointWithFullGC failures is a mess of jdb and 
>> debuggee outputs (the test runs debuggee by using CommandLineLaunch 
>> connector, so jdb redirects debuggee stdout to its own stdout).
>> To solve it test framework was updated to launch debuggee first 
>> (redirecting its output) and then connecting jdb to existing process.
>> The approach allow to drop "simple prompt" logic (jdb mode when 
>> debugee is not yet running).
>>
>> Mach5 passed 400 runs ("4 std platforms" x "--test-repeat 100") 
>> without any issues.
>>
>> --alex
>