RFR: 8073331: [TESTBUG] Test for VM option file feature (VM options specified in file)

Tue Sep 22 20:34:49 UTC 2015

Dan, thank you for review! I will correct the typo. Still need one more 
reviewer and sponsor. Thanks!

Dmitry

On 22.09.2015 19:13, Daniel D. Daugherty wrote:
> Hi Dmitry,
>
> test/runtime/CommandLine/VMOptionsFile/TestVMOptionsFile.java
>     I missed one typo:
>
>     -     * Make file non-readable by modifing it permissions.
>     -     * If file support "posix" attributes, then modify it.
>     +     * Make file non-readable by modifing its permissions.
>     +     * If file supports "posix" attributes, then modify it.
>
>     Typo: 'modifing' -> 'modifying'
>
> test/runtime/CommandLine/VMOptionsFile/optionFILE_2
>     No comments.
>
> test/runtime/CommandLine/VMOptionsFile/optionfile_quote
>     No comments.
>
> Thumbs up. I don't need to see another webrev if you fix the above.
>
> Dan
>
>
> On 9/22/15 8:07 AM, Dmitry Dmitriev wrote:
>> Hello Dan,
>>
>> Thank you for comments! Here an updated webrev:
>> webrev.01: http://cr.openjdk.java.net/~ddmitriev/8073331/webrev.01/ 
>> <http://cr.openjdk.java.net/%7Eddmitriev/8073331/webrev.01/>
>> webrev.01.vs.00: 
>> http://cr.openjdk.java.net/~ddmitriev/8073331/webrev.01.vs.00/ 
>> <http://cr.openjdk.java.net/%7Eddmitriev/8073331/webrev.01.vs.00/>
>> Note: 2 option files were changed, because I add tests for another 
>> two properties.
>>
>> Thank you,
>> Dmitry
>>
>> On 22.09.2015 0:12, Daniel D. Daugherty wrote:
>>> On 9/15/15 9:08 AM, Dmitry Dmitriev wrote:
>>>> Hello,
>>>>
>>>> Please review a new test for recently added feature: JDK-8061999 
>>>> "Enhance VM option parsing to allow options to be specified in a 
>>>> file".  Also, I need a sponsor for this fix.
>>>>
>>>> This test verifies new "-XX:VMOptionsFile" VM Options. Several 
>>>> valid VM Options files are tested: with different number and types 
>>>> of options, empty file, file with only tab and spaces, file which 
>>>> contains properties with quotes. Also invalid situations are 
>>>> tested: not existing file, folder instead of the file, file with 
>>>> size greater than allowed, several VM option files and so on. Also, 
>>>> test verifies "last argument wins" situation: when the same option 
>>>> or property is defined several times in environment variables, on 
>>>> command line and in VM Options file. In this case option or 
>>>> property should get the last value from the most priority source. 
>>>> Here a priority of the sources(from less priority to the more 
>>>> priority):
>>>> 1) Flags file
>>>> 2) JAVA_TOOL_OPTIONS environment variables
>>>> 3) Pseudo command line from launcher(which can include VM Options 
>>>> file)
>>>> 4) _JAVA_OPTIONS
>>>>
>>>> Webrev: http://cr.openjdk.java.net/~ddmitriev/8073331/webrev.00/
>>>
>>> I was also expecting to see 
>>> test/runtime/VMOptions/EnvironmentTest.java,
>>> but perhaps that new test is coming via a different bug ID...
>>>
>>> I don't see the following test data files in the webrev:
>>>
>>>     optionfile_with_optionfile
>>>     optionfile_with_same_optionfile
>>>     optionfile_wo_read_perm
>>>
>>> Update: OK, I see now. These are generated by the test...
>>>
>>>
>>> test/runtime/CommandLine/VMOptionsFile/TestVMOptionsFile.java
>>>     L122      * Make file non-readable by modifing it permissions.:
>>>         Typo: 'it' -> 'its'.
>>>
>>>     L123:      * If file support "posix" attributes,
>>>         Typo: 'support' -> 'supports'
>>>
>>>     L132:         } else if (supportedAttr.contains("acl")) {
>>> <snip>
>>>     L145:             view.setAcl(acl);
>>>         Wow! Java makes ACL code look easy. This is great evolution
>>>         from POSIX 1003.6 APIs...
>>>
>>>     L157:         /* Create VM option file with following 
>>> parametares "-XX:VMOptionFile=<absolute_path_to_the_VM_option_file> */
>>>         Typo: 'parametares' -> 'parameters'.
>>>
>>>     L161:         /* Create VM option file with following 
>>> parametares "-XX:MinHeaFreeRatio=12
>>>         Typo: 'parametares' -> 'parameters'.
>>>         Typo: 'MinHeaFreeRatio' -> 'MinHeapFreeRatio'
>>>
>>>     L166:         /* Copy valid VM option file and change it 
>>> permission to make them not accessible */
>>>         Typo: 'it' -> 'its'
>>>         Typo: 'them' -> 'it'
>>>
>>>     L261:      * Check property value by examing output
>>>     L268:      * Check VM Option value by examing output
>>>         Typo: 'examing' -> 'examining'
>>>
>>>     L287:         /* Check that parameters are get from first VM 
>>> Option file.
>>>     L299:         /* Check that parameters are get from first VM 
>>> Option file ...
>>>     L312:          * Check that parameters are get from second VM 
>>> Option...
>>>     L329:         /* Check that parameters are get from third VM 
>>> Option file.
>>>         Typo: 'are get' -> 'are gotten'
>>>
>>>     L301:         addVMParam("@" + VM_OPTION_FILE_WITH_VM_OPTION_FILE);
>>>         I'm cool with '@<optionfile>' test, but it is not a feature
>>>         that was added by 8061999.
>>>
>>>     L363:          * Verify that VM Option file accept file with 64 
>>> properties and with one options on separate
>>>         Typo: 'accept file' -> 'accepts a file'
>>>         Typo: 'with one options' -> 'with one option'
>>>
>>>     L364:          * lines and properties use quotes a lot.
>>>         Grammar: 'and properties use' -> 'and properties that use'
>>>
>>>     L381          * Verify that VM Option file accept file maximum 
>>> allowed
>>>         Typo: 'accept file' -> 'accepts a file'
>>>
>>>     L445:          * Here an order of options processing and last 
>>> argument wins:
>>>         Typo: 'Here an order' -> 'Here is the order':
>>>
>>>     L450:          * In every cathegory arguments processed from 
>>> left to right and from up to down
>>>         Typo: 'cathegory' -> 'category'
>>>
>>>     L464:         /* Each category define it's own properties */
>>>         Typo: "define it's own" -> "defines its own"
>>>
>>>     L566:         /* Pass VM option file which not 
>>> accessible(without read permissions) */
>>>         Typo: 'which not' -> 'which is not'
>>>
>>> test/runtime/CommandLine/VMOptionsFile/flags_file
>>>     No comments.
>>>
>>> test/runtime/CommandLine/VMOptionsFile/optionFILE_2
>>>     No comments.
>>>
>>> test/runtime/CommandLine/VMOptionsFile/optionfile_1
>>>     No comments.
>>>
>>> test/runtime/CommandLine/VMOptionsFile/optionfile_3
>>>     No comments.
>>>
>>> test/runtime/CommandLine/VMOptionsFile/optionfile_bad_option
>>>     No comments.
>>>
>>> test/runtime/CommandLine/VMOptionsFile/optionfile_empty
>>>     No comments.
>>>
>>> test/runtime/CommandLine/VMOptionsFile/optionfile_long_property
>>>     No comments.
>>>
>>> test/runtime/CommandLine/VMOptionsFile/optionfile_lot_of_options_quote
>>>     No comments.
>>>
>>> test/runtime/CommandLine/VMOptionsFile/optionfile_only_tabsandspaces
>>>     No comments.
>>>
>>> test/runtime/CommandLine/VMOptionsFile/optionfile_quote
>>>     No comments.
>>>
>>> test/runtime/CommandLine/VMOptionsFile/optionfile_quote_max_size
>>>     No comments.
>>>
>>> test/runtime/CommandLine/VMOptionsFile/optionfile_unmatched_quote_1
>>>     No comments.
>>>
>>> test/runtime/CommandLine/VMOptionsFile/optionfile_unmatched_quote_2
>>>     No comments.
>>>
>>> test/runtime/CommandLine/VMOptionsFile/optionfile_very_long_property
>>>     No comments.
>>>
>>> I think all my comments are editorial. I didn't see anything wrong
>>> with any of the test code or data files. I know there are more
>>> test cases coming so I didn't check for completeness...
>>>
>>> Dan
>>>
>>>
>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8073331
>>>> Feature JBS: https://bugs.openjdk.java.net/browse/JDK-8061999
>>>> Tested: JPRT with new test
>>>>
>>>> Thank you,
>>>> Dmitry
>>>
>>
>