On Mon, 25 Mar 2024 16:40:56 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
>> This test has failed once and we are not seeing its failure after that
>> instance in our test systems.
>>
>> This test verifies whether bounds of GridPane gets updated properly on
>> adding an invisible node.
>> Initial test has 8 nodes in GridPane and then we update it with another node
>> with larger bounds without making it visible. After adding additional node
>> we make the scenegraph visible and check for colors of the newly added node.
>>
>> We are making this test robust to make sure we don't see any of these single
>> instance failures.
>> Test is updated to:
>> 1) To always show on top, so that we pick proper color.
>> 2) Add additional sleep so that we give more time for test to be visible.
>> 3) Pick color exactly at mid point in y axis, so that we pick the green
>> color properly.
>
> tests/system/src/test/java/test/robot/scenegraph/JDK8130122Test.java line 102:
>
>> 100: horizontalListView.setVisible(true);
>> 101: });
>> 102: sleep(1000);
>
> we still have JDK-8176902 open, but perhaps here we could add a new method to
> VisualTestBase similar to waitNextFrame(), with an appropriate parameter?
>
> Something like
>
> protected void waitForIdle() {
> frameWait(100);
> }
>
>
> instead of 1 second sleep() ? What do you think?
I think waitForIdle() would not be a proper name for this use case.
And we already have waitFirstFrame() which does frameWait(100).
What i will do is, use waitFirstFrame() and add comment in test case that we
are waiting after setVisible(true).
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1433#discussion_r1538649261
