RFR: 8240698: LingeredApp does not pass getTestJavaOpts() to the children process if vmArguments is already specified

[Stefan Karlsson]

On 2020-03-25 17:40, Igor Ignatyev wrote:
> Hi Leonid,
>
> I have briefly looked at the patch, a few comments so far:
>
> test/hotspot/jtreg/serviceability/sa/ClhsdbFlags.java:
>   - at L#114, could you please call static method using class name (as the opposite of using instance)? or was it meant to be theApp.runAppVmOpts(vmArgs) ?
>
> test/lib/jdk/test/lib/apps/LingeredApp.java:
> - it seems that code indent of startApp(LingeredApp, String[]) isn't correct
> - I don't like startAppVmOpts name, but unfortunately don't have a better suggestion (yet)

I was going to say the same. Jtreg has the concept of "java options" and 
"vm options". We have had a fair share of bugs and wasted time when 
tests have been using the "vm options" part (VM_OPTIONS, 
test.vm.options, etc), and we've been moving away from using that way to 
pass options. I recently cleaned up some of this with:

8237111: LingeredApp should be started with getTestJavaOpts

Because of this, I would prefer if we used a name that doesn't include 
"VmOpts", because it's too alike the other concept. Some suggestions:
  startAppJavaOptions
  startAppUsingJavaOptions
  startAppWithJavaOptions
  startAppExactJavaOptions
  startAppJvmOptions

Thanks,
StefanK

> Thanks,
> -- Igor
>
>> On Mar 25, 2020, at 8:55 AM, Leonid Mesnik <leonid.mesnik at oracle.com> wrote:
>>
>> Hi
>>
>> Could you please review following fix which change LingeredApp to prepend vm options to java/vm.test.opts when startApp is used and provide startAppVmOpts to override options completely.
>>
>> The intention is to avoid issue like in this bug where test/jtreg options were ignored by tests. Also I fixed some tests where intention was to append vm options rather than to override them.
>>
>> webrev: http://cr.openjdk.java.net/~lmesnik/8240698/webrev.00/
>>
>> bug: https://bugs.openjdk.java.net/browse/JDK-8240698
>>
>> Leonid
>>