RFR: JDK-8156800 - Convert QuickSort_test to GTest

Jesper Wilhelmsson jesper.wilhelmsson at oracle.com
Wed Oct 19 14:21:05 UTC 2016


Thanks Marcus!

I'll sort and remove before pushing.
/Jesper

Den 19/10/16 kl. 16:17, skrev Marcus Larsson:
> Hi Jesper,
>
>
> On 10/19/2016 12:30 PM, Jesper Wilhelmsson wrote:
>> Hi,
>>
>> I managed to mix up the bugIDs and sent an update to this webrev to the TestOS
>> GTest conversion in this thread:
>>
>> http://mail.openjdk.java.net/pipermail/hotspot-dev/2016-October/024911.html
>>
>> Inlining the mail below for clarity:
>>
>> -----------------------------------------------------------
>> Kirill gave me some feedback offline and this is an updated version of the test.
>>
>> It's using EXPECT instead of ASSERT to allow all test cases to be executed even
>> in the presence of failures and the test cases has been renamed.
>>
>> I also updated the copyright date since I learned that the copyright should
>> follow the code, not the file, and we are moving older code into a new file.
>>
>> New webrev: http://cr.openjdk.java.net/~jwilhelm/8156800/webrev.01/
>
> The #includes in test_quicksort.cpp need to be sorted. Also, you should remove
> the include for allocation.hpp, since you include the .inline.hpp.
>
> Otherwise, this looks good to me!
>
> Thanks,
> Marcus
>
>> -----------------------------------------------------------
>>
>> (I have moved the webrev to the right place and updated the link i the mail
>> above.)
>>
>> Rachel replied and suggested to add parentheses in lines 57 and 58 to ease
>> visual scanning. I have added that in my local change.
>>
>>  57   bool a_is_odd = ((a % 2) == 1);
>>  58   bool b_is_odd = ((b % 2) == 1);
>>
>>
>> I now have three reviewer approvals for this change but no Reviewer. Could
>> someone please have a look?
>>
>> Thanks,
>> /Jesper
>>
>>
>> Den 25/8/16 kl. 18:55, skrev Jesper Wilhelmsson:
>>> Hi,
>>>
>>> Please review this test conversion of the quicksort tests to GTest.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8156800
>>> Webrev: http://cr.openjdk.java.net/~jwilhelm/8156800/webrev.00/
>>>
>>> Thanks,
>>> /Jesper
>


More information about the hotspot-dev mailing list