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

Lukasz Kostyra duke at openjdk.org
Tue Nov 29 10:27:04 UTC 2022


On Mon, 28 Nov 2022 14:51:40 GMT, Kevin Rushforth <kcr at 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


More information about the openjfx-dev mailing list