RFR: JDK-8157453 - Convert DependencyContext_test to GTest

[Jesper Wilhelmsson]

Den 30/8/16 kl. 14:04, skrev Kirill Zhaldybin:
> Jesper,
>
> As far as I recall one of gtest's avantages was a way to test both product and
> debug VM - before we could test only debug.
> So why do not we make is_dependent_nmethod and find_stale_entries available in
> product?
> This will increase our coverage (we could test product too).
> Both methods also do not change anything and were used from the internal test
> only so I think risks are very low.

Yes, the risk is clearly low and I could absolutely do that. I would however 
prefer if someone who knows and can take responsibility for the code took that 
decision. As it is now we have the same test coverage as we had before.
/Jesper

>
> Thank you.
>
> Regards, Kirill
>
> On 30.08.2016 11:38, 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
>>
>> 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.
>>
>> 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
>>>>
>