RFR 8059557: Test set for "Validate JVM Command-Line Flag Arguments"
Dmitry Dmitriev
dmitry.dmitriev at oracle.com
Tue Jun 16 15:58:36 UTC 2015
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