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

Dmitry Dmitriev dmitry.dmitriev at oracle.com
Wed Jun 17 19:29:36 UTC 2015 

Hi Gerard,

Can you please look at small enhancement that I made by Christian 
suggestion - ran test JVM with the same type as current(for all cases, 
not only for "-client" as in 06 webrev). Tested by JPRT. Sorry for 
confusion. Thank you!

Webrev: http://cr.openjdk.java.net/~ddmitriev/8059557/webrev.07/ 
<http://cr.openjdk.java.net/%7Eddmitriev/8059557/webrev.07/>
Webrev incremental(from 06): 
http://cr.openjdk.java.net/~ddmitriev/8059557/webrev.07.incremental.from06/ 
<http://cr.openjdk.java.net/%7Eddmitriev/8059557/webrev.07.incremental.from06/>

Regards,
Dmitry

On 17.06.2015 18:07, Gerard Ziemski wrote:
> 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