RFR: 8350013: Add a test for JDK-8150442

Alexander Matveev almatvee at openjdk.org
Thu Feb 27 23:58:58 UTC 2025


On Thu, 27 Feb 2025 14:17:30 GMT, Alexey Semenyuk <asemenyuk at 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


More information about the core-libs-dev mailing list