RFR (XS): 8078673: Update TEST.groups for recent GC tests
Derek White
derek.white at oracle.com
Tue Feb 23 16:41:38 UTC 2016
[RFR back from the dead...]
I think last webrev for this was updated to follow the consensus
opinion, but it never received thumbs up/down from reviewers.
I've created a new webrev for critical copyright notice updates.
Summary:
1) Updated the "needs_g1gc" list in TEST.groups to prevent G1 tests
running in embedded.
2) Added appropriate "@requires vm.gc" annotations to a few GC tests
that use the process builder and pass in a collector explicitly. The new
@requires annotations prevent unexpected GC-specific tests running when
a conflicting collector argument is specified by the outer test harness.
CR:
https://bugs.openjdk.java.net/browse/JDK-8078673
Webrev:
http://cr.openjdk.java.net/~drwhite/8078673/webrev.04/
Testing:
JPRT
Thanks!
- Derek
On 7/13/15 5:37 PM, Derek White wrote:
> 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/20160223/e7b7207e/attachment.htm>
More information about the hotspot-gc-dev
mailing list