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

[Ioi Lam]

Hi Lenoid,

Thanks for fixing this.

If you just look at a test case, it's not very obvious what the 
difference is between

     LingerApp.startApp(myApp, "-XX:Xyz=123");
     LingerApp.startAppVmOpts(myApp, "-XX:Xyz=123");

How about renaming startAppVmOpts/runAppVmOpts -> 
startAppExactVmOpts/runAppExactVmOpts?

===

  415     public static void startApp(LingeredApp theApp, String... 
additonalVMOpts) throws IOException {
  416             String[] vmOpts = additonalVMOpts == null ? 
Utils.getTestJavaOpts() : Utils.appendTestJavaOpts(additonalVMOpts);
  417             startAppVmOpts(theApp, vmOpts);
  418     }

I think there's no need to check for additonalVMOpts == null. If the 
caller passes no arguments, additonalVMOpts will be an empty array (but 
not null);

You will get a null for additonalVMOpts only if the caller explicitly 
passes in a null, like this

       LingerApp.startApp(theApp, null);

but this is not good programming style and you will get a Javac warning:

public class DotDotDot {
   public static void main(String args[]) {
     doit();
     doit(null);
   }
   static void doit(String ...args) {
     System.out.println(args);
   }
}

$ javac DotDotDot.java
DotDotDot.java:4: warning: non-varargs call of varargs method with 
inexact argument type for last parameter;
     doit(null);
          ^
   cast to String for a varargs call
   cast to String[] for a non-varargs call and to suppress this warning
1 warning

Thanks!
- Ioi


On 3/25/20 8:55 AM, Leonid Mesnik 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
>