RFR(S): 8165433: Convert Test_linked_list to Gtest
david.holmes at oracle.com
Wed Sep 7 05:52:17 UTC 2016
On 7/09/2016 5:20 AM, Kirill Zhaldybin wrote:
> 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
> 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:
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 ??
> Thank you.
> Regards, Kirill
>> Otherwise conversion seems fine.
>>> Thank you.
>>> Regards, Kirill
More information about the hotspot-runtime-dev