On Wed, 23 Apr 2025 06:08:59 GMT, Kim Barrett <kbarr...@openjdk.org> wrote:
>> Brent Christian has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> make waitForReferenceProcessingMethod volatile
>
> test/lib/jdk/test/whitebox/WhiteBox.java line 605:
>
>> 603: throw (InterruptedException) cause;
>> 604: } else {
>> 605: throw new RuntimeException(e);
>
> I still think it would be better to finish unwrapping e, rather than further
> wrapping it. We know the only checked exception is InterruptedException,
> because we checked that when looking up the method. That leaves either
> RuntimeException or Error. I think it is generally inappropriate to wrap
> Errors, and we don't really need to additionally wrap the RuntimeException
> case. But I'm going to approve this change anyway, and leave that question to
> real Java programmers.
For production code, I might do more.
But for a test library (where, IMO, the exception handling path won't see much
action), doing enough to get the information into the test log seems sufficient
to me.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24527#discussion_r2056588317
