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

[Matthew Donovan]

On Mon, 16 Oct 2023 23:56:24 GMT, Stuart Marks <smarks at openjdk.org> wrote:

>> Matthew Donovan has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   retained a reference to the RMI server and improved naming
>
> 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.

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

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