RFR: 8073331: [TESTBUG] Test for VM option file feature (VM options specified in file)
Ron Durbin
ron.durbin at oracle.com
Wed Sep 23 18:11:38 UTC 2015
I am looking at it now
>-----Original Message-----
>From: Dmitry Dmitriev
>Sent: Wednesday, September 23, 2015 11:52 AM
>To: Daniel Daugherty; hotspot-runtime-dev at openjdk.java.net; George Triantafillou
>Subject: Re: RFR: 8073331: [TESTBUG] Test for VM option file feature (VM options specified in file)
>
>Dan, thank you for sponsorship!
>
>As I understand that you and George reviews are sufficient, but we can
>wait for Ron review as well.
>
>I correct all typos, change JVM_FAIL_1 to JVM_FAIL_WITH_EXIT_CODE_1 and
>also now create empty file at runtime, because diff doesn't like empty
>files and skip them.
>
>webrev.02: http://cr.openjdk.java.net/~ddmitriev/8073331/webrev.02/
><http://cr.openjdk.java.net/%7Eddmitriev/8073331/webrev.02/>
>webrev.02.vs.01:
>http://cr.openjdk.java.net/~ddmitriev/8073331/webrev.02.vs.01/
><http://cr.openjdk.java.net/%7Eddmitriev/8073331/webrev.02.vs.01/>
>
>Thanks,
>Dmitry
>
>On 23.09.2015 16:20, Daniel D. Daugherty wrote:
>> 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