RFR(S): 8165433: Convert Test_linked_list to Gtest

[Kirill Zhaldybin]

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/
>
>>>
>>> - 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*.

Thank you.

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