RFR (M) : 8024319 : Add regression tests on documented GC-related -XX:+Print* options

[Filipp Zhinkin]

I've added comments with examples to tests and moved TestPrintGCTools to 
testlibrary.

Updated webrev: http://cr.openjdk.java.net/~kshefov/8024319/webrev.01/

Thanks,
Filipp.

On 10/07/2013 07:14 PM, Filipp Zhinkin wrote:
> Mikael,
>
> thanks for looking at it!
>
> On 10/07/2013 06:25 PM, Mikael Gerdin wrote:
>> Filipp,
>>
>> On 10/03/2013 12:50 PM, Filipp Zhinkin wrote:
>>> Hi,
>>>
>>> may I get one more review for this change?
>>
>> Sorry for taking so long to get to this.
>>
>> Please don't add @author-tags to the tests, hg stores that 
>> information fine.
> Yes, sure.
>>
>> TestPrintGCTools seems like it should be somewhere in the 
>> /testlibrary folder since it's not a test and just a utility class 
>> for testing gc log output.
> I agree with you.
> There is a one another RFR on tests for heap ratio options, I'll made 
> applicable changes there as well.
>> test/gc/arguments/print/TestCMSPrintGCFlags.java:
>> DefNew + CMS is deprecated in JDK 8. Do we still want a test for that?
> I can't see nothing bad in it, because on the one hand it does not 
> require any specific test cases
> and on the other hand it verifies that it will work for someone who 
> decided to use DefNew + CMS despite the deprecation.
>>
>> test/gc/arguments/print/TestG1PrintGCFlags.java:
>> I think it would be useful to have an example of a matching record 
>> for this test.
> Yep, it does not looks obvious. I'll add a comment.
>>
>> test/gc/arguments/print/TestPrintGCAppTimeFlags.java:
>> Just pass the flags as strings to the function instead of passing 
>> booleans.
>>  48         //-XX:-PrintGCApplicationConcurrentTime 
>> -XX:-PrintGCApplicationStoppedTime
>>   49         testGCAppTime(false, false, options);
>>   50         //-XX:-PrintGCApplicationConcurrentTime 
>> -XX:+PrintGCApplicationStoppedTime
>>   51         testGCAppTime(false, true,  options);
> It is there to simplify results parsing: "options" may already contain 
> PrintGCApplication*Time options,
> so to make a decision whether or not appropriate strings are expected 
> in test output I'll have to check
> not just occurance of -XX:[+-]PrintGCApplication*Time, but find all 
> PrintGCApplication*Time entries and
> check the sign before the last one.
> There is nothing hard in such check, but passing booleans looks much 
> simpler.
>>
>> test/gc/arguments/print/TestPrintGCTaskTimeStampsFlag.java:
>> Add some sample VM output in a comment.
>> "Simple sanity check for PrintGCTaskTimeStamps entries." simple? A 
>> 150 line parser class isn't exactly a simple sanity check :)
> Well, maybe it was a mistake to name it simple. :) I'll add an example.
>>
>> test/gc/arguments/print/TestPrintTenuringDistributionFlag.java:
>> Why do you add +UseAdaptiveSizePolicy? It's only supported (and 
>> enabled by default) in +UseParalell{Old}GC
> This test designed to work with any options passed to it, so 
> +UseAdaptiveSizePolicy was added for the case when
> options contains -XX:-UseAdaptiveSizePolicy to make sure that Parallel 
> GC will calculate new survivor size after GC
> and print tenuring distribution.
>>
>> test/gc/arguments/print/TestSerialAndParallelPrintGCFlags.java:
>> ParNew + Serial is deprecated in JDK 8. Do we still want a test for 
>> that?
> The same logic as with 
> test/gc/arguments/print/TestCMSPrintGCFlags.java applies here too.
>
> Thanks,
> Filipp.
>>
>> /Mikael
>>
>>
>>>
>>> Thanks,
>>> Filipp.
>>>
>>> On 09/18/2013 01:16 AM, Jesper Wilhelmsson wrote:
>>>> Looks good!
>>>> I ran the tests and they passed. I introduced a bug in the verbose
>>>> logging and the relevant test failed. Awesome!
>>>>
>>>> /Jesper
>>>>
>>>> Filipp Zhinkin skrev 17/9/13 6:14 PM:
>>>>> I'll be very appreciated if someone will take a look at it.
>>>>>
>>>>> Thanks,
>>>>> Filipp.
>>>>>
>>>>> On 09/10/2013 01:15 PM, Filipp Zhinkin wrote:
>>>>>> Hi,
>>>>>>
>>>>>> I'd like to get a review on new regression tests on various 
>>>>>> GC-related
>>>>>> -XX:+Print* options:
>>>>>>
>>>>>> http://bugs.sun.com/view_bug.do?bug_id=8024319
>>>>>>
>>>>>> webrev:
>>>>>> http://cr.openjdk.java.net/~kshefov/8024319/webrev.00/
>>>>>>
>>>>>> Brief overview of tests to be added:
>>>>>> Following tests verify that PrintGC and PrintGCDetails flags work
>>>>>> and emit
>>>>>> appropriate output.
>>>>>> Tests grouped by GC in order to handle different GC logging format.
>>>>>> gc/arguments/print/TestCMSPrintGCFlags
>>>>>> gc/arguments/print/TestG1PrintGCFlags.java
>>>>>> gc/arguments/print/TestSerialAndParallelPrintGCFlags
>>>>>>
>>>>>> Test on PrintGCTimeStamps and PrintGCDateStamps flags. It 
>>>>>> verifies that
>>>>>> options and their
>>>>>> combinations emit appropriate output and verifies that time and date
>>>>>> stamps
>>>>>> are not decreasing over time.
>>>>>> gc/arguments/print/TestGCStampFlags
>>>>>>
>>>>>> Test on PrintGCApplicationConcurrentTime and
>>>>>> PrintGCApplicationStoppedTime.
>>>>>> It verifies that these options emit appropriate output.
>>>>>> gc/arguments/print/TestPrintGCAppTimeFlags
>>>>>>
>>>>>> Test on PrintGCTaskTimeStamps flah. It verifies that when this
>>>>>> option emit
>>>>>> output only with parallel GC
>>>>>> and also verifies that all timestamps are not decreasing and for
>>>>>> each GC
>>>>>> thread verifies that number of
>>>>>> reported task timestamps equal to expected number of tasks.
>>>>>> gc/arguments/print/TestPrintGCTaskTimeStampsFlag
>>>>>>
>>>>>> Test on PrintTenuringDistribution flag. It verifies that appropriate
>>>>>> output is
>>>>>> emitted when this option is
>>>>>> turned on and also verifies that reported values lies between 0 and
>>>>>> selected
>>>>>> MaxTenuringThreshold.
>>>>>> gc/arguments/print/TestPrintTenuringDistributionFlag*
>>>>>>
>>>>>> *Testing done: manual testing and JPRT.
>>>>>>
>>>>>> Thanks,
>>>>>> Filipp.
>>>>>>
>>>>>>
>>>>>
>>>
>>
>