RFR: 8165600: Convert internal logging tests to GTest

[Robbin Ehn]

Looks good, thanks!
/Robbin

On 09/08/2016 04:00 PM, Marcus Larsson wrote:
> Hi again,
>
> Patch has been split up into smaller parts, see separate email threads. Reusing this issue for the last few conversions.
>
> Updated webrev:
> http://cr.openjdk.java.net/~mlarsson/8165600/webrev.01/
>
> Thanks,
> Marcus
>
>
> On 09/08/2016 02:23 PM, Marcus Larsson wrote:
>> Hi Kirill,
>>
>> Thanks for looking at this.
>>
>>
>> On 09/08/2016 02:02 PM, Kirill Zhaldybin wrote:
>>> Marcus,
>>>
>>> Thank you for taking responsibility to convert logging tests to GTest!
>>>
>>> The change you made is pretty large. Would you consider to separate it to several smaller ones?
>>> It would make review much simpler.
>>
>> Will do.
>>
>>>
>>> A couple of fast comments:
>>> 1. Naming: I think we have a naming convention to name tests <Tested Class>, <scenarion_in_snake_case>.
>>
>> Oh right, I've forgotten to rename them properly. Will fix!
>>
>>> 2. You used EXPECT_GT in TEST(logging, LogTagLevelExpression_combination_limit). As far as understand EXPECT_* checks do not abort current routine but I am unsure why
>>> you used EXPECT_GT in the last line of test.  I also see EXPECT_* as last line in several other places. Could you please let me know why you used this macro not ASSERT_*?
>>
>> It's just a habit. I've preferred to use EXPECT_* except in those cases it makes no sense. Changing from EXPECT_ to ASSERT_ just because it's the last assertion in a test
>> seems pointless to me, and it would cause more noise when/if we add more assertions to a test in follow up patches.
>>
>> I'll post updated webrevs soon.
>>
>> Thanks,
>> Marcus
>>
>>>
>>> Thank you.
>>>
>>> Regards, Kirill
>>>
>>>
>>> On 08.09.2016 10:47, Marcus Larsson wrote:
>>>> Hi,
>>>>
>>>> Please review the following patch to convert existing internal VM tests for UL to the GTest framework.
>>>>
>>>> Summary:
>>>> Most of it is just a simple conversion from assert() to ASSERT_*() macros. Tests which log to file, or otherwise change the log configuration, have been modified to use
>>>> the LogTestFixture for automated log file handling and configuration cleanup.
>>>>
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~mlarsson/8165600/webrev.00/
>>>>
>>>> Issue:
>>>> https://bugs.openjdk.java.net/browse/JDK-8165600
>>>>
>>>> Testing:
>>>> JPRT
>>>>
>>>> Thanks,
>>>> Marcus
>>>
>>
>