<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<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>
</body>
</html>