On Fri, 9 Feb 2024 16:02:19 GMT, Andy Goryachev <ango...@openjdk.org> wrote:

>> Kevin Rushforth has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Address review feedback
>
> tests/system/src/test/java/test/robot/javafx/scene/TransparentLCDTest.java 
> line 69:
> 
>> 67:  * @bug 8311492
>> 68:  */
>> 69: public class TransparentLCDTest {
> 
> Was there a reason you did not use `VisualTestBase` class?
> The code is fine the way it is, I am just curious.

I started out using `VisualTestBase`, but I needed to set system properties 
before launching the FX runtime, and there is no way that I could find to 
insert any code into the subclass that would get executed before  the 
superclass.

The only thing I ended up using from `VisualTestBase` was the 
`assertColorEquals` method, which could be moved to Util anyway. There is other 
cleanup that could be done to `VisualTestBase`, so I'll file a follow-on bug.

> tests/system/src/test/java/test/robot/javafx/scene/TransparentLCDTest.java 
> line 190:
> 
>> 188:     @AfterEach
>> 189:     public void doTeardown() {
>> 190:         Platform.runLater(() -> {
> 
> should this be Util.runAndWait() ?

It doesn't really matter in this case since Runnables are run in order, meaning 
that the creation and showing of the stage for the next text is guaranteed to 
happen after the stage is hidden. I can change it, though.

-------------

PR Review Comment: https://git.openjdk.org/jfx/pull/1361#discussion_r1484526979
PR Review Comment: https://git.openjdk.org/jfx/pull/1361#discussion_r1484529935

Reply via email to