RFR: 8073331: [TESTBUG] Test for VM option file feature (VM options specified in file)
Dmitry Dmitriev
dmitry.dmitriev at oracle.com
Wed Sep 23 17:52:18 UTC 2015
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