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 18:11:33 UTC 2015 

On 9/23/15 11:52 AM, Dmitry Dmitriev wrote:
> Dan, thank you for sponsorship!

No problem. I'm setting up the push repo now... :-)


> As I understand that you and George reviews are sufficient, but we can 
> wait for Ron review as well.

I let Ron know about this latest version and he is starting his
review shortly.


>
> 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/>

I'm good with the delta webrev. Let's give Ron a little while and
see if he spots anything else...

Dan


>
> 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