RFR: 8253426: jpackage is unable to generate working EXE for add-launcher …

[Alexey Semenyuk]

On Thu, 24 Sep 2020 20:25:27 GMT, Andy Herrick <herrick at openjdk.org> wrote:

> 8253426: jpackage is unable to generate working EXE for add-launcher configurations.
> secondary launchers ignored module, main-jar, and main-class in launcher properties file because  the LAUNCHER_DATA
> param was not removed from the set of params for each new add-module. testcase added in AddLauncherTest,java

test/jdk/tools/jpackage/share/AddLauncherTest.java line 215:

> 213:         TKit.assertEquals(moduleValue, JavaAppDesc.parse(
> 214:                 modularAppDesc.toString()).setBundleFileName(null).toString(),
> 215:                 "app.mainmodule value in cfg file not as expected");

I'd suggest to change comment to neutral "Check value of app.mainmodule in cfg file".

test/jdk/tools/jpackage/share/AddLauncherTest.java line 213:

> 211:         String mainClass = null;
> 212:         String classpath = null;
> 213:         TKit.assertEquals(moduleValue, JavaAppDesc.parse(

The first parameter of TKit.assertEquals() function should be expected value which I assume is the value obtained with
the call to JavaAppDesc.parse(...) and the second parameter is actual value that seems to be the value of moduleValue
variable. So the first two parameters of TKit.assertEquals() call should be swapped.

test/jdk/tools/jpackage/share/AddLauncherTest.java line 218:

> 216:
> 217:         TKit.trace("moduleValue: " + moduleValue + " mainClass: " + mainClass
> 218:                     + " classpath: " + classpath);

`classpath` variable is `null` at this point. What is the point to log its value? The whole log statement is excessive
because `classpath` variable is `null` and value of `moduleValue` variable is logged implicitly in TKit.assertEquals()
call.

test/jdk/tools/jpackage/share/AddLauncherTest.java line 224:

> 222:         mainClass = cfg.getValue("Application", "app.mainclass");
> 223:         classpath = cfg.getValue("Application", "app.classpath");
> 224:         TKit.assertEquals(mainClass, nonModularAppDesc.className(),

Two first parameters of TKit.assertEquals() call need to be swapped as the first parameter should be expected and the
second actual value.

test/jdk/tools/jpackage/share/AddLauncherTest.java line 225:

> 223:         classpath = cfg.getValue("Application", "app.classpath");
> 224:         TKit.assertEquals(mainClass, nonModularAppDesc.className(),
> 225:                 "app.mainclass value in NonModularAppLauncher cfg file");

I'd insert log "Check" word at the beginning of the log comment to keep it aligned with other log statements.

test/jdk/tools/jpackage/share/AddLauncherTest.java line 228:

> 226:         TKit.assertTrue(classpath.startsWith("$APPDIR" + File.separator
> 227:                 + nonModularAppDesc.jarFileName()),
> 228:                 "app.classpath value in ModularAppLauncher cfg file");

Changing log comment to `String.format("Check value of app.classpath=[%s] in ModularAppLauncher cfg file is as
expected", classpath)` would output value of "app.classpath" property in test log and eliminate need for separate
TKit.trace(...) call.

-------------

PR: https://git.openjdk.java.net/jdk/pull/347