RFR: 8298939: Refactor open/test/jdk/javax/rmi/ssl/SSLSocketParametersTest.sh to jtreg java test [v4]

Matthew Donovan duke at openjdk.org
Mon Feb 27 14:04:49 UTC 2023


On Tue, 21 Feb 2023 19:06:02 GMT, Stuart Marks <smarks at openjdk.org> wrote:

>> Matthew Donovan 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 six additional commits since the last revision:
>> 
>>  - added System.exit when exceptions are thrown and refactored for clarity
>>  - Merge branch 'master' into rmi-sslparams
>>  - added default switch case and additional refactoring
>>  - added exceptions for cases 4 and 5
>>  - clarified cases 4 and 5
>>  - 8298939: Refactor open/test/jdk/javax/rmi/ssl/SSLSocketParametersTest.sh to jtreg java test
>
> test/jdk/javax/rmi/ssl/SSLSocketParametersTest.java line 221:
> 
>> 219:             }
>> 220:         }
>> 221:     }
> 
> Overall this test method makes sense, as it asserts that an IllegalArgumentException must be thrown with the given exception message. (A comment to this effect would be helpful.) As such, the catch of NoSuchAlgorithmException is confusing. I think this is here because SSLContext.getDefault() lists this as a checked exception. If so, then I'd suggest simply propagating it by adding a `throws` clause for this method. This also means adding a `throws` clause to the `runTest()` method, possibly just `throws Exception`. In general this is OK for jtreg tests to propagate any checked exception out of main, since jtreg will handle and report any unexpectedly propagated exceptions. It's thus unnecessary for test code to have catch-clauses for anything that the test isn't actually handling.

The exception handling has been cleaned up here and the other place you pointed out.

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

PR: https://git.openjdk.org/jdk/pull/11910


More information about the core-libs-dev mailing list