RFR: 8165600: Convert internal logging tests to GTest

[Marcus Larsson]

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