RFR: 8165600: Convert internal logging tests to GTest

Kirill Zhaldybin kirill.zhaldybin at oracle.com
Thu Sep 8 12:02:45 UTC 2016


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.

A couple of fast comments:
1. Naming: I think we have a naming convention to name tests <Tested 
Class>, <scenarion_in_snake_case>.
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_*?

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:
> Thanks,
> Marcus

More information about the hotspot-runtime-dev mailing list