RFR: 8165600: Convert internal logging tests to GTest
marcus.larsson at oracle.com
Thu Sep 8 14:00:57 UTC 2016
Patch has been split up into smaller parts, see separate email threads.
Reusing this issue for the last few conversions.
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:
>> 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
> I'll post updated webrevs soon.
>> Thank you.
>> Regards, Kirill
>> On 08.09.2016 10:47, Marcus Larsson wrote:
>>> Please review the following patch to convert existing internal VM
>>> tests for UL to the GTest framework.
>>> 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.
More information about the hotspot-runtime-dev