RFR 8059557: Test set for "Validate JVM Command-Line Flag Arguments"

Gerard Ziemski gerard.ziemski at oracle.com
Wed Jun 17 15:07:34 UTC 2015 

hi Dmitry,

Please consider this reviewed (with small "r").

Just one quick question: is there an issue filed to followup on why the 
test hangs with CICompiler and to enable it back, once we figure it out?


cheers

On 6/16/2015 10:58 AM, Dmitry Dmitriev wrote:
> Hello,
>
> Here a updated version of test set for Command Line Option Validation. 
> The main changes are: added new options types - int and uint, added 
> @modules, test all options with range in TestOptionsWithRanges.java test.
>
> Tested: JPRT
>
> Webrev: http://cr.openjdk.java.net/~ddmitriev/8059557/webrev.05/ 
> <http://cr.openjdk.java.net/%7Eddmitriev/8059557/webrev.05/>
> Webrev incremental(from 04): 
> http://cr.openjdk.java.net/~ddmitriev/8059557/webrev.05.incremental/ 
> <http://cr.openjdk.java.net/%7Eddmitriev/8059557/webrev.05.incremental/>
> JEP: https://bugs.openjdk.java.net/browse/JDK-8059557
>
> Thanks,
> Dmitry
>
> On 03.06.2015 16:47, Dmitry Dmitriev wrote:
>> Hello David,
>>
>> Thank you for pointing on style/grammar mistakes! I corrected all of 
>> them.
>> Here a link to the updated 
>> webrev:http://cr.openjdk.java.net/~ddmitriev/8059557/webrev.04/ 
>> <http://cr.openjdk.java.net/%7Eddmitriev/8059557/webrev.04/>
>>
>> About "options validation" package - I agree that this looks odd, but 
>> if you don't mind I prefer to leave it. It slightly simplify calling 
>> static methods by using static import and also I use package-private 
>> access level.
>>
>> Regards,
>> Dmitry
>>
>> On 02.06.2015 7:48, David Holmes wrote:
>>> Hi Dmitry,
>>>
>>> On 28/05/2015 11:32 PM, Dmitry Dmitriev wrote:
>>>> Hello all,
>>>>
>>>> Here is a 3 version of the tests taking into account feedback from
>>>> Christian, David and Gerard.
>>>>
>>>> I limit number of options in TestOptionsWithRanges.java to 15. This
>>>> include options of different types(intx, uintx etc.) and with 
>>>> different
>>>> combination of min/max range. TestOptionsWithRangesDynamic.java leaved
>>>> as is, because amount of manageable numeric options is very small and
>>>> currently only 3 of them have range. Also, I improve code for double
>>>> option.
>>>>
>>>> Webrev: http://cr.openjdk.java.net/~ddmitriev/8059557/webrev.03/
>>>> <http://cr.openjdk.java.net/%7Eddmitriev/8059557/webrev.03/>
>>>
>>> Only a few style/grammar nits (the downside of writing so many doc 
>>> comments :) ).
>>>
>>> Meta-question: is there a specific reason to use the package 
>>> "options validation"? It looks odd to me to have 
>>> OptionsValidation/common/optionsvalidation/ in the paths.
>>>
>>> General doc-comment comments:
>>> - @param/@return descriptions should start with lower-case (you 
>>> currently mix-n-match)
>>> - @throws description should start with "if", so:
>>>      @throws IOException if an error occurred reading the data
>>>
>>>
>>> General Java-style comments:
>>> - access modifiers (public, private, protected) always come first
>>>
>>> ---
>>>
>>> test/runtime/CommandLine/OptionsValidation/common/optionsvalidation/IntJVMOption.java 
>>>
>>>
>>> 265                  * passed value is negative then error will be 
>>> catched earlier for
>>>
>>> catched -> caught
>>>
>>> ---
>>>
>>> test/runtime/CommandLine/OptionsValidation/common/optionsvalidation/JVMOption.java 
>>>
>>>
>>> 327      * @param param Tested parameter passed to the java
>>>
>>> "the java" is not a noun - suggest "the JVM" ?
>>>
>>> Maybe change Java to JVM to avoid use of "java" as a noun everywhere.
>>>
>>>  350             failedMessage(name, fullOptionString, valid, "JVM 
>>> output reports about fatal error. JVM exit with code " + returnCode 
>>> + "!");
>>>
>>> The message isn't grammatically correct: about -> a; exit -> exited
>>>
>>> Similarly "JVM exit" -> "JVM exited"
>>>
>>>  377                 failedMessage(name, fullOptionString, valid, 
>>> "JVM output not contains "
>>>
>>> not contains -> does not contain
>>>
>>> 400      * Return list of strings which contain options with valid 
>>> values which used
>>>
>>> which used -> which can be used
>>>
>>>  416      * Return list of strings which contain options with 
>>> invalid values which
>>>  417      * used for testing on command line
>>>
>>> Ditto
>>>
>>> ---
>>>
>>> test/runtime/CommandLine/OptionsValidation/common/optionsvalidation/JVMOptionsUtils.java 
>>>
>>>
>>> 101      * Add dependency for option depending on it's type. E.g. 
>>> ran java in
>>>
>>> ran java -> run the JVM
>>>
>>> 119      * @param withRanges true if needed options with defined ranges
>>>
>>> I'm not sure what this means. Occurs elswhere too.
>>>
>>> 121      * "product", "diagnostic" etc. Accept option only if 
>>> acceptOrigin return
>>>
>>> return -> returns (or even return -> evaluates). Occurs elsewhere too.
>>>
>>> 260      * methods. Tested only writeable options.
>>>
>>> Suggestion: Only tests writeable options. Occurs elsewhere too.
>>>
>>> 264      * @throws Exception
>>>
>>> When? Why? Occurs elsewhere too.
>>>
>>> Thanks,
>>> David
>>> -----
>>>
>>>> Thanks,
>>>> Dmitry
>>>>
>>>>
>>>> On 21.05.2015 23:57, Dmitry Dmitriev wrote:
>>>>> Hello all,
>>>>>
>>>>> Recently I correct several typos, so here a new webrev for tests:
>>>>> http://cr.openjdk.java.net/~ddmitriev/8059557/webrev.02/
>>>>> <http://cr.openjdk.java.net/%7Eddmitriev/8059557/webrev.02/>
>>>>>
>>>>> Thanks,
>>>>> Dmitry
>>>>>
>>>>> On 18.05.2015 18:48, Dmitry Dmitriev wrote:
>>>>>> Hello all,
>>>>>>
>>>>>> Please review test set for verifying functionality implemented by 
>>>>>> JEP
>>>>>> 245 "Validate JVM Command-Line Flag Arguments"(JDK-8059557). Review
>>>>>> request for this JEP can be found there:
>>>>>> http://mail.openjdk.java.net/pipermail/hotspot-dev/2015-May/018539.html 
>>>>>>
>>>>>>
>>>>>> I create 3 tests for verifying options with ranges. The tests mostly
>>>>>> rely on common/optionsvalidation/JVMOptionsUtils.java. Class in this
>>>>>> file contains functions to get options with ranges as list(by 
>>>>>> parsing
>>>>>> new option "-XX:+PrintFlagsRanges" output), run command line test 
>>>>>> for
>>>>>> list of options and other. The actual test code contained in
>>>>>> common/optionsvalidation/JVMOption.java file - testCommandLine(),
>>>>>> testDynamic(), testJcmd() and testAttach() methods.
>>>>>> common/optionsvalidation/IntJVMOption.java and
>>>>>> common/optionsvalidation/DoubleJVMOption.java source files contain
>>>>>> classes derived from JVMOption class for integer and double JVM
>>>>>> options correspondingly.
>>>>>>
>>>>>> Here are description of the tests:
>>>>>> 1)
>>>>>> hotspot/test/runtime/CommandLine/OptionsValidation/TestOptionsWithRanges.java 
>>>>>>
>>>>>>
>>>>>>
>>>>>> This test get all options with ranges by parsing output of new 
>>>>>> option
>>>>>> "-XX:+PrintFlagsRanges" and verify these options by starting Java 
>>>>>> and
>>>>>> passing options in command line with valid and invalid values.
>>>>>> Currently it verifies about 106 options which have ranges.
>>>>>> Invalid values are values which out-of-range. In test used values
>>>>>> "min-1" and "max+1".In this case Java should always exit with code 1
>>>>>> and print error message about out-of-range value(with one exception,
>>>>>> if option is unsigned and passing negative value, then out-of-range
>>>>>> error message is not printed because error occurred earlier).
>>>>>> Valid values are values in range, e.g. min&max and also several
>>>>>> additional values. In this case Java should successfully exit(exit
>>>>>> code 0) or exit with error code 1 for other reasons(low memory with
>>>>>> certain option value etc.). In any case for values in range Java
>>>>>> should not print messages about out of range value.
>>>>>> In any case Java should not crash.
>>>>>> This test excluded from JPRT because it takes long time to execute
>>>>>> and also fails - some options with value in valid range cause 
>>>>>> Java to
>>>>>> crash(bugs are submitted).
>>>>>>
>>>>>> 2)
>>>>>> hotspot/test/runtime/CommandLine/OptionsValidation/TestOptionsWithRanges.java 
>>>>>>
>>>>>>
>>>>>>
>>>>>> This test get all writeable options with ranges by parsing output of
>>>>>> new option "-XX:+PrintFlagsRanges" and verify these options by
>>>>>> dynamically changing it's values to the valid and invalid values.
>>>>>> Used 3 methods for that: DynamicVMOption isValidValue and
>>>>>> isInvalidValue methods, Jcmd and by attach method. Currently 3
>>>>>> writeable options with ranges are verified by this test.
>>>>>> This test pass in JPRT.
>>>>>>
>>>>>> 3)
>>>>>> hotspot/test/runtime/CommandLine/OptionsValidation/TestJcmdOutput.java 
>>>>>>
>>>>>>
>>>>>> This test verified output of Jcmd when out-of-range value is set to
>>>>>> the writeable option or value violates option constraint. Also this
>>>>>> test verify that jcmd not write error message to the target process.
>>>>>>  This test pass in JPRT.
>>>>>>
>>>>>>
>>>>>> I am not write special tests for constraints for this JEP because
>>>>>> there are exist test for that(e.g.
>>>>>> test/runtime/CompressedOops/ObjectAlignment.java for
>>>>>> ObjectAlignmentInBytes or
>>>>>> hotspot/test/gc/arguments/TestHeapFreeRatio.java for
>>>>>> MinHeapFreeRatio/MaxHeapFreeRatio).
>>>>>>
>>>>>> Webrev: http://cr.openjdk.java.net/~ddmitriev/8059557/webrev.00/
>>>>>> <http://cr.openjdk.java.net/%7Eddmitriev/8059557/webrev.00/>
>>>>>>
>>>>>> JEP: https://bugs.openjdk.java.net/browse/JDK-8059557
>>>>>>
>>>>>> Thanks,
>>>>>> Dmitry
>>>>>>
>>>>>
>>>>
>>
>

More information about the hotspot-dev mailing list