RFR: tests for JDK-8028720 Make the Min/MaxHeapFreeRatio flags manageable

[Andrey Zakharov]

Hi
Thanks for comments.
I have completely rework this tests according to comments.
Also we have found that it will be easier to follow standard work-flow 
of submitting patches to branch 9 and then back-porting it to 8 and 7.
So, here is webrev against http://hg.openjdk.java.net/jdk9/hs-gc/hotspot
I hope, Jesper will help me to upload it right place.

Thanks.


On 05.03.2014 23:35, Lev wrote:
> Igor, you are right. You could check it in 
> proposal: http://openjdk.java.net/jeps/161
>
> Best regards,
> Lev
>
>
> -------- Original message --------
> From: Igor Ignatyev
> Date:05/03/2014 13:12 (GMT+04:00)
> To: Andrey Zakharov ,Lev Priima
> Cc: hotspot-gc-dev at openjdk.java.net
> Subject: Re: RFR: tests for JDK-8028720 Make the Min/MaxHeapFreeRatio 
> flags manageable
>
> Hi Andrey,
>
> 0. In all your tests you have:
> >   27  * @key gc
> it's redundant, since the tests are already in gc directory. please
> remove it.
> >   28  * @bug JDK-8028391
> according to convention, you have to use '8028391' instead of 
> 'JDK-8028391'.
> $ grep -R "@bug" ./hotspot/test/ | wc -l
> 434
> $ grep -R "@bug" ./hotspot/test/ | grep -c JDK
> 0
>
> 1. VMOptionValueChecker has checkInvalidValue and checkValid, please
> normalize them:
>    checkInvalidValue/checkValidValue
>    or
>    checkInvalid/checkValid
>
> 2. I don't like that 'VMOptionValueChecker.getIntWritableValue' method
> do two things, could you please replace it w/ two methods:
> VMOptionValueChecker.getIntValue(name)
> VMOptionValueChecker.checkIsWriteable(name)
>
> 3. You use management packages, but not all JDK/JRE bundles have it, so
> you have to add your tests in some group (needs_compact3, I think). LevP
> should have more information on it.
>
> Lev, could you please help us here? com.sun.management.* and
> java.lang.management.* are available only in compact3, aren't they?
>
> 4. your tests look similar, can you create a super class for both tests?
> we will be able to reuse it in other tests for writable flags
>
> > class TestDynIntVMFlags  {
> > String name;
> > int originalValue;
> > TestDynIntVMFlags(name) {
> >   name = name;
> >   int originalValue = getIntValue(name)
> >   println(name + " = " + originalValue);
> > }
> >
> > void test() {
> >       checkIsWriteable(name);
> >       checkInvalidValue(name, "" + Integer.MIN_VALUE);
> >       checkInvalidValue(name, "" + Integer.MAX_VALUE);
> >       checkInvalidValue(name, "-10");
> >       checkInvalidValue(name, "190");
> >       checkValid(name, "0");
> >       extraTest();
> > }
> > void extraTest() { }
> > }
> > //// TestDynMinHeapFreeRatio.java
> > TestDynMinHeapFreeRatio extends TestDynIntVMFlags {
> > private TestDynMinHeapFreeRatio() { super("MinHeapFreeRatio"); }
> >   String MaxFreeRatioFlagName = "MaxHeapFreeRatio";
> >   void main() {
> >     new TestDynMinHeapFreeRatio().test();
> >   }
> >   void extraTest() {
> >     int maxHeapFreeValue = getIntValue(MaxFreeRatioFlagName);
> >     checkInvalidValue(name, "" + (maxHeapFreeValue + 1));
> >     checkValid(name, "" + (maxHeapFreeValue));
> >   }
> > }//// TestDynMaxHeapFreeRatio.java
> > TestDynMaxHeapFreeRatio extends TestDynIntVMFlags {
> > TestDynMaxHeapFreeRatio() { super("MaxHeapFreeRatio"); }
> >   String MaxFreeRatioFlagName = "MinHeapFreeRatio";
> >   void main() {
> >     new TestDynMaxHeapFreeRatio().test();
> >   }
> >   void extraTest() {
> >     int minHeapFreeValue = getIntValue(MinFreeRatioFlagName);
> >     checkInvalidValue(name, "" + (minHeapFreeValue - 1));
> >     checkValid(name, "" + (minHeapFreeValue));
> >   }
> > }
>
> Thanks,
> Igor
>
> On 02/20/2014 10:09 PM, Thomas Schatzl wrote:
> > Hi,
> >
> > On Thu, 2014-02-20 at 15:15 +0400, Andrey Zakharov wrote:
> >> Any comments are welcome!
> >> Thanks.
> >>
> >> On 14.02.2014 21:13, Jesper Wilhelmsson wrote:
> >>> Looks good.
> >>> /Jesper
> >>>
> >>> Andrey Zakharov skrev 14/2/14 16:33:
> >>>> Simple tests to check that MaxHeapFreeRatio and MinHeapFreeRatio
> >>>> flags are
> >>>> writable and managable in proper ways.
> >>>> webrev: http://cr.openjdk.java.net/~jwilhelm/8028391/webrev.test.02/
> >>>> bug: https://bugs.openjdk.java.net/browse/JDK-8028720
> >>>>
> >>>> Thanks.
> >>
> >
> >    some typos: "managable" -> "manageable"
> >
> > Otherwise looks good.
> >
> > Thomas
> >
> >

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20140307/4fe9e0d5/attachment.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 7607562.tar.gz
Type: application/gzip
Size: 10254 bytes
Desc: not available
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20140307/4fe9e0d5/7607562.tar.gz>