[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-serviceability-dev
Subject: Re: RFR (L) 8228998: Remove the testing against NSK_FALSE from tests
From: Chris Plummer <chris.plummer () oracle ! com>
Date: 2019-08-02 6:48:32
Message-ID: 80805460-255b-1ef3-6b49-f6f25487c022 () oracle ! com
[Download RAW message or body]
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">Looks good.</div>
<div class="moz-cite-prefix"><br>
</div>
<div class="moz-cite-prefix">Chris<br>
</div>
<div class="moz-cite-prefix"><br>
</div>
<div class="moz-cite-prefix">On 8/1/19 6:55 PM, Jean Christophe
Beyler wrote:<br>
</div>
<blockquote type="cite"
cite="mid:CAF9BGBxk3=B-2YUFgmP7nZdSeWtc9x1BwHHLw1VhMeTf2-qAEQ@mail.gmail.com">
<meta http-equiv="content-type" content="text/html; charset=UTF-8">
<div dir="ltr">Hi Serguei,<br>
<div><br>
</div>
<div>Thanks :)</div>
<div><br>
</div>
<div>Done, I updated it and the copyrights and did an in place
replacement:</div>
<div><br>
</div>
<div>
<div>Webrev: <a
href="http://cr.openjdk.java.net/~jcbeyler/8228998/webrev.01/"
target="_blank" \
moz-do-not-send="true">http://cr.openjdk.java.net/~jcbeyler/8228998/webrev.01/</a></div>
<div>Bug: <a
href="https://bugs.openjdk.java.net/browse/JDK-8228998"
target="_blank" \
moz-do-not-send="true">https://bugs.openjdk.java.net/browse/JDK-8228998</a></div> \
</div> <div><br>
</div>
<div>Thanks again,</div>
<div>Jc</div>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Thu, Aug 1, 2019 at 5:27 PM
<<a href="mailto:serguei.spitsyn@oracle.com"
moz-do-not-send="true">serguei.spitsyn@oracle.com</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px
0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div bgcolor="#FFFFFF"> Hi Jc,<br>
<br>
Looks good.<br>
<br>
This still aligned incorrectly:<br>
<pre><span class="gmail-m_-4465974035326398045changed"> 221 static bool \
getFieldsAndObjects(jvmtiEnv* jvmti,</span> <span \
class="gmail-m_-4465974035326398045changed"> 222 \
JNIEnv* jni,</span> <span class="gmail-m_-4465974035326398045changed"> 223 \
jclass debugeeClass,</span> <span class="gmail-m_-4465974035326398045changed"> \
224 jclass rootObjectClass,</span> <span \
class="gmail-m_-4465974035326398045changed"> 225 \
jclass chainObjectClass,</span> <span \
class="gmail-m_-4465974035326398045changed"> 226 \
jobject* rootObjectPtr,</span> <span class="gmail-m_-4465974035326398045changed"> \
227 jfieldID* reachableChainField,</span> <span \
class="gmail-m_-4465974035326398045changed"> 228 \
jfieldID* unreachableChainField,</span> <span \
class="gmail-m_-4465974035326398045changed"> 229 \
jfieldID* nextField) {</span></pre> <br>
Some copyright comments need an update.<br>
No need in another review if you fix it.<br>
You may want to update the webrev in place for other
reviewers.<br>
<br>
Thanks,<br>
Serguei<br>
<br>
<br>
<div class="gmail-m_-4465974035326398045moz-cite-prefix">On
8/1/19 4:53 PM, Jean Christophe Beyler wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">Hi Serguei,
<div><br>
</div>
<div>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:</div>
<div><br>
</div>
<div>Webrev: <a
href="http://cr.openjdk.java.net/%7Ejcbeyler/8228998/webrev.01/"
target="_blank" \
moz-do-not-send="true">http://cr.openjdk.java.net/~jcbeyler/8228998/webrev.01/</a></div>
<div>Bug: <a
href="https://bugs.openjdk.java.net/browse/JDK-8228998"
target="_blank" \
moz-do-not-send="true">https://bugs.openjdk.java.net/browse/JDK-8228998</a></div> \
<div><br> </div>
<div>Thanks for your review :)</div>
<div>Jc</div>
<div><br>
</div>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Thu, Aug 1, 2019 at
4:07 PM <<a
href="mailto:serguei.spitsyn@oracle.com"
target="_blank" \
moz-do-not-send="true">serguei.spitsyn@oracle.com</a>> wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px
0px 0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
<div bgcolor="#FFFFFF"> Hi Jc,<br>
<br>
Thank you for taking care about this!<br>
Most of the links in the webrev can not be resolved.<br>
I'm getting the error: "403 - Forbidden".<br>
<br>
The only item that works is:<br>
<p><code> <a
href="http://cr.openjdk.java.net/%7Ejcbeyler/8228998/webrev/test/hotspot/jtreg/vmTestbase/nsk/jvmti/unit/FollowReferences/followref001/followref001.cpp.cdiff.html"
target="_blank" moz-do-not-send="true">Cdiffs</a>
<a
href="http://cr.openjdk.java.net/%7Ejcbeyler/8228998/webrev/test/hotspot/jtreg/vmTestbase/nsk/jvmti/unit/FollowReferences/followref001/followref001.cpp.udiff.html"
target="_blank" moz-do-not-send="true">Udiffs</a>
<a
href="http://cr.openjdk.java.net/%7Ejcbeyler/8228998/webrev/test/hotspot/jtreg/vmTestbase/nsk/jvmti/unit/FollowReferences/followref001/followref001.cpp.sdiff.html"
target="_blank" moz-do-not-send="true">Sdiffs</a>
<a
href="http://cr.openjdk.java.net/%7Ejcbeyler/8228998/webrev/test/hotspot/jtreg/vmTestbase/nsk/jvmti/unit/FollowReferences/followref001/followref001.cpp.frames.html"
target="_blank" moz-do-not-send="true">Frames</a>
<a
href="http://cr.openjdk.java.net/%7Ejcbeyler/8228998/webrev/test/hotspot/jtreg/vmTestbase/nsk/jvmti/unit/FollowReferences/followref001/followref001.cpp-.html"
target="_blank" moz-do-not-send="true">Old</a>
<a
href="http://cr.openjdk.java.net/%7Ejcbeyler/8228998/webrev/test/hotspot/jtreg/vmTestbase/nsk/jvmti/unit/FollowReferences/followref001/followref001.cpp.html"
target="_blank" moz-do-not-send="true">New</a>
----- <a
href="http://cr.openjdk.java.net/%7Ejcbeyler/8228998/webrev/raw_files/new/test/hotspot/jtreg/vmTestbase/nsk/jvmti/unit/FollowReferences/followref001/followref001.cpp"
target="_blank" moz-do-not-send="true">Raw</a>
<br>
</code> \
<b>test/hotspot/jtreg/vmTestbase/nsk/jvmti/unit/FollowReferences/followref001/followref001.cpp</b>
</p>
Also, the patch is readable:<br>
<a
class="gmail-m_-4465974035326398045gmail-m_6604593012462213921moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Ejcbeyler/8228998/webrev/jdk-false.changeset"
target="_blank" \
moz-do-not-send="true">http://cr.openjdk.java.net/%7Ejcbeyler/8228998/webrev/jdk-false.changeset</a><br>
<br>
<br>
It looks pretty good.<br>
<br>
Only a one comments:<br>
<br>
<a
class="gmail-m_-4465974035326398045gmail-m_6604593012462213921moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Ejcbeyler/8228998/webrev/test/hotspot/jtreg/vmTestbase/nsk/jvmti/unit/FollowReferences/followref001/followref001.cpp.frames.html"
target="_blank" moz-do-not-send="true">http://cr.openjdk.java.net/%7Ejcbeyler/822899 \
8/webrev/test/hotspot/jtreg/vmTestbase/nsk/jvmti/unit/FollowReferences/followref001/followref001.cpp.frames.html</a><br>
<br>
A couple of fragments are not aligned properly:<br>
<pre><span \
class="gmail-m_-4465974035326398045gmail-m_6604593012462213921changed"> 221 static \
bool getFieldsAndObjects(jvmtiEnv* jvmti,</span> 222 \
JNIEnv* jni, 223 jclass debugeeClass,
. . . <span class="gmail-m_-4465974035326398045gmail-m_6604593012462213921changed"></span></pre>
<pre><span \
class="gmail-m_-4465974035326398045gmail-m_6604593012462213921changed"> 434 static \
bool checkTestedObjects(jvmtiEnv* jvmti,</span> 435 \
JNIEnv* jni, 436 int chainLength,
437 ObjectDesc objectDescList[])</pre>
<br>
Thanks,<br>
Serguei<br>
<br>
<br>
<div
class="gmail-m_-4465974035326398045gmail-m_6604593012462213921moz-cite-prefix">On
8/1/19 3:16 PM, Jean Christophe Beyler wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">Hi all,
<div><br>
</div>
<div>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 (! .*).</div>
<div><br>
</div>
<div>Webrev: <a
\
href="http://cr.openjdk.java.net/%7Ejcbeyler/8228998/webrev/"
target="_blank" \
moz-do-not-send="true">http://cr.openjdk.java.net/~jcbeyler/8228998/webrev/</a></div> \
<div>Bug: <a
href="https://bugs.openjdk.java.net/browse/JDK-8228998"
target="_blank" \
moz-do-not-send="true">https://bugs.openjdk.java.net/browse/JDK-8228998</a></div> \
<div><br> </div>
<div>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.
<div dir="ltr"
class="gmail-m_-4465974035326398045gmail-m_6604593012462213921gmail_signature">
<div dir="ltr">
<div><br>
</div>
<div>This was tested by running the tests
changed on my dev machine, I'll push it
to the submit repo after review :-)</div>
<div><br>
</div>
Thanks and have a great evening,
<div>Jc</div>
</div>
</div>
</div>
</div>
</blockquote>
<br>
</div>
</blockquote>
</div>
<br clear="all">
<div><br>
</div>
-- <br>
<div dir="ltr"
class="gmail-m_-4465974035326398045gmail_signature">
<div dir="ltr">
<div><br>
</div>
Thanks,
<div>Jc</div>
</div>
</div>
</blockquote>
<br>
</div>
</blockquote>
</div>
<br clear="all">
<div><br>
</div>
-- <br>
<div dir="ltr" class="gmail_signature">
<div dir="ltr">
<div><br>
</div>
Thanks,
<div>Jc</div>
</div>
</div>
</blockquote>
<p><br>
</p>
</body>
</html>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic