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

[Dmitry Dmitriev]

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
>