RFR: JDK-8157453 - Convert DependencyContext_test to GTest

[David Holmes]

On 30/08/2016 6:38 PM, Jesper Wilhelmsson wrote:
> Hi,
>
> Good catch Kirill!
>
> Den 30/8/16 kl. 06:26, skrev David Holmes:
>> Hi Kirill,
>>
>> On 30/08/2016 5:32 AM, Kirill Zhaldybin wrote:
>>> Jesper,
>>>
>>> As far as I understand test_dependencyContext could be run in both
>>> product and debug.
>>> But in the class DependencyContext
>>>
>>>  148 #ifndef PRODUCT
>>>  149   void print_dependent_nmethods(bool verbose);
>>>  150   bool is_dependent_nmethod(nmethod* nm);
>>>  151   bool find_stale_entries();
>>>  152 #endif //PRODUCT
>>>
>>> so is_dependent_nmethod and find_stale_entries are compiled only in
>>> non-product configs but they are used in test_dependencyContext
>>
>> I presume this is why these calls are wrapped in the ASSERT_TRUE and
>> ASSERT_FALSE macros - I expect them to be no-ops in a product build.
>> But I can't
>> find the definitions of these ASSERT_xxx macros ??
>
> No, Kirill is right, this won't compile in product. I completely forgot
> that these tests are run in product mode these days (they weren't when I
> did this test conversion a while ago).
>
> ASSERT_TRUE and ASSERT_FALSE are part of the GTest framework. They are
> two of the handful of ASSERT_X macros that we can use in the tests.
> These are defined in /test/fmw/gtest/include/gtest/gtest.h

Not familiar with Gtest itself but this seems like a design flaw to me - 
the GTest macros should be obviously part of GTest! :(

> A new webrev that I have verified with product is found here:
>
> http://cr.openjdk.java.net/~jwilhelm/8157453/webrev.01/
>
> The test will still do some things with dependency contexts in product
> mode but it won't verify anything. I chose not to use ifndef PRODUCT
> around the entire test to highlight what parts need to be fixed if the
> test should do some verification in product as well. I leave it up to
> the team who owns the DependencyContext code to decide if that is
> important or not.

I'd probably prefer #ifndef PRODUCT blocks instead of multiple lines of 
NOT_PRODUCT, but won't insist.

Thanks,
David

>
> Thanks,
> /Jesper
>
>
>>
>> David
>> -----
>>
>>
>>
>>
>>
>>
>>> Have you tried to build VM and run this test in product config?
>>>
>>> http://cr.openjdk.java.net/~jwilhelm/8157453/webrev.00/test/native/code/test_dependencyContext.cpp.html
>>>
>>>
>>>
>>>
>>>   86 TEST(code, dependency_context) {
>>>
>>> According to "almost existing naming convention" the test should be
>>> named (<Tested Class>, <Test Case>).
>>>
>>> Thank you.
>>>
>>> Regards, Kirill
>>>
>>> On 25.08.2016 20:53, Jesper Wilhelmsson wrote:
>>>> Hi,
>>>>
>>>> Please review this test conversion of the DependencyContext tests to
>>>> GTest.
>>>>
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8157453
>>>> Webrev: http://cr.openjdk.java.net/~jwilhelm/8157453/webrev.00/
>>>>
>>>> Thanks,
>>>> /Jesper
>>>