RFR(S): 8165433: Convert Test_linked_list to Gtest

[David Holmes]

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.

>>
>> - 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 ??

Thanks,
David

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