On 6/6/18 17:37, David Holmes wrote:
Looks okay.

I didn't mention the spelling issues because they were everywhere and would just have obscured the fix initially. Once you start cleaning up these old tests you might never stop. ;-)

It is why I'm against filing a bug on the Debugee class. :)
I'm afraid it is too much of cleanup.

Thanks,
Serguei


Thanks,
David

On 7/06/2018 6:04 AM, Daniil Titov wrote:
Hi Serguei,

I updated the webrev to correct typo for debuggee instances as well.   However, I didn’t rename the shared class nsk.share.jdi.Debugee and nsk.share.jdi.Binder.bindToDebugee method  since I am not sure it should be done as a part of this fix. I would suggest filing a separate issue for that as that changes will affect multiple tests.

Webrev: http://cr.openjdk.java.net/~dtitov/8203033/webrev.04/

Issue: https://bugs.openjdk.java.net/browse/JDK-8203033

Thanks,

Daniil

*From: *"[email protected]" <[email protected]>
*Date: *Wednesday, June 6, 2018 at 12:18 PM
*To: *Daniil Titov <[email protected]>, <[email protected]> *Subject: *Re: RFR 8203033: [Testbug] vmTestbase/nsk/jdi/TypeComponent/isSynthetic/issynthetic002/TestDescription.java fails with nestmates

It looks good.
There are also instances of "debugee" but they are
because of the supporting class named "Debugee".

Thanks,
Serguei


On 6/6/18 12:09, Daniil Titov wrote:

    Thank you, Serguei and David, for reviewing this change.

    Please review a new version  of the change that corrects the
    diagnostic message as Serguei suggested  and fixes typos.

    Webrev: http://cr.openjdk.java.net/~dtitov/8203033/webrev.03
<http://cr.openjdk.java.net/%7Edtitov/8203033/webrev.03>

    Issue: https://bugs.openjdk.java.net/browse/JDK-8203033

    Best regards,

    Daniil

    *From: *"[email protected]"
    <mailto:[email protected]> <[email protected]>
    <mailto:[email protected]>
    *Date: *Wednesday, June 6, 2018 at 11:23 AM
    *To: *Daniil Titov <[email protected]>
    <mailto:[email protected]>,
    <[email protected]>
    <mailto:[email protected]>
    *Subject: *Re: RFR 8203033: [Testbug]
vmTestbase/nsk/jdi/TypeComponent/isSynthetic/issynthetic002/TestDescription.java
    fails with nestmates

    Hi Daniil,

    It looks good.
    One minor comment.

http://cr.openjdk.java.net/~dtitov/8203033/webrev.02/test/hotspot/jtreg/vmTestbase/nsk/jdi/TypeComponent/isSynthetic/issynthetic002.java.frames.html
<http://cr.openjdk.java.net/%7Edtitov/8203033/webrev.02/test/hotspot/jtreg/vmTestbase/nsk/jdi/TypeComponent/isSynthetic/issynthetic002.java.frames.html>

    153                     log.complain("debuger FAILURE 3> Method's "
    + name + signature

    154                             + " synthetic is true, but expected "

    155                             + "is false");

    The sub-string at lines 154 and 155 can be combined into one:
    + " synthetic is true, but expected is false");

    But I'd suggest to simplify it to something like this:
        log.complain("debugger FAILURE 3> found unexpected synthetic
    method " + name + signature);

    Also, there are many instances of the typo: debuger => debugger.
    Could you, please, fix it?

    Thank you a lot for taking care about this!

    Thanks,
    Serguei


    On 6/5/18 11:38, Daniil Titov wrote:

        Please review the change that fixes vmTestbase/nsk/jdi/TypeComponent/isSynthetic/issynthetic002/TestDescription.java test.


        The fix uses the implementation of a parametrized interface in the test class to trigger the compiler to generate a synthetic bridge method. The test works fine in both nestmates and jdk/jdk repositories.


        Issue:https://bugs.openjdk.java.net/browse/JDK-8203033
Webrev:http://cr.openjdk.java.net/~dtitov/8203033/webrev.02
<http://cr.openjdk.java.net/%7Edtitov/8203033/webrev.02>


        Thanks,

        Daniil






Reply via email to