RFR: 8165600: Convert internal logging tests to GTest

[Marcus Larsson]

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
>