Hi Martin,
See my comments inline.
> On 25 May 2016, at 20:48, Martin Buchholz <marti...@google.com> 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);
I considered this option for Basic.java test. But it doesn’t make code
clearer.The test requires both test.tool.vm.opts and test.vm.opts for jar tool
and java invocation.
For CLICompatibility.java test.tool.vm.opts is enough since test invokes only
jar tool.
>
> ---
>
> Maybe splitting on "\\s+" would be better.
In theory yes. I’ll update.
>
> ---
>
> 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.
Agree. But I think it should be separate patch with changes in tests that use
process builder with such option. Better we need some framework which will pass
options for us. It’s common problem not passing through VM options.
>
> ---
>
>
> On Wed, May 25, 2016 at 9:07 AM, Andrey Nazarov
> <andrey.x.naza...@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
>>
