On Sun, 16 Jan 2022 22:01:33 GMT, Nir Lisker <nlis...@openjdk.org> wrote:
> * Most tests that I have seen in JavaFX use the assert overloads that include
> a message that explains what the value should be or what it means if the
> assertion failed. I don't know how much of a requirement it is. I can help
> write these if you want.
I'm a bit at a loss there, many of the asserts are think are self explanatory
already, and a text describing the events that led to this assert would
basically describe the same as the path of all `@Nested` classes + method name.
For example:
- ObservableFluentBindingsTest
- When_map_Called
- WithNotNullReturns_ObservableValue_Which
- When_orElse_CalledReturns_ObservableValue_Which
- WhenObserved
- ShouldApplyMapThenOrElseOperation
I've however cleaned up the entire test using different values that more
clearly show what is happening (like `Left` after map becomes `Left+map` and
after a second map it becomes `Left+map+map2`). For `orElse` I use `Empty` now
to make it more clear. I also added `assertObserved` and
`assertNothingIsObserved` methods to get rid of the ugly checking of the
`values` list to see what the test listener was observing.
Please let me know if that helps.
> * There is a utility class `JMemoryBuddy` that was added to JavaFX to test
> for memory leaks. This would have been helpful in the GC tests, but it seems
> that the class is not suited too well for this case (it requires you to
> `null` you own reference etc.). I think the way you did it is fine, but maybe
> that class should be updated (in a different patch) to be more welcoming for
> these checks.
I've applied it in the helper class I was using.
> I would also add tests of chaining the observables in common use cases. I
> wrote some myself to test it for `map`:
I added additional nested classes for Map which adds another nesting that
applies another Map or an OrElse. Same for FlatMap.
I see the fluent binding test mostly as a test to ensure the basic
functionality and to ensure things are getting garbage collected when they
should and not when they shouldn't. If we want to test more specific cases
without having to worry about GC, perhaps another test class is better suited.
> You can clean these up and use them or write your own. `flatMap` should also
> have one. I can clean mine up and post it if it helps (I did some dirty
> testing there).
That's good, I hope you didn't find anything surprising :)
-------------
PR: https://git.openjdk.java.net/jfx/pull/675
