On Wed, 10 Aug 2022 06:56:50 GMT, Ajit Ghaisas <[email protected]> wrote:
> A null check has been added to below SpinnerValueFactory classes in
> valueProperty() ChangeListeners-
> - `IntegerSpinnerValueFactory`
> - `LocalDateSpinnerValueFactory`
> - `LocalTimeSpinnerValueFactory`
> - `ListSpinnerValueFactory`
>
> Added 5 tests that detect NPE for above 4 Spinner factories.
> Only `DoubleSpinnerValueFactory` already had a valid null check - so the test
> for it passes before and after.
Fix looks good and follows the same pattern as in `DoubleSpinnerValueFactory`.
Left one minor suggestion for all added tests.
modules/javafx.controls/src/test/java/test/javafx/scene/control/SpinnerTest.java
line 1498:
> 1496: @Test public void testSetValueNull_IntegerSpinner() {
> 1497: intValueFactory.setValue(null);
> 1498: assertEquals(null, intSpinner.getValue());
All the tests can be simplified to just use `assertNull()` instead. ->
`assertNull(intSpinner.getValue());`
-------------
Marked as reviewed by mhanl (Committer).
PR: https://git.openjdk.org/jfx/pull/865
