On Thu, 13 Mar 2025 03:31:13 GMT, Nir Lisker <nlis...@openjdk.org> wrote:
>> John Hendrikx has updated the pull request incrementally with two additional >> commits since the last revision: >> >> - Change StackOverflowException to warning log >> - Support keeping last message in Logging helper > > modules/javafx.base/src/test/java/test/javafx/beans/ObservableValueTest.java > line 70: > >> 68: >> 69: /* >> 70: * ObservableValue cases to test: > > I would add that 2 values are provided to transition between for each > properties test. For bindings, a setter and a modifier are provided too. I > suggest that `Case`'s docs will outline what they are for. I've been re-reading my own test code (as it is I think 2 years old already), and I added more docs where it was unclear :) I added more docs here especially. > modules/javafx.base/src/test/java/test/javafx/beans/ObservableValueTest.java > line 669: > >> 667: * Takes a list of values and creates an iterator of pairs of those >> values. The iterator >> 668: * does not only return all possible pair combinations, but also >> different transitions between >> 669: * two pairs. Effectively, given x values it returns x^3 >> combinations. > > Very worth noting that the pairs are used for the numbers for invalidation > and change listeners. > > Also, there are duplicate pairs with the same pairs before and after, so it > seems to be repeating the same state. For example, the sequence > [1, 2] > [1, 0] > [1, 50] > repeats several times. Not sure if this is on purpose, but looks redundant. It was intended; the add/removal code is complex (as it may switch implementations to conserve memory) and we need to be absolutely sure that a simple action like adding or removing a listener (even of a different type) doesn't affect the system. In `ExpressionListener` there were bugs were adding or removing a listener could affect the next change being detected or not, and we'd want to make sure that can't happen. ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1081#discussion_r2134524537 PR Review Comment: https://git.openjdk.org/jfx/pull/1081#discussion_r2134524011
