RFR: 8310049: Refactor Charset tests to use JUnit [v3]
Justin Lu
jlu at openjdk.org
Thu Jun 15 23:09:24 UTC 2023
On Thu, 15 Jun 2023 21:58:56 GMT, Naoto Sato <naoto at openjdk.org> wrote:
>> Justin Lu has updated the pull request incrementally with eight additional commits since the last revision:
>>
>> - Review: Clarify RegisteredCharsets.java and add comments to test methods
>> - Minor cleanup
>> - Refactor IllegalCharsetName.java to use method source
>> - Update EncDec.java to be more informative + cautious
>> - Update data source: other -> standard Charsets
>> - Review: Add comments to Contains.java to explain each test
>> - Review: Add comment for test in AvailableCharsetNames.java
>> - Review: Make CharsetContainmentTest.java data source and test method more clear
>
> test/jdk/java/nio/charset/Charset/Contains.java line 51:
>
>> 49: @ParameterizedTest
>> 50: @MethodSource("standardCharsets")
>> 51: public void standardCharsetsTest(Charset containerCs, Charset cs, boolean cont){
>
> `ISO-8859-15` and `CP1252` are not `StandardCharsets` so renaming the method to standardCharsetsTest seems incorrect.
Did not realize, thank you. I simply renamed it to charsets (as I am not sure if those tested are supposed to represent a particular group, or if they are just random).
> test/jdk/java/nio/charset/Charset/IllegalCharsetName.java line 51:
>
>> 49: assertThrows(UnsupportedCharsetException.class,
>> 50: () -> Charset.forName("default"));
>> 51: }
>
> Seems incorrect to merge this test into `IllegalCharsetName.java` because it is throwing `UnsupportedCharsetException`.
I moved it back to RegisteredCharsets.java
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/14500#discussion_r1231612571
PR Review Comment: https://git.openjdk.org/jdk/pull/14500#discussion_r1231612904
More information about the nio-dev
mailing list