RFR: 8231591: [TESTBUG] Create additional two phase jpackage tests

[Alexander Matveev]

On Mon, 21 Sep 2020 14:39:33 GMT, Alexey Semenyuk <asemenyuk at openjdk.org> wrote:

> How about testing of other jpackage command line options in two phase mode? Like "--name", "--version"? Any plans to
> add them?

Plan was to file separate bugs for any additional options. I do not think we should put everything in one issue. This
issue for desktop integration for additional launchers.

> test/jdk/tools/jpackage/helpers/jdk/jpackage/test/WindowsHelper.java line 140:
> 
>> 138:             cmd.verifyIsOfType(PackageType.WINDOWS);
>> 139:             this.cmd = cmd;
>> 140:             this.name = name;
> 
> I'd suggest to have the following initialization of "name" field:
> `this.name = (name == null ? cmd.name() : name)`
> This will help to avoid multiple `(name == null ? cmd.name() : name)` in the code.

Fixed.

> test/jdk/tools/jpackage/share/MultiLauncherTwoPhaseTest.java line 54:
> 
>> 52: public class MultiLauncherTwoPhaseTest {
>> 53:
>> 54:     public static void main(String[] args) throws Exception {
> 
> Please replace main() function with dedicated test function. You can use SimplePackageTest as a template, This would
> give better control on how to run the test in debug mode.

Fixed.

> test/jdk/tools/jpackage/share/MultiLauncherTwoPhaseTest.java line 56:
> 
>> 54:     public static void main(String[] args) throws Exception {
>> 55:         TKit.run(args, () -> {
>> 56:             Path appimageOutput = TKit.workDir().resolve("appimage");
> 
> Please use TKit.createTempDirectory() to create directories for intermediate data of test runs. This allows clean
> multiple runs of the test in the same work directory.

Fixed.

> test/jdk/tools/jpackage/share/MultiLauncherTwoPhaseTest.java line 80:
> 
>> 78:                     })
>> 79:                     .addInstallVerifier(cmd -> {
>> 80:                         launcher1.verify(cmd);
> 
> There is no need in explicit call to AdditionalLauncher.verify() as this function is scheduled for the call from
> AdditionalLauncher.applyTo() call.  Thus there is no need to change access to this function from "private" to "public"
> in your patch to AdditionalLauncher.java.

Fixed.

> test/jdk/tools/jpackage/helpers/jdk/jpackage/test/PackageTest.java line 559:
> 
>> 557:                         && !cmd.isPackageUnpacked(
>> 558:                                 "Not verifying desktop integration")) {
>> 559:                     new WindowsHelper.DesktopIntegrationVerifier(cmd, null);
> 
> I'd rather add a loop calling DesktopIntegrationVerifier() for all launchers than copy/paste the code in
> AdditionalLauncher.java

Fixed. I also removed code from AdditionalLaunchers.

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

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