On Mon, 16 Jun 2025 17:44:48 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
>> Currently, the VirtualFlowTestUtils used ONLY for tests has many utility
>> methods to get and do something with the VirtualFlow of Virtualized Controls.
>>
>> It has one flaw: It may creates a temporary Stage with the StageLoader, when
>> the Control was never inside a Scene (and Stage) yet. This is done to get
>> the VirtualFlow, which is not possible otherwise.
>>
>> Take the following test code:
>>
>> VirtualFlowTestUtils.clickOnRow(tableView, 2);
>> VirtualFlowTestUtils.clickOnRow(tableView, 4);
>>
>>
>> What it does it to create a Stage for the first method and dispose it after.
>> And create another Stage for the second method and dispose it after.
>>
>> This does not test a realistic scenario and the chance is quite high that
>> developers used that methods without even knowing that it contains such
>> logic.
>>
>> So the idea is to remove the StageLoader code from VirtualFlowTestUtils and
>> rather use it in the Test code before calling the Util methods.
>>
>> For the example above, this would result in:
>>
>> stageLoader = new StageLoader(tableView);
>> VirtualFlowTestUtils.clickOnRow(tableView, 2);
>> VirtualFlowTestUtils.clickOnRow(tableView, 4);
>>
>>
>> The stageLoader should be disposed in an @AfterEach.
>>
>> Note: Nearly all touched tests are indeed much older test code. New tests
>> are not affected and already use it correcty.
>> Sometimes a call to `Toolkit.getToolkit().firePulse();` was needed.
>> Previously, creating a new Stage for every call to the util methods did that
>> implicitly.
>
> modules/javafx.controls/src/test/java/test/com/sun/javafx/scene/control/infrastructure/VirtualFlowTestUtils.java
> line 349:
>
>> 347: flow = (VirtualFlow<?>)control.lookup("#virtual-flow");
>> 348:
>> 349: if (sl != null) {
>
> should we have a check here with a helpful message (which recommends to use
> `StageLoader`), instead of returning `null`?
I thought about this as well. No strong opinion from my side.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1829#discussion_r2154281361
