RFR: 8065749: [TESTBUG]: gc/arguments/TestG1HeapRegionSize.java fails at nightly

[Dmitry Fazunenko]

Hi Bengt,


On 24.11.2014 18:33, Bengt Rutisson wrote:
>
> Hi Evgeniya,
>
> On 2014-11-24 14:13, Evgeniya Stepanova wrote:
>> Hi,
>>
>> Could you please review changes for 8065749 ?
>> Test TestG1HeapRegionSize.java is G1-specific and could not work 
>> without -XX:+UseG1GC passed directly.
>> Option added to the run.
>> bug: https://bugs.openjdk.java.net/browse/JDK-8065749
>> fix: http://cr.openjdk.java.net/~eistepan/8065749/webrev.00/
>
> I think your suggested fix will work, but I have a question regarding 
> the original change.
>
> Before the original change the test explicitly checked that it was run 
> with G1 and just skipped otherwise:
>
> http://hg.openjdk.java.net/jdk9/hs-gc/hotspot/comparison/6464714dd742/test/gc/arguments/TestG1HeapRegionSize.java
>
> Was this test really failing at all? I thought we were only going to 
> add @requires to tests that were failing due to conflicting GC 
> combinations. If it was working before I think a different approach to 
> fix the current problem would be to revert back to the old code 
> without @requires.

You are right, it was our mistake. This test wasn't reverted when we 
decided to update only failing tests.
Yes, a possible solution would be just to roll back the previous change, 
but I don't think it makes sense just to roll back.

What I suggest:
1) keep this test G1 specific (@requires g1 and +UseG1 explicitly) - 
what is proposed by this change
2) make tests GC independent - remove @requires and check in tests that 
RegionSize == <expected> in case of G1, and RegionSize == 0 in case of 
another GC.

I don't think that original variant (silently pass if not g1) is the 
right solution.

What do you think?

-- Dima





>
> Thanks,
> Bengt
>
>>
>> Thanks,
>> Evgeniya Stepanova
>>
>> -- 
>> /Evgeniya Stepanova/
>>
>>
>

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