On Fri, 28 Jun 2024 05:17:41 GMT, Ambarish Rapte <[email protected]> wrote:
>> eduardsdv has updated the pull request incrementally with three additional
>> commits since the last revision:
>>
>> - JDK-8322619: Fix waiting for the stage
>> - JDK-8322619: Improve output message in test
>> - JDK-8322619: Avoid using of Thread.sleep(..)
>
> tests/system/src/test/java/test/com/sun/prism/impl/NGNodeDirtyFlagTest.java
> line 68:
>
>> 66: primaryStage.show();
>> 67:
>> 68: launchLatch.countDown();
>
> This countDown() should be called after stage is shown to ensure the stage
> shown before proceeding.
> `primaryStage.setOnShown(e -> Platform.runLater(launchLatch::countDown));`
Done
> tests/system/src/test/java/test/com/sun/prism/impl/NGNodeDirtyFlagTest.java
> line 75:
>
>> 73: public static void setupOnce() {
>> 74: Util.launch(launchLatch, MyApp.class);
>> 75: assertEquals(0, launchLatch.getCount());
>
> Util.launch() asserts if launchLatch.countDown() does not occur in 15
> seconds, so this line can be removed.
Done
> tests/system/src/test/java/test/com/sun/prism/impl/NGNodeDirtyFlagTest.java
> line 90:
>
>> 88: StackPane root = myApp.root;
>> 89:
>> 90: runAndWait(() -> {
>
> All such calls can be changed to use `Util.runAndWait()`
Done
> tests/system/src/test/java/test/com/sun/prism/impl/NGNodeDirtyFlagTest.java
> line 103:
>
>> 101: });
>> 102:
>> 103: Thread.sleep(500);
>
> Could use `Util.waitForIdle(Scene)` instead of sleep. It would reduce the
> test time and should work. We can consider a sleep if
> `Util.waitForIdle(Scene)` does not work. Similarly please see if other sleep
> calls can be removed.
Done
> tests/system/src/test/java/test/com/sun/prism/impl/NGNodeDirtyFlagTest.java
> line 149:
>
>> 147: throw new RuntimeException(e);
>> 148: }
>> 149: }
>
> Please use the `Util.runAndWait()` instead of adding this method. So this
> method can be removed and all calls can be changed to Util.runAndWait().
> Please do remove any imports that become unused afterwards.
Done
> tests/system/src/test/java/test/com/sun/prism/impl/NGNodeDirtyFlagTest.java
> line 188:
>
>> 186: return result;
>> 187: }
>> 188: }
>
> Please add an empty line at the end.
Done
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1451#discussion_r1658440382
PR Review Comment: https://git.openjdk.org/jfx/pull/1451#discussion_r1658440585
PR Review Comment: https://git.openjdk.org/jfx/pull/1451#discussion_r1658444155
PR Review Comment: https://git.openjdk.org/jfx/pull/1451#discussion_r1658444269
PR Review Comment: https://git.openjdk.org/jfx/pull/1451#discussion_r1658443932
PR Review Comment: https://git.openjdk.org/jfx/pull/1451#discussion_r1658443599
