On Mon, 15 Dec 2025 21:32:34 GMT, Alexey Semenyuk <[email protected]> wrote:
>> - Add exception type as an additional generic parameter to all `Throwing...`
>> interfaces
>> - Add unit tests for "jdk.jpackage.internal.util.function" package
>> - Add `ExceptionBox.toUnchecked()`. It is equivalent to
>> `ExceptionBox.rethrowUnchecked()`, but doesn't throw
>> - Replace `ExceptionBox.rethrowUnchecked(...)` with
>> `ExceptionBox.toUnchecked(...)`; this increased test coverage of
>> "jdk.jpackage.internal.util.function" package from 86% to 97%
>> - Change exception handling in `ExceptionBox.toUnchecked()` (former
>> `ExceptionBox.rethrowUnchecked()`):
>> - `InterruptedException`: instead of rethrowing an exception of this
>> type, it calls `Thread.currentThread().interrupt()` and then rethrows it
>> - Add `ExceptionBox.unbox()` (former `TKit.unbox()`). It is complementary
>> to `ExceptionBox.toUnchecked()`
>> - Catch `Exception` instead of `Throwable` where appropriate to avoid
>> catching `Error`-s. The idea is not to handle fatal errors
>> - Replace vague `throws Throwable` exception specifications with more
>> specific ones where appropriate
>> - Add `ExceptionBox.reachedUnreachable()` for use in locations that control
>> flow is not expected to reach
>>
>> Supplementary changes:
>> - Take advantage of the updated `Throwing...` interfaces in the `Result`
>> class.
>> - Add unit tests for the `Result` class and fix uncovered minor issues.
>
> Alexey Semenyuk 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 seven additional
> commits since the last revision:
>
> - Result: add unit tests, remove unused mapErrors(UnaryOperator<Collection<?
> extends Exception>>) function; better and more consistent error reporting
> based on testing
> - Result: replace create(Supplier<T>) with of(ThrowingSupplier<T, E>)
> - CustomInfoPListTest: fix local test failure:
>
> [22:38:21.786] TRACE: assertStringListEquals(): Check contents of
> [CustomInfoPListTest/testPackage.571a7829/vanilla/PackageCustomInfoPListTest.app/Contents/runtime/Contents/Info.plist]
> and
> [CustomInfoPListTest/testPackage.571a7829/unpacked-pkg/unpacked/Applications/PackageCustomInfoPListTest.app/Contents/runtime/Contents/Info.plist]
> plist files are the same
> [22:38:21.786] TRACE: assertStringListEquals( 1,
> /CFBundleDevelopmentRegion: en-US)
> [22:38:21.786] TRACE: assertStringListEquals( 2, /CFBundleExecutable:
> libjli.dylib)
> [22:38:21.786] TRACE: assertStringListEquals( 3, /CFBundleIdentifier:
> Hello)
> [22:38:21.787] TRACE: assertStringListEquals( 4,
> /CFBundleInfoDictionaryVersion: 7.0)
> [22:38:21.787] TRACE: assertStringListEquals( 5, /CFBundleName:
> PackageCustomInfoPListTest)
> [22:38:21.787] TRACE: assertStringListEquals( 6, /CFBundlePackageType:
> BNDL)
> [22:38:21.787] TRACE: assertStringListEquals( 7,
> /CFBundleShortVersionString: 1.0)
> [22:38:21.787] TRACE: assertStringListEquals( 8, /CFBundleSignature: ????)
> [22:38:21.787] TRACE: assertStringListEquals( 9, /CFBundleVersion: 1.0)
> [22:38:21.787] ERROR: Actual list is shorter than expected by 5 elements:
> Check contents of
> [CustomInfoPListTest/testPackage.571a7829/vanilla/PackageCustomInfoPListTest.app/Contents/runtime/Contents/Info.plist]
> and
> [CustomInfoPListTest/testPackage.571a7829/unpacked-pkg/unpacked/Applications/PackageCustomInfoPListTest.app/Contents/runtime/Contents/Info.plist]
> plist files are the same
> [22:38:21.787] [ FAILED ] CustomInfoPListTest.testPackage(APP); checks=54
> Exception in thread "main" java.lang.AssertionError: Actual list is
> shorter than expected by 5 elements: Check contents of
> [CustomInfoPListTest/testPackage.571a7829/vanilla/PackageCustomInfoPListTest.app/Contents/runtime/Contents/Info.plist]
> and
> [CustomInfoPListTest/testPackage.571a7829/unpacked-pkg/unpacked/Applications/PackageCustomInfoPListTest.app/Cont...
Looks good.
test/jdk/tools/jpackage/helpers/jdk/jpackage/test/TKit.java line 237:
> 235: throw ExceptionBox.unbox(ex);
> 236: }
> 237: } catch (Exception|AssertionError t) {
Should we use space around `|` or not? I see some old cases when we do `catch
(ExceptionBox | InvocationTargetException ex)` and in new cases we do `catch
(Exception|AssertionError t)`.
-------------
PR Review: https://git.openjdk.org/jdk/pull/28731#pullrequestreview-3581053974
PR Review Comment: https://git.openjdk.org/jdk/pull/28731#discussion_r2621610599
