RFR(S): 8164028: Convert TestPredictions_test to GTest

Kirill Zhaldybin kirill.zhaldybin at oracle.com
Mon Aug 22 15:58:20 UTC 2016


Erik,

Thank you for review!

Regards, Kirill

On 22.08.2016 18:13, Erik Helin wrote:
> On 2016-08-22, Kirill Zhaldybin wrote:
>> Erik,
>>
>> I deleted some comments, please do not hesitate me if you think I deleted
>> too much.Here are a new WebRev:
>>>> http://cr.openjdk.java.net/~kzhaldyb/webrevs/JDK-8164028/webrev.04/
>>>>
>>>> Could you please read comments inline?
>>>> - Looking at the unified diff (much easier now, thanks) it seems like
>>>>    the old code used <, which means that the new code must use ASSERT_GE,
>>>>    not ASSERT_GT.
>>>> Sorry, I did not find a line you mentioned. Could you please let me know the
>>>> line number?
>>> Just look at the unified diff:
>>> http://cr.openjdk.java.net/~kzhaldyb/webrevs/JDK-8164028/webrev.04/test/native/gc/g1/test_g1Predictions.cpp.udiff.html
>>>
>>> Everywhere you see a pattern similar to:
>>> -  assert(p2 < p1, "First prediction must be larger than second, but they are %f %f", p1, p2);
>>> +  ASSERT_GT(p1, p2) << "First prediction must be greater than second";
>>>
>>> you need to change ASSERT_GT to ASSERT_GE.
>> In my understanding assert (p2<p1) means the same as ASSERT_GT(p1, p2)
>> (p1>p2 <=> p2<p1)
>> Could you please correct me if I am wrong?
> No, you are right. Sorry, my bad. Looks good, Reviewed.
>
> Thanks,
> Erik
>
>> Thank you.
>>
>> Regards, Kirill
>>> Thanks,
>>> Eirk
>>>




More information about the hotspot-gc-dev mailing list