> On 25 May 2016, at 20:21, Martin Buchholz <marti...@google.com> wrote: > > Agreed and Reviewed!
+1. This looks fine. Hopefully it can be simplified, somewhat, in the future. -Chris. > > On Wed, May 25, 2016 at 11:57 AM, Andrey Nazarov > <andrey.x.naza...@oracle.com> wrote: >> 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 >>>> >>
