RFR: 8303525: Refactor/cleanup open/test/jdk/javax/rmi/ssl/SSLSocketParametersTest.java [v3]

[Stuart Marks]

On Tue, 17 Oct 2023 11:33:00 GMT, Matthew Donovan <mdonovan at openjdk.org> wrote:

>> test/lib/jdk/test/lib/Asserts.java line 588:
>> 
>>> 586:         }
>>> 587:         // fail() throws a RuntimeException so this is unreachable.
>>> 588:         return null;
>> 
>> Hm, this is unfortunate. Even though this method throws along all code paths, the compiler doesn't do this analysis. Even though this "never happens" there's still a question of "but what if?" (I guess it could happen if somebody in the future were to modify the code above.) I'd suggest unconditionally throwing an exception here (probably InternalError) to be more explicit. Returning null will result in an inexplicable NPE if somebody dereferences the return value.
>
> Actually, this is a mistake. If I call `fail()` immediately after the test method executes (because an exception wasn't thrown), the RuntimeException which will promptly be caught in my try/catch and that isn't what we want. I'm moving that invocation to the end which looks and works a lot better.

Ah, right. Throwing unconditionally at the end of the method is the right thing. Good catch.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/14932#discussion_r1364340476