On Fri, 29 Sep 2023 17:30:42 GMT, Naoto Sato <na...@openjdk.org> wrote:
>> Justin Lu has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> cleanup util classes in text/testlib
>
> test/jdk/java/text/BreakIterator/BreakIteratorTest.java line 112:
>
>> 110: fail(message);
>> 111: }
>> 112:
>
> We could simply eliminate these, by making `compareFragmentLists` return
> success indicator.
I think the previous code was set up so that if forward and backward did not
pass, than expected and actual was skipped. Since IntlTest could be ran with
-nothrow which allowed for tests to continue even if an error was encountered.
Since that is no longer the case, I simply adjusted `compareFragmentLists` to
fail if an error is found.
> test/jdk/java/text/testlib/CollatorTestUtils.java line 32:
>
>> 30: * Collator related tests can use.
>> 31: */
>> 32: public final class CollatorTestUtils {
>
> Can the utilities in this class be merged into `TestUtils`?
Merged CollatorTestUtils into TestUtils. I adjusted some method names which
were previously in CollatorTestUtils to make more sense, now that they are in
TestUtils.
> test/jdk/java/util/TimeZone/TimeZoneBoundaryTest.java line 343:
>
>> 341: if (inDaylight != lastDST)
>> 342: {
>> 343: System.out.println("Switch " + (inDaylight ? "into" : "out of")
>
> I'd remove this commented-out code entirely.
Removed this and the other occurrence you spotted.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/15954#discussion_r1341801084
PR Review Comment: https://git.openjdk.org/jdk/pull/15954#discussion_r1341801193
PR Review Comment: https://git.openjdk.org/jdk/pull/15954#discussion_r1341801258
