RFR: JDK-8157850 Jar tests should pass through VM options

[Jonathan Gibbons]

On 05/25/2016 01:33 PM, David Holmes wrote:
> On 26/05/2016 6:04 AM, Jonathan Gibbons wrote:
>> Using JAVA_OPTIONS for tools is conceptually wrong.
>>
>> JAVA_OPTIONS is specifically intended to pass options to VM instances,
>> as created by a "java" command.  It is not intended that you should
>> prefix the options with -J and use them for arbitrary tools.
>
> I have to agree. There is no guarantee the options being passed for 
> the VM under test make any sense for the running of the jar tool in 
> the jar tests. I think a number of hotspot related test options could 
> cause problems here.
>
> Are there specific VM options that people think should be passed 
> through? The bug report has no detail at all.
>
> David

Generally, I think that blindly passing through all the options 
regardless is as bad a programming practice as never passing them 
though.  There are some that make sense to all VMs, like -ea, -esa etc, 
and maybe system properties, but for those, the VM options mechanism is 
generally good enough.  (i.e. system properties test.vm.opts, 
test.tool.vm.opts)

 From a jtreg point of view, I'd be interested to know uses cases for 
passing additional more specific options to the VMs used to run tools 
like jar, jlink, javac, etc

-- Jon.



>
>> The code in the webrev is also perverse for taking the trouble to split
>> the string to a stream, collect the results into a list, convert the
>> list back into a stream again and use .forEach.
>>
>> You could do better, and much simpler, with something like
>>
>>     if (!option.isEmpty()) {
>>         commands.addAll(Arrays.asList(option.split("\\s+",-1)));
>>     }
>>
>> -- Jon
>>
>>
>> On 05/25/2016 10:48 AM, Martin Buchholz wrote:
>>> Hi Andrey,
>>>
>>> Looking at http://openjdk.java.net/jtreg/vmoptions.html,
>>> I see we have both test.vm.opts and test.tool.vm.opts
>>> and the latter is supposed to take care of adding "-J".
>>>
>>> +        VM_OPTIONS.stream().map(opt -> "-J" +
>>> opt).forEach(commands::add);
>>> +        JAVA_OPTIONS.stream().map(opt -> "-J" +
>>> opt).forEach(commands::add);
>>>
>>> ---
>>>
>>> Maybe splitting on "\\s+" would be better.
>>>
>>> ---
>>>
>>> I think we should have test library methods to return the List<String>
>>> for java subprocesses, one that could try hard to get the option
>>> tokenization correct.
>>>
>>> ---
>>>
>>>
>>> On Wed, May 25, 2016 at 9:07 AM, Andrey Nazarov
>>> <andrey.x.nazarov at oracle.com> wrote:
>>>> Some jar tests start VMs without passing vmoptions from jtreg.
>>>>
>>>> This fix pass jtreg's vmoptions and javaoptions to processes(java, 
>>>> jar,
>>>> javac) started by tests.
>>>>
>>>> webrev: http://cr.openjdk.java.net/~anazarov/8157850/webrev.01/
>>>>
>>>> jbs: https://bugs.openjdk.java.net/browse/JDK-8157850
>>>>
>>>> --Andrey
>>>>
>>