RFR: 8073331: [TESTBUG] Test for VM option file feature (VM options specified in file)
Daniel D. Daugherty
daniel.daugherty at oracle.com
Tue Sep 22 16:13:02 UTC 2015
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
>>
>
More information about the hotspot-runtime-dev
mailing list