On Wed, 26 Nov 2025 01:49:47 GMT, Alexander Matveev <[email protected]>
wrote:
>> Added following tests:
>> - Verify mac app store PKG doesn't have scripts (added to
>> `PkgScriptsTest.java`).
>> - `--launcher-as-service` ignored for mac app store PKG (added to
>> `ServiceTest.java`).
>> - `--launcher-as-service` for additional launcher ignored for mac app store
>> PKG (added to `ServiceTest.java`).
>
> Alexander Matveev has updated the pull request with a new target base due to
> a merge or a rebase. The incremental webrev excludes the unrelated changes
> brought in by the merge/rebase. The pull request contains three additional
> commits since the last revision:
>
> - Merge remote-tracking branch 'upstream/master' into JDK-8351095
> - 8351095: [macos] Add more jpackage tests for --mac-app-store option [v2]
> - 8351095: [macos] Add more jpackage tests for --mac-app-store option
Changes requested by asemenyuk (Reviewer).
test/jdk/tools/jpackage/helpers/jdk/jpackage/test/LauncherAsServiceVerifier.java
line 171:
> 169: // Launcher should not be a service with
> "--launcher-as-service"
> 170: // and "--mac-app-store" specified.
> 171: verify(cmd, launcherName, launcherAsService &&
> !cmd.hasArgument("--mac-app-store"));
This change looks redundant. In case of "--mac-app-store" the list of launcher
names that should be installed as services will (should) be empty after the
tweak to the `boolean launcherAsService(JPackageCommand cmd, String
launcherName)` function.
test/jdk/tools/jpackage/helpers/jdk/jpackage/test/LauncherAsServiceVerifier.java
line 202:
> 200: }
> 201:
> 202: if (launcherAsServiceNames.isEmpty() || cmd.isRuntime() ||
> cmd.hasArgument("--mac-app-store")) {
Same as above, this is a redundant change. `launcherAsServiceNames` should be
empty if "--mac-app-store" is present.
test/jdk/tools/jpackage/helpers/jdk/jpackage/test/LauncherAsServiceVerifier.java
line 238:
> 236: } else {
> 237: return
> Objects.requireNonNull(partitionLaunchers(cmd).get(true));
> 238: }
Same as above, this is a redundant change: `partitionLaunchers()` function will
return empty partition with the names of launchers installed as services after
the tweak to the `boolean launcherAsService(JPackageCommand cmd, String
launcherName)` function.
-------------
PR Review: https://git.openjdk.org/jdk/pull/28487#pullrequestreview-3508026204
PR Review Comment: https://git.openjdk.org/jdk/pull/28487#discussion_r2562528333
PR Review Comment: https://git.openjdk.org/jdk/pull/28487#discussion_r2562533081
PR Review Comment: https://git.openjdk.org/jdk/pull/28487#discussion_r2562541016
