RFR: 8298939: Refactor open/test/jdk/javax/rmi/ssl/SSLSocketParametersTest.sh to jtreg java test [v4]
Stuart Marks
smarks at openjdk.org
Tue Feb 21 19:08:35 UTC 2023
On Fri, 17 Feb 2023 20:41:16 GMT, Matthew Donovan <duke at openjdk.org> wrote:
>> Removed SSLSocketParametersTest.sh script (which just called a Java file) and configured the java code to run directly with jtreg
>
> 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.
-------------
PR: https://git.openjdk.org/jdk/pull/11910
More information about the core-libs-dev
mailing list