Request for review (S): 8006398: Add regression tests for deprectated GCs

[Bengt Rutisson]

Thanks for the reviews Erik, Jesper and Jon!

Pushed this change.

Bengt


On 2/15/13 12:51 PM, Bengt Rutisson wrote:
>
> On 2/14/13 9:06 PM, Jon Masamitsu wrote:
>>
>> On 2/14/2013 6:32 AM, Bengt Rutisson wrote:
>>>
>>> Hi Jon,
>>>
>>> Are you ok with me pushing this? We can always add more tests later.
>>
>> Your call.  I myself would do it now so I don't have to think about it
>> again but that's because I so easily forget about things :-)
>
> OK. My memory is probably not that good either. :) I added the test 
> TestCMSNoIncrementalMode.java:
>
> http://cr.openjdk.java.net/~brutisso/8006398/webrev.02/
>
> I also changed the folder name from gc/start-warnings to 
> gc/start_warnings since a "-" in the name makes it difficult to filter 
> these tests out in JPRT. The -rtests option thinks that "-" is a 
> separator for the platform definitions.
>
> Thanks,
> Bengt
>
>>
>> Ship it.
>>
>> Jon
>>>
>>> Thanks,
>>> Bengt
>>>
>>> On 2/13/13 9:11 AM, Bengt Rutisson wrote:
>>>>
>>>> Hi John,
>>>>
>>>> Thanks for looking at this!
>>>>
>>>> On 2/13/13 3:40 AM, Jon Masamitsu wrote:
>>>>> Bengt,
>>>>>
>>>>> Changes look good.
>>>>>
>>>>> You include tests for UseG1GC and UseParallelGC (should
>>>>> not print warnings) but not UseConcMarkSweepGC nor
>>>>> UseSerialGC.  Why the former but not the latter?
>>>>
>>>> The TestSerialGC.java is running with UseSerialGC. Am I missing 
>>>> something or is this the test you meant?
>>>>
>>>> For CMS I think you have a point. I was kind of thinking that the 
>>>> test TestParNewCMS was anyway testing that GC combination, but we 
>>>> should really have a separate CMS test as well. I just added 
>>>> TestCMS.java to test UseConcMarkSweepGC explicitly. Here is an 
>>>> updated webrev:
>>>>
>>>> http://cr.openjdk.java.net/~brutisso/8006398/webrev.01/
>>>>
>>>>>
>>>>> Do you want to test
>>>>>
>>>>> -XX:+UseConcMarkSweepGC -XX:-CMSIncrementalMode -version
>>>>>
>>>>> to check that no warning is emitted?
>>>>
>>>> Hm. I don't know. If we start combining all the flags we will get 
>>>> into an explosion of tests. I think this is a valid combination to 
>>>> test. But in that case so are things like "-XX:+UseConcMarkSweepGC 
>>>> -XX:-UseSerialGC". I like the way you think about this testing, I 
>>>> just think I would like to keep these tests to be smoke testing the 
>>>> more commonly used combinations.
>>>>
>>>> Make sense?
>>>>
>>>> Thanks again for looking at this!
>>>> Bengt
>>>>
>>>>>
>>>>> Jon
>>>>>
>>>>> On 2/12/2013 4:56 AM, Bengt Rutisson wrote:
>>>>>>
>>>>>> Hi again,
>>>>>>
>>>>>> Christian has pushed his test library change and it has 
>>>>>> propagated to the hotspot-gc repository. So, this change is ready 
>>>>>> to be pushed. Just need some reviews. Any takers? :)
>>>>>>
>>>>>> Thanks,
>>>>>> Bengt
>>>>>>
>>>>>> On 1/16/13 1:58 PM, Bengt Rutisson wrote:
>>>>>>>
>>>>>>> Hi all,
>>>>>>>
>>>>>>> Could I have a couple of reviews for this change?
>>>>>>> http://cr.openjdk.java.net/~brutisso/8006398/webrev.00/
>>>>>>>
>>>>>>> Recently we deprecated some GC combinations. Those should now 
>>>>>>> print a warning at startup. Other GC combinations should not 
>>>>>>> print any warnings.
>>>>>>>
>>>>>>> With the new process handling support that Christian Törnqvist 
>>>>>>> is adding to the JTREG tests for hotspot it is very easy to 
>>>>>>> write test that start a VM and checks the output.
>>>>>>>
>>>>>>> This changes makes use of Christian's testlibrary to verify that 
>>>>>>> warnings are printed as expected.
>>>>>>>
>>>>>>> I'm also adding the "gc" keyword to JTREG to make it possible to 
>>>>>>> filter out GC tests. We should probably use this for all test in 
>>>>>>> the the /gc folder, but I think that should be done as a 
>>>>>>> separate change.
>>>>>>>
>>>>>>> The webrev above is based on Christian's webrev to add the 
>>>>>>> testlibrary:
>>>>>>> http://cr.openjdk.java.net/~brutisso/8006413/webrev.00/
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Bengt
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>