On Thu, 11 Jun 2026 14:32:20 GMT, Christopher Schnick <[email protected]> wrote:
>> Okay, I wonder if we should either replace or rename the method in the test >> then. If it would just be called updateXXX and returns a display node, that >> will look better and is more honest about the side effect (because the PR >> before I was not aware of the side effect - so it already tricked me when >> reading the test) >> >> I think having 3 similar tests is not a problem, we have that a lot for >> tables as well. There is just no better way, many tests often require a >> similar setup. The advantage is that if some fail, you know exactly what is >> broken (unless they are called jdkXYZ or rtXYZ). > > I updated the comment, maybe that is better For me it's fine, but it probably makes sense to refine the tests in a follow up. The most simple thing is to rename (only in the test class) `getDisplayNode()` to `updateDisplayNode()`. And maybe we find a solution to make the tests smaller, if the initial setup is similar, we could extract the `ComboBox` setup. Example what I did in the past here: https://github.com/openjdk/jfx/blob/b1eccb6ed0d984ca3e854b6f46163d56bd8f84d1/modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/TableRowSkinTest.java#L298-L317 ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/2179#discussion_r3396827067
