On Wed, 3 Jul 2024 14:55:16 GMT, Kevin Rushforth <k...@openjdk.org> wrote:
>> eduardsdv has updated the pull request with a new target base due to a merge >> or a rebase. The incremental webrev excludes the unrelated changes brought >> in by the merge/rebase. The pull request contains five additional commits >> since the last revision: >> >> - Merge remote-tracking branch 'refs/remotes/origin/master' into >> bugfix/JDK-8322619-render-dirty-flag >> - JDK-8322619: Fix waiting for the stage >> - JDK-8322619: Improve output message in test >> - JDK-8322619: Avoid using of Thread.sleep(..) >> - JDK-8322619: Combine clearDirtyTree() and clearDirty() methods. > > tests/system/src/test/java/test/com/sun/prism/impl/NGNodeDirtyFlagTest.java > line 1: > >> 1: package test.com.sun.prism.impl; > > This needs a standard Copyright header. > > Also, all tests that use Robot _must_ be under the `test.robot.**` package > hierarchy. Please move the test and change the package. I moved it to the ``test.robot.com.sun.prism`` package and added the copyright header. Now the test needs an additional parameter in the start command **-PUSE_ROBOT=true**: ``gradlew -PFULL_TEST=true -PUSE_ROBOT=true :systemTests:test --tests NGNodeDirtyFlagTest`` > tests/system/src/test/java/test/com/sun/prism/impl/NGNodeDirtyFlagTest.java > line 135: > >> 133: Bounds screenBounds = >> node.localToScreen(node.getBoundsInLocal()); >> 134: WritableImage image = robot.getScreenCapture(null, >> screenBounds.getMinX(), screenBounds.getMinY(), 100, 100); >> 135: Assert.assertEquals("A node was not rendered properly. Wrong >> color found", name(expected), name(image.getPixelReader().getColor(1, 1))); > > This method of color comparison seems convoluted and error prone. I recommend > using one of the existing color comparison methods with toloerance. See > VisualTestBase, for example. The test now uses ``VisualTestBase.assertColorEquals(Color expected, Color actual, double delta)`` to compare colors. ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1451#discussion_r1664469982 PR Review Comment: https://git.openjdk.org/jfx/pull/1451#discussion_r1664474502