On Thu, 24 Sep 2020 20:25:27 GMT, Andy Herrick <herr...@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
