On Wed, 8 Jan 2025 19:40:29 GMT, Alexey Ivanov <aiva...@openjdk.org> wrote:
> **Problem:**
>
> The `javax/swing/JRadioButton/ButtonGroupFocus/ButtonGroupFocusTest.java`
> test fails in CI on Linux.
>
> The focus is on _Button 2_ instead of _Button 4_.
>
> **Root cause:**
>
> The additional logging revealed, an expected `KEY_PRESS` event. The test uses
> `Robot` API to press the <kbd>Tab</kbd> key and release it. When the test
> fails, there are two `KEY_PRESS` events followed by a single `KEY_RELEASE`
> event. Because the <kbd>Tab</kbd> key is pressed twice, the focus moves
> twice: from _Button 2_ (the initial state) to _Button 4_ and then
> back to _Button 2_.
>
> **Fix:**
>
> Use `CountDownLatch`es to track whether a radio button received focus. Detect
> the case where two `KEY_PRESS` events moved focus to _Button 2_ and
> report the failure.
>
> Log focus movements and dispatched key events to facilitate failure analysis.
>
> Take a screenshot of the test frame in case of failure.
>
> These CI hosts seem to be quite slow, removing the delay added by
> `robot.setAutoDelay(100)` has reduced the number of unexpected `KEY_PRESS`
> events. This didn't affect Windows or macOS.
Tested this on Windows and Linux locally and on CI. Logging looks good and the
test passes with repeats. LGTM
test/jdk/javax/swing/JRadioButton/ButtonGroupFocus/ButtonGroupFocusTest.java
line 121:
> 119: + "\n\t" +
> evt.getNewValue());
> 120: }
> 121: });
Suggestion:
evt -> System.out.println(evt.getPropertyName()
+ "\n\t" + evt.getOldValue()
+ "\n\t" + evt.getNewValue()));
Nit. Possible lambda replacement here.
test/jdk/javax/swing/JRadioButton/ButtonGroupFocus/ButtonGroupFocusTest.java
line 124:
> 122:
> 123: // ...and dispatched key events
> 124: Toolkit.getDefaultToolkit().addAWTEventListener(new
> AWTEventListener() {
The `AWTEventListener` can also be replaced with a lambda if preferred.
test/jdk/javax/swing/JRadioButton/ButtonGroupFocus/ButtonGroupFocusTest.java
line 172:
> 170: BufferedImage image =
> robot.createScreenCapture(getFrameBounds());
> 171: ImageIO.write(image, "png",
> 172: new File("image.png"));
Does this overflow the 80 character line limit? Or is this a standard?
-------------
Marked as reviewed by dnguyen (Committer).
PR Review: https://git.openjdk.org/jdk/pull/22977#pullrequestreview-2540703533
PR Review Comment: https://git.openjdk.org/jdk/pull/22977#discussion_r1909302224
PR Review Comment: https://git.openjdk.org/jdk/pull/22977#discussion_r1909303094
PR Review Comment: https://git.openjdk.org/jdk/pull/22977#discussion_r1909297406
