On Tue, 8 Mar 2022 21:10:46 GMT, Nir Lisker <[email protected]> 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
