RFR(S): 8165433: Convert Test_linked_list to Gtest

[David Holmes]

Hi Kirill,

On 7/09/2016 9:22 PM, Kirill Zhaldybin wrote:
> David,
>
> On 07.09.2016 08:52, David Holmes wrote:
>> Hi Kirill,
>>
>> On 7/09/2016 5:20 AM, Kirill Zhaldybin wrote:
>>> David,
>>>
>>> Thank you for reviewing the test!
>>>
>>> On 06.09.2016 02:47, David Holmes wrote:
>>>> Hi Kirill,
>>>>
>>>> Generally seems okay but:
>>>>
>>>> - why is there no include for allocation.hpp or else
>>>> allocation.inline.hpp when it is obviously used in the cpp file
>>> "utilities/linkedlist.hpp" is included and it includes
>>> "memory/allocation.hpp"
>>> Do you think I need to include allocation.hpp in the test too?
>>
>> No that's okay - thanks. But minor nit:
>>
>>   26 #include "utilities/linkedlist.hpp"
>>   27 #include "unittest.hpp"
>>
>> this order should be reversed for alphabetical include ordering, which
>> we try to adhere to.
> Fixed in a new WebRev:
> http://cr.openjdk.java.net/~kzhaldyb/webrevs/JDK-8165433/webrev.02/

Update looks fine.

>>
>>>>
>>>> - why do you not use NULL when checking pointers eg::
>>>>
>>>>  73   ASSERT_NE(i, (Integer*) 0) << "Should find it";
>>>>
>>>> Explicitly casting 0 to a pointer is not good form IIRC.
>>> Fixed in new WebRev:
>>> http://cr.openjdk.java.net/~kzhaldyb/webrevs/JDK-8165433/webrev.01/
>>
>> You should not have to cast NULL:
>>
>>  77   ASSERT_EQ(i, (Integer*) NULL) << "Not in the list";
>>
>> why is this needed? Is this something broken in gtest ??
> There is no ASSERT_NULL (we likely should add it) in GTest so ASSERT_EQ
> is used.
> ASSERT_EQ is pretty strict about types so since NULL resolves to long
> int (at least on my host) it cannot be compared with Integer*.

I'm unhappy about this, it is a deficiency in Gtest. We should not be 
casting  0 or NULL to specific pointer types.

cc'ing Kim to get his opinion on the best way to handle this.

Thanks,
David

> Thank you.
>
> Regards, Kirill
>>
>> Thanks,
>> David
>>
>>> Thank you.
>>>
>>> Regards, Kirill
>>>
>>>>
>>>> Otherwise conversion seems fine.
>>>>
>>>> Thanks,
>>>> David
>>>>
>>>>> Thank you.
>>>>>
>>>>> Regards, Kirill
>>>
>