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

JC Beyler jcbeyler at google.com
Thu Aug 9 18:46:42 UTC 2018


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
:
 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> 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/
>
> 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20180809/7040dc63/attachment.html>


More information about the serviceability-dev mailing list