RFR (XS): 8078673: Update TEST.groups for recent GC tests

[Derek White]

On 7/12/15 10:38 PM, David Holmes wrote:
> Hi Derek,
>
> On 11/07/2015 1:28 AM, Derek White wrote:
>> Please review this partial fix for GC tests that require certain
>> collectors (e.g. shouldn't run in embedded).
>>
>> This is an updated webrev to account for Leonid's fix for "8079134:
>> [TESTBUG] Remove applicable_*gc and needs_*gc groups from TEST.groups",
>> which removed a pile of TEST.groups lists including needs_gc,
>> needs_serialgc, needs_parallelgc, and needs_cmsgc.
>>
>> Now the fix simply updates the needs_g1gc list in TEST.groups and adds
>> appropriate "@requires vm.gc" annotations to a few GC tests.
>>
>> Note that the "@requires vm.gc" changes are /almost/ purely documentary.
>> These are ProcessBuilder-based tests, so any "-XX:+UsexxxGC" flags
>> passed in by jtreg are ignored. It's very confusing, as well as
>> unnecessary, for a jtreg run specifying -XX:+UseG1GC to be running a
>> test that then replaces the flag with "-XX:+UseParallelGC" (etc).
>
> So even though we would never pass through the jtreg specific GC 
> option we will skip these tests if that option doesn't match with the 
> GC's the test will be testing. As long as that doesn't lead to these 
> tests being untested that seems okay.
In all of the tests I modified, it always runs the test if jtreg did not 
specify any GC. Most of the tests will also run if jtreg specifies the 
same GC as the test. So we should have test coverage.
>
> But really this highlights a basic problem we have with our approach 
> to testing with different VM options. Unless the GC option is the only 
> option that changes across the runs you would want the other options 
> to be passed through to the actual tests in many cases. :(

Some test do pass along the outer (jtreg) arguments, and some don't. So 
the fix with "@requires vm.gc" removes the possibility of conflicting 
GC's being specified in that case. There could still be conflicts 
between other arguments specified by jtreg vs. the ProcessBuilder. If 
the ProcessBuilder arguments were constant, they could also be specified 
by |"@requires vm.opt." annotations, but this leaves out many cases.

I'm also not satisfied by the current state of affairs, but I think this 
fix gets us slightly closer to what we want.

  - Derek

|
>
>> *CR:*
>> https://bugs.openjdk.java.net/browse/JDK-8078673
>>
>> *Webrev:*
>> http://cr.openjdk.java.net/~drwhite/8078673/webrev.03/
>>
>> *Testing:*
>>   JPRT
>>
>> *Open review comments:*
>> David H: Your last comments on this subject requested changes to the
>> "needs_gc" list, which has been removed by 8079134.
>>
>> Kim and Jesper: I agree with your comments about wanting some better for
>> both testing multiple GCs and dealing with SE Embedded properly in the
>> testing infrastructure. This webrev is simply a good fix within the
>> existing infrastructure.
>>
>> Thanks,
>>   - Derek
>>
>> On 4/29/15 5:03 PM, Kim Barrett wrote:
>>> On Apr 29, 2015, at 4:38 PM, Jesper 
>>> Wilhelmsson<jesper.wilhelmsson at oracle.com>  wrote:
>>>> Hi,
>>>>
>>>> There are tests like 
>>>> hotspot/test/gc/g1/TestShrinkAuxiliaryData**.java where there is a 
>>>> base class that provides the test and a bunch of test classes that 
>>>> only starts the base test with different arguments. This case would 
>>>> be similar but slightly more ugly since the actual code would be 
>>>> the same and trigger the same base test, but with different 
>>>> @requires in the comment above the test.
>>>>
>>>> I'm not sure it would help though. What we really would need here 
>>>> is a @requires that could check the host name or the hardware 
>>>> platform or OS.
>>> @requires has os.{name, family, arch, simpleArch} properties that 
>>> can be tested.  But I’m not sure any of those are really right for 
>>> testing for “embedded system” (whatever that actually means).
>>>
>>>> Kim Barrett skrev den 29/4/15 20:35:
>>>>> On Apr 29, 2015, at 2:06 PM, Derek White<derek.white at oracle.com>  
>>>>> wrote:
>>>>>> So most of these tests use ProcessBuilder to specify a command 
>>>>>> line, and explicitly mention a GC to use. A single test might 
>>>>>> actually run each collector (gc/logging/TestGCId, for example). 
>>>>>> Does @requires matter in this case?
>>>>>>
>>>>>> Oh, maybe they should all have @requires vm.gc=="null", becuase 
>>>>>> the actual test is ignoring GC passed in by the test harness GC 
>>>>>> and running as a separate process anyway. It's misleading if the 
>>>>>> harness said UseG1GC and the test was actually running 
>>>>>> UseParallelGC, for example.
>>>>> That’s one solution.  A different solution would be to clone into 
>>>>> multiple tests, one for each relevant collector, where the vm.gc 
>>>>> can be “null” or the corresponding collector.  That cloning is 
>>>>> kind of ugly though; it’s too bad there can only be one @requires 
>>>>> constraint per test, rather than per @run line or the like.  But 
>>>>> we didn’t get any traction with such a 
>>>>> suggestion:https://bugs.openjdk.java.net/browse/CODETOOLS-7901090.
>>>>>
>>>>>> FYI, it sounds like my original problem does require the exclude 
>>>>>> lists to keep the embedded JVM from running GC tests that it 
>>>>>> shouldn't.
>>>>> I’m not sure how to address this problem.  For example, we don’t 
>>>>> want to exclude TestSmallHeap.java on embedded JVMs, we just want 
>>>>> to exclude its sub-cases that would attempt to use a gc that isn’t 
>>>>> supported.
>>>
>>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20150713/17bbdaf7/attachment.htm>