On Thu, 27 Feb 2025 14:17:30 GMT, Alexey Semenyuk <asemen...@openjdk.org> wrote:
> Add a test to verify jpackage is using a custom MSI condition blocking > package installation depending on the version of Windows where the package > installer runs. Support for this MSI condition was added in > [JDK-8150442](https://bugs.openjdk.org/browse/JDK-8150442). > > The test adds an unconditionally failing OS-version MSI condition to the > resource directory. MSI installer using this condition should always fail. > The exit code of the failed installation would be 1603. Extended error > information can be dug in the MSI log file. To make the test work, the > following changes to jpackage test lib have been made: > - Support non-0 exit code from MSI install handler. Support for non-0 exit > codes was added to install handlers of all other types too. Added > `PackageTest.setExpectedInstallExitCode(int)` method to configure the > expected exit code of a package installation; > - Support using msi log files when MSI and EXE packages get installed, > unpacked, or uninstalled. Added `PackageTest.createMsiLog(boolean)` to enable > or disable creation of msi log files in a PackageTest instance and > `Optional<Path> JPackageCommand.winMsiLogFile()` to access the current log > file from the callbacks registered with the PackageTest instance. > > Added tests for PackageTest class (PackageTestTest). > > Additionally, improved the code in WindowsHelper detecting paths to Start > Menu, Desktop, and other common paths. Previously, it relied on reading these > paths from the registry. On some machines, required registry keys are not > available. The code now will call .NET `Environment.GetFolderPath()` function > through powershell if a registry key is missing. Looks good with minor comments. test/jdk/tools/jpackage/helpers-test/jdk/jpackage/test/PackageTestTest.java line 407: > 405: > 406: void configureUninstallVerifiers(PackageTest test, > Consumer<Verifiable> verifiableAccumulator) { > 407: for (final var verifier : uninstallVerifiers) { Extra space after `verifier`. test/jdk/tools/jpackage/helpers/jdk/jpackage/test/PackageTest.java line 493: > 491: TKit.trace(String.format("No handler of [%s] action > for %s command", > 492: action, cmd.getPrintableCommandLine())); > 493: return; Do we really need `return` here and at line 495? Also, do you think it might be better to make `switch` consistent? For example this one uses `->` and below `:`. ------------- PR Review: https://git.openjdk.org/jdk/pull/23825#pullrequestreview-2649270868 PR Review Comment: https://git.openjdk.org/jdk/pull/23825#discussion_r1974455774 PR Review Comment: https://git.openjdk.org/jdk/pull/23825#discussion_r1974505960
