<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
Do I need another review or review of Bengt is enough?<br>
<br>
Thanks.<br>
<br>
<div class="moz-cite-prefix">16.02.2015 22:41, Bengt Rutisson пишет:<br>
</div>
<blockquote cite="mid:54E247D5.6020106@oracle.com" type="cite">
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
<div class="moz-cite-prefix"><br>
Hi again,<br>
<br>
On 16/02/15 17:07, Andrey Zakharov wrote:<br>
</div>
<blockquote cite="mid:54E215CD.8010704@oracle.com" type="cite">
<meta content="text/html; charset=utf-8"
http-equiv="Content-Type">
<br>
<div class="moz-cite-prefix">16.02.2015 18:41, Bengt Rutisson
пишет:<br>
</div>
<blockquote cite="mid:54E20FA8.4000302@oracle.com" type="cite">
<meta content="text/html; charset=utf-8"
http-equiv="Content-Type">
<div class="moz-cite-prefix"><br>
Hi Andrey,<br>
<br>
On 16/02/15 16:24, Andrey Zakharov wrote:<br>
</div>
<blockquote cite="mid:54E20B96.90509@oracle.com" type="cite">
<meta http-equiv="content-type" content="text/html;
charset=utf-8">
Hi<br>
<blockquote type="cite">
<pre>But JDK-8019361 looks like a wrong number. It must be JDK-8051984, I think.</pre>
</blockquote>
<br>
I'm wrongly got reason of ignore bug for RFR. Thanks, Dima.<br>
<br>
webrev:<br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Eazakharov/8051984/webrev/">http://cr.openjdk.java.net/~azakharov/8051984/webrev/</a><i><br>
<br>
</i>bug:<br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="https://bugs.openjdk.java.net/browse/JDK-8051984">https://bugs.openjdk.java.net/browse/JDK-8051984</a><br>
<br>
<pre><blockquote type="cite"><pre>If so, what about other sources listed in the description:
./test/gc/arguments/TestParallelHeapSizeFlags.java
./test/gc/arguments/TestUseCompressedOopsErgo.java
./test/gc/g1/TestHumongousShrinkHeap.java</pre></blockquote></pre>
Its already fixed either by removing @ignore either by
inserting @requires<br>
<i><br>
</i>
<blockquote type="cite">
<pre>Why did you change the static import of
com.oracle.java.testlibrary.Asserts? Seems unrelated to the @ignore
change and I don't think there is a reason for it either. We use static
import of the asserts a lot in our test code.
Thanks,
Bengt
><i>
</i>><i> Thanks.</i></pre>
</blockquote>
<br>
There is nothing especial in import static here, its only
serves to reduce Asserts package names in code, but it also
leads to less readability and question like "what
assertLessThan comes from?".<br>
Asserts.assertLessThan is better - it doesn't junk global
namespace. In only this case - its only code style question.
If you have any other conserns about this, please tell me.<br>
</blockquote>
<br>
For people reading the hotspots tests a lot I think
assertLessThan() is more readable than
Asserts.assertLessThan(). Please don't change this for this
test only. If you feel strongly about this issue I think you
should bring it up for a wider discussion so we can agree on a
guideline for how to use it. Right now it just seems like a
completely unrelated change that is not necessary to solve
your bug.<br>
</blockquote>
Ok. At least I must change Asserts.* to Asserts.assertLessThan;<br>
</blockquote>
<br>
I still find it odd to do this as part of this change. But ok.<br>
<br>
<blockquote cite="mid:54E215CD.8010704@oracle.com" type="cite"> <br>
webrev:<br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Eazakharov/8051984/webrev.02/">http://cr.openjdk.java.net/~azakharov/8051984/webrev.02/</a><br>
</blockquote>
<br>
Looks ok. Reviewed.<br>
<br>
<blockquote cite="mid:54E215CD.8010704@oracle.com" type="cite"> <br>
Just for information:<br>
in src/hs-gc/hotspot/test<br>
<br>
<tt>grep 'import com.oracle.java.testlibrary.Asserts.' -R ./
--exclude-dir=.hg | wc -l</tt><tt><br>
</tt><tt>56</tt><tt><br>
</tt><tt><br>
</tt><tt>grep 'import static
com.oracle.java.testlibrary.Asserts.' -R ./ --exclude-dir=.hg
| wc -l</tt><tt><br>
</tt><tt>66</tt><tt><br>
</tt><tt><br>
</tt><tt>grep 'import static
com.oracle.java.testlibrary.Asserts.' -R ./gc
--exclude-dir=.hg | wc -l</tt><tt><br>
</tt><tt>17</tt><tt><br>
</tt><tt><br>
</tt><tt>grep 'import com.oracle.java.testlibrary.Asserts.' -R
./gc --exclude-dir=.hg | wc -l</tt><tt><br>
</tt><tt>5</tt><tt><br>
</tt></blockquote>
<br>
Right, so static import is the more common way for importing the
asserts.<br>
<br>
There are also the JFR tests for hotspot:<br>
<br>
$ grep -r 'import static jrockit.Asserts'
jdk/test/closed/com/oracle/jfr/gc/ | wc -l<br>
36<br>
<br>
$ grep -r 'import jrockit.Asserts'
jdk/test/closed/com/oracle/jfr/gc/ | wc -l<br>
3<br>
<br>
Thanks,<br>
Bengt<br>
<br>
<blockquote cite="mid:54E215CD.8010704@oracle.com" type="cite"><tt>
</tt><tt><br>
</tt>Thanks.<br>
<br>
<blockquote cite="mid:54E20FA8.4000302@oracle.com" type="cite">
<br>
Thanks,<br>
Bengt<br>
<br>
<blockquote cite="mid:54E20B96.90509@oracle.com" type="cite">
<br>
<br>
Thanks.<br>
<br>
<br>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</body>
</html>