RFR: 8073331: [TESTBUG] Test for VM option file feature (VM options specified in file)

[Daniel D. Daugherty]

 > About JVM_FAIL_1 - it means fail with 1 exit code, but probably
 > it seems confusing and I will change it just to JVM_FAIL.

I would be OK with JVM_FAIL_WITH_EXIT_CODE_1 to be clear.

Dan


On 9/23/15 1:56 AM, Dmitry Dmitriev wrote:
> Hi George,
>
> Thank you for review! I will correct all typos.
>
> About JVM_FAIL_1 - it means fail with 1 exit code, but probably it seems confusing and I will change it just to JVM_FAIL.
>
> Thanks,
> Dmitry
>
> ----- Original Message -----
> From: george.triantafillou at oracle.com
> To: hotspot-runtime-dev at openjdk.java.net
> Sent: Wednesday, September 23, 2015 2:28:05 AM GMT +03:00 Iraq
> Subject: Re: RFR: 8073331: [TESTBUG] Test for VM option file feature (VM options specified in file)
>
> Hi Dmitry,
>
> In test/runtime/CommandLine/VMOptionsFile/TestVMOptionsFile.java:
>
> 89     /* VM Option file which not exist */
>     Typo:  "which does not exist"
>
> 99     private static final int JVM_FAIL_1 = 1;
>     Just a comment.  I'm curious why you chose to add "_1" to the
> variable name.
>
> 107     /* VM Options which passed to the JVM */
>     Typo: "which are passed to"
>
> 317         /* Check that parameters are gotten from third VM Option
> file. Contain mix of the options */
>     Change to "from third VM Option file which contains a mix of the options"
>
> 504          * The same as previous but without arguments from pseudo
> command line..
>     Typo: Remove second period, change to "pseudo command line."
>
> 555         /* Pass VM option file which is not accessible(without read
> permissions) */
>     Typo: Add a space before the open parenthesis: "which is not
> accessible (without"
>
> Otherwise, this looks good.  I don't need to see a new webrev if you
> make these changes.
>
> -George
>
> On 9/22/2015 4:34 PM, Dmitry Dmitriev wrote:
>> 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'.