On Mon, 24 Feb 2025 20:29:09 GMT, Alexey Semenyuk <asemen...@openjdk.org> wrote:
>> - Unify TKit.createTempDirectory() and TKit.createTempFile() functions.
>> Remove `throws IOexception` from their signatures. Add relevant tests.
>> - JPackageCommand.java: remove try/catch(IOexception) around
>> `TKit.createTempDirectory()` call.
>> - AppImagePackageTest.java: replace `TKit.createTempDirectory(null)` with
>> `TKit.createTempDirectory("appimage")` to fix NPE.
>> - Make `TKit.assertDirectoryExists(Path, Optional<Boolean>)` private. This
>> is internal and shouldn't be exposed.
>> - add `TKit.assertEquals(boolean, boolean)` and
>> `TKit.assertNotEquals(boolean, boolean)` with tests.
>> - AdditionalLauncher.java: remove redundant code duplicating functionality
>> of `TKit.createTempFile()` (should have been cleaned long ago).
>> - JPackageCommand.java: add a check for the main jar in the app image if
>> the main jar was specified. Missing part of
>> [JDK-8325089](https://bugs.openjdk.org/browse/JDK-8325089).
>
> Alexey Semenyuk has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Copyright year update
Looks fine overall. I left one comment.
test/jdk/tools/jpackage/helpers-test/jdk/jpackage/test/TKitTest.java line 232:
> 230: if ((expectedExceptionClass == null) == (expectedPath ==
> null)) {
> 231: throw new IllegalArgumentException("Only one of
> `expectedPath` and `expectedExceptionClass` can be null");
> 232: }
This one is confusing. It will evaluate to true if both != null. Did you mean
"&&" instead of "=="?
-------------
PR Review: https://git.openjdk.org/jdk/pull/23754#pullrequestreview-2638639705
PR Review Comment: https://git.openjdk.org/jdk/pull/23754#discussion_r1968536523
