On Wed, 11 Mar 2026 08:00:37 GMT, Ambarish Rapte <[email protected]> wrote:
>> Jose Pereda has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Address feedback from reviewer
>
> tests/system/src/test/java/test/robot/javafx/scene/SystemMenuBarEnableTest.java
> line 102:
>
>> 100: // Select the second item via keyboard
>> 101: Util.runAndWait(() -> {
>> 102: Robot robot = new Robot();
>
> `robot` can be a member variable of class `SystemMenuBarEnableTest`,
> initialized in `TestApp.start()` method and reused instead of re-creating.
Done
> tests/system/src/test/java/test/robot/javafx/scene/SystemMenuBarEnableTest.java
> line 105:
>
>> 103: robot.keyType(KeyCode.DOWN);
>> 104: robot.keyType(KeyCode.DOWN);
>> 105: System.out.println("Pressed DOWN+DOWN+ENTER, menu
>> should have selected second item");
>
> May be remove these print statements. the comment before the block are
> sufficient, and the prints are only noise in test report.
Done, those were left overs.
> tests/system/src/test/java/test/robot/javafx/scene/SystemMenuBarEnableTest.java
> line 151:
>
>> 149:
>> 150: final AtomicBoolean enabledItemFired = new AtomicBoolean(false);
>> 151: final AtomicBoolean disabledItemFired = new
>> AtomicBoolean(false);
>
> General pattern we have in other tests is that, such variables are declared
> as member of the public enclosing class. It would avoid the need of TestApp
> object reference for using these variables. I would recommend to move them,
> it would maintain similarity with existing tests.
Done
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/2103#discussion_r2917085458
PR Review Comment: https://git.openjdk.org/jfx/pull/2103#discussion_r2917087072
PR Review Comment: https://git.openjdk.org/jfx/pull/2103#discussion_r2917087933
