RFR: 8274771: Map, FlatMap and OrElse fluent bindings for ObservableValue [v8]

John Hendrikx jhendrikx at openjdk.java.net
Thu Mar 10 05:36:49 UTC 2022


On Tue, 8 Mar 2022 21:10:46 GMT, Nir Lisker <nlisker at openjdk.org> wrote:

>> John Hendrikx has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Fix wrong test values
>
> modules/javafx.base/src/test/java/test/javafx/beans/value/ObservableValueFluentBindingsTest.java line 356:
> 
>> 354:             }
>> 355:         }
>> 356:         // TODO test for when something is flatMapped to null in getValue call
> 
> Is this still relevant? I think that you tested it in line 407 `shouldIgnoreFlatMapsToNull`.
> 
> What I would like to see is a test when `left`, `right` or `unknown`'s value is set to `null`. What I see is `property`'s value being set to `null`, or the reference of one of the above being set to `null`, but not the value itself. Only when you compose `orElse` on `flatMap` this case is tested (line 604).
> Is this what you meant?

I left that TODO in by accident after my clean up of the test case, I added the case then.  I've however also added one more case that you described above:

                @Test
                void shouldObserveNullWhenFlatMappedPropertyIsSetToNull() {
                    property.set("Right");

                    assertObserved("RIGHT");

                    property.set(null);

                    assertObserved((String)null);
                }

> modules/javafx.base/src/test/java/test/javafx/beans/value/ObservableValueFluentBindingsTest.java line 457:
> 
>> 455:                     right = null;
>> 456: 
>> 457:                     property.set("Right");
> 
> Isn't `unknown = null;` enough like you did previously? It doesn't really matter, just for consistency.

Just trying to be thorough on testing all the `null` cases

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

PR: https://git.openjdk.java.net/jfx/pull/675


More information about the openjfx-dev mailing list