RFR: 8073331: [TESTBUG] Test for VM option file feature (VM options specified in file)
Dmitry Dmitriev
dmitry.dmitriev at oracle.com
Wed Sep 23 19:00:32 UTC 2015
Ron, thank you for review!
Yes, new test cases will be added after removing the size limit.
Dmitry
On 23.09.2015 21:58, Ron Durbin wrote:
> This looks good for now, we will need more test covrerage for removing the size limit.
>
> Thx Dmitry
>
>> -----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