RFR: 8265828: [TestBug] Save and restore the default Locale in javafx.base unit test LocalDateTimeStringConverterTest [v4]

Andy Goryachev angorya at openjdk.org
Mon Nov 28 16:14:42 UTC 2022


On Mon, 28 Nov 2022 14:56:49 GMT, Lukasz Kostyra <duke at 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 incrementally with one additional commit since the last revision:
> 
>   Remove trailing whitespaces

Changes requested by angorya (Author).

modules/javafx.base/src/test/java/test/javafx/util/converter/LocalDateTimeStringConverterTest.java line 156:

> 154: 
> 155:     @After
> 156:     public void teardown() {

is this method empty?  can it be removed?

modules/javafx.base/src/test/java/test/javafx/util/converter/LocalTimeStringConverterTest.java line 131:

> 129:                 this.converter = new LocalTimeStringConverter();
> 130:                 this.locale = Locale.getDefault(Locale.Category.FORMAT);
> 131:                 // this.formatter and this.parser remain null

I think it would be better to set these to null explicitly here and WITH_FORMAT_STYLES.
this way you don't need to rely on some far away code and assumptions.

modules/javafx.base/src/test/java/test/javafx/util/converter/LocalTimeStringConverterTest.java line 151:

> 149: 
> 150:     @After
> 151:     public void teardown() {

can this method be removed?

-------------

PR: https://git.openjdk.org/jfx/pull/954


More information about the openjfx-dev mailing list