RFR (L) 8228998: Remove the testing against NSK_FALSE from tests

[Jean Christophe Beyler]

Hi Serguei,

Thanks :)

Done, I updated it and the copyrights and did an in place replacement:

Webrev: http://cr.openjdk.java.net/~jcbeyler/8228998/webrev.01/
Bug: https://bugs.openjdk.java.net/browse/JDK-8228998

Thanks again,
Jc

On Thu, Aug 1, 2019 at 5:27 PM <serguei.spitsyn at oracle.com> wrote:

> Hi Jc,
>
> Looks good.
>
> This still aligned incorrectly:
>
>  221 static bool getFieldsAndObjects(jvmtiEnv*  jvmti, 222                                 JNIEnv*     jni, 223                                 jclass      debugeeClass, 224                                 jclass      rootObjectClass, 225                                 jclass      chainObjectClass, 226                                 jobject*    rootObjectPtr, 227                                 jfieldID*   reachableChainField, 228                                 jfieldID*   unreachableChainField, 229                                 jfieldID*   nextField) {
>
>
> Some copyright comments need an update.
> No need in another review if you fix it.
> You may want to update the webrev in place for other reviewers.
>
> Thanks,
> Serguei
>
>
> On 8/1/19 4:53 PM, Jean Christophe Beyler wrote:
>
> Hi Serguei,
>
> My apologies. I fixed the forbidden on the old webrev link. Then I
> rechecked the white-spaces, and made webrev not ignore white space changes.
> Here is the new webrev:
>
> Webrev: http://cr.openjdk.java.net/~jcbeyler/8228998/webrev.01/
> Bug: https://bugs.openjdk.java.net/browse/JDK-8228998
>
> Thanks for your review :)
> Jc
>
>
> On Thu, Aug 1, 2019 at 4:07 PM <serguei.spitsyn at oracle.com> wrote:
>
>> Hi Jc,
>>
>> Thank you for taking care about this!
>> Most of the links in the webrev can not be resolved.
>> I'm getting the error: "403 - Forbidden".
>>
>> The only item that works is:
>>
>> Cdiffs
>> <http://cr.openjdk.java.net/%7Ejcbeyler/8228998/webrev/test/hotspot/jtreg/vmTestbase/nsk/jvmti/unit/FollowReferences/followref001/followref001.cpp.cdiff.html>
>> Udiffs
>> <http://cr.openjdk.java.net/%7Ejcbeyler/8228998/webrev/test/hotspot/jtreg/vmTestbase/nsk/jvmti/unit/FollowReferences/followref001/followref001.cpp.udiff.html>
>> Sdiffs
>> <http://cr.openjdk.java.net/%7Ejcbeyler/8228998/webrev/test/hotspot/jtreg/vmTestbase/nsk/jvmti/unit/FollowReferences/followref001/followref001.cpp.sdiff.html>
>> Frames
>> <http://cr.openjdk.java.net/%7Ejcbeyler/8228998/webrev/test/hotspot/jtreg/vmTestbase/nsk/jvmti/unit/FollowReferences/followref001/followref001.cpp.frames.html>
>> Old
>> <http://cr.openjdk.java.net/%7Ejcbeyler/8228998/webrev/test/hotspot/jtreg/vmTestbase/nsk/jvmti/unit/FollowReferences/followref001/followref001.cpp-.html>
>> New
>> <http://cr.openjdk.java.net/%7Ejcbeyler/8228998/webrev/test/hotspot/jtreg/vmTestbase/nsk/jvmti/unit/FollowReferences/followref001/followref001.cpp.html>
>> ----- Raw
>> <http://cr.openjdk.java.net/%7Ejcbeyler/8228998/webrev/raw_files/new/test/hotspot/jtreg/vmTestbase/nsk/jvmti/unit/FollowReferences/followref001/followref001.cpp>
>>
>> *test/hotspot/jtreg/vmTestbase/nsk/jvmti/unit/FollowReferences/followref001/followref001.cpp*
>> Also, the patch is readable:
>>
>> http://cr.openjdk.java.net/%7Ejcbeyler/8228998/webrev/jdk-false.changeset
>>
>>
>> It looks pretty good.
>>
>> Only a one comments:
>>
>>
>> http://cr.openjdk.java.net/%7Ejcbeyler/8228998/webrev/test/hotspot/jtreg/vmTestbase/nsk/jvmti/unit/FollowReferences/followref001/followref001.cpp.frames.html
>>
>> A couple of fragments are not aligned properly:
>>
>>  221 static bool getFieldsAndObjects(jvmtiEnv*  jvmti,
>>  222                                 JNIEnv*     jni,
>>  223                                 jclass      debugeeClass,
>>  . . .
>>
>>  434 static bool checkTestedObjects(jvmtiEnv*  jvmti,
>>  435                               JNIEnv*    jni,
>>  436                               int        chainLength,
>>  437                               ObjectDesc objectDescList[])
>>
>>
>> Thanks,
>> Serguei
>>
>>
>> On 8/1/19 3:16 PM, Jean Christophe Beyler wrote:
>>
>> Hi all,
>>
>> It took me a while to pick this item back but here we go :-). Here is a
>> webrev that removes all the if (.* == NSK_FALSE) and replaces them with if
>> (! .*).
>>
>> Webrev: http://cr.openjdk.java.net/~jcbeyler/8228998/webrev/
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8228998
>>
>> For the EM tests, I also updated the returns to be boolean, entirely
>> removing the NSK_FALSE/NSK_TRUE parts because of the way the tests were
>> done. Let me know if you'd rather I divide those up.
>>
>> This was tested by running the tests changed on my dev machine, I'll push
>> it to the submit repo after review :-)
>>
>> Thanks and have a great evening,
>> Jc
>>
>>
>>
>
> --
>
> Thanks,
> Jc
>
>
>

-- 

Thanks,
Jc
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20190801/e0774c58/attachment.html>