RFR: JDK-8156800 - Convert QuickSort_test to GTest

Marcus Larsson marcus.larsson at oracle.com
Wed Oct 19 14:17:13 UTC 2016


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