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


Reply via email to