On Mon, 28 Nov 2022 14:44:17 GMT, Lukasz Kostyra <d...@openjdk.org> wrote:
>> The change moves Locale setting in the test to `@BeforeClass` and >> `@AfterClass` calls. `@BeforeClass` method call stores current default VM >> locale and applies Locale.US, while `@AfterClass` method restores old VM >> locale after all tests are completed. >> >> I tested it both on Mac and Windows, in both cases Locale is changed, >> restored properly and tests pass. > > 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 54: > 52: @RunWith(Parameterized.class) > 53: public class LocalDateStringConverterTest { > 54: private static final LocalDate VALID_DATE = LocalDate.of(1985, 1, 12); I presume that this is not locale sensitive? modules/javafx.base/src/test/java/test/javafx/util/converter/LocalDateStringConverterTest.java line 123: > 121: @Before > 122: public void setup() { > 123: if (this.converter == null) { There is no need to check for `null` here. It always will be, since `converter` is initialized to `null` in the constructor and there is no other assignment. 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. 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). ------------- PR: https://git.openjdk.org/jfx/pull/954
