RFR: JDK-8157850 Jar tests should pass through VM options
martinrb at google.com
Wed May 25 19:21:17 UTC 2016
Agreed and Reviewed!
On Wed, May 25, 2016 at 11:57 AM, Andrey Nazarov
<andrey.x.nazarov at oracle.com> wrote:
> Hi Martin,
> See my comments inline.
>> On 25 May 2016, at 20:48, Martin Buchholz <martinrb at 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.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
More information about the core-libs-dev