On Mon, 28 Nov 2022 14:51:40 GMT, Kevin Rushforth <k...@openjdk.org> wrote:
>> Lukasz Kostyra has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains four commits: >> >> - Merge branch 'master' of https://git.openjdk.org/jfx into >> JDK-8265828-locale >> - Refactor remaining LocalStringConverter tests >> >> Treatment done in this commit is similar to the previous change. >> - LocalDateTimeStringConverterTest: Refactor test to properly utilize Locale >> >> * Locale initialization was moved to @BeforeClass method. >> * DateTimeFormatter objects are allocated after Locale initialization >> * Since LocalDateTimeStringConverter depends on DateTimeFormatter and on >> VM's Locale, >> creation of it was moved to @Before method. >> - 8265828: [TestBug] Save and restore the default Locale in javafx.base >> unit test LocalDateTimeStringConverterTest > > modules/javafx.base/src/test/java/test/javafx/util/converter/LocalDateStringConverterTest.java > line 143: > >> 141: break; >> 142: default: >> 143: throw new InvalidParameterException("Invalid converter >> variant: " + this.converterVariant.toString()); > > This does not seem like the right exception to throw. I would just use > `fail(String)` here. Done > modules/javafx.base/src/test/java/test/javafx/util/converter/LocalDateStringConverterTest.java > line 150: > >> 148: @After >> 149: public void teardown() { >> 150: } > > Minor: Since this method is empty, I think you can remove it (along with the > import). Done ------------- PR: https://git.openjdk.org/jfx/pull/954
