RFR JDK-8209109: [TEST] rewrite com/sun/jdi shell tests to java version - step1

[Alex Menkov]

Hi Jc,

Thank you for the review.
Initial implementation had only a single wait while outputHandler get 
some new data.
But handling jdb output (actually detection of the point when jdb 
completes command execution and waits for user input) is quite tricky.
So after researching existing code which works with jdb I decided to add 
extra delays to minimize probability of false detection.

--alex

On 08/09/2018 11:46, JC Beyler wrote:
> Hi Alexey,
> 
> I'm evidently not a reviewer but the webrev looks great to me. Since the 
> new classes are to be used for other tests, I was looking a bit more 
> into them. I saw a small nit on the waitForMsg method:
> 
> In 
> http://cr.openjdk.java.net/~amenkov/sh2java/webrev/test/jdk/com/sun/jdi/lib/jdb/Jdb.java.html 
> <http://cr.openjdk.java.net/%7Eamenkov/sh2java/webrev/test/jdk/com/sun/jdi/lib/jdb/Jdb.java.html>:
>   174             try {
>   175                 Thread.sleep(sleepTime);
>   176             } catch (InterruptedException e) {
>   177                 // ignore
>   178             }
> 
> Seems like that try/catch could be removed entirely. The next block 
> says: if the outputHandler is empty, wait on it for a notify. So instead 
> of doing a :
> 
>    - Thread wait
>    - Check on outputHandler and wait on it
> 
> You would only check on the outputHandler and wake up when something new 
> came in. That might however provoke a lot of wake/notifies, on the other 
> hand, right now you do:
> 
> - Thread wait
> - Check if there is something and wake up at the first write
> - If that first write does not have the message
> - Thread wait
> 
> So, then the question becomes, is it really not simpler to "just" have 
> the thread wait?
> 
> But apart from that (and even with it), it looks great to me :)
> 
> Thanks for doing this!
> Jc
> 
> 
> 
> 
> 
> On Tue, Aug 7, 2018 at 5:09 PM Alex Menkov <alexey.menkov at oracle.com 
> <mailto:alexey.menkov at oracle.com>> wrote:
> 
>     Hi all,
> 
>     Please review a fix for
>     https://bugs.openjdk.java.net/browse/JDK-8209109
>     webrev:
>     http://cr.openjdk.java.net/~amenkov/sh2java/webrev/
>     <http://cr.openjdk.java.net/%7Eamenkov/sh2java/webrev/>
> 
>     This is a first step in shell to java conversion of com/sun/jdi tests
>     The fix introduces base classes to work with jdb and converts couple
>     tests.
> 
>     --alex
> 
> 
> 
> -- 
> 
> Thanks,
> Jc