RFR: 8073331: [TESTBUG] Test for VM option file feature (VM options specified in file)
Daniel D. Daugherty
daniel.daugherty at oracle.com
Wed Sep 23 13:20:35 UTC 2015
I will sponsor once we see one more review.
I believe Ron is planning to review.
Dan
On 9/22/15 2:34 PM, Dmitry Dmitriev wrote:
> 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
>>>>
>>>
>>
>
More information about the hotspot-runtime-dev
mailing list