RFR: 8295774 : Write a test to verify that List Item selection events. [v2]

ravi gupta duke at openjdk.org
Wed Nov 2 04:16:35 UTC 2022


On Sun, 30 Oct 2022 12:57:38 GMT, Alexey Ivanov <aivanov at openjdk.org> wrote:

>> ravi gupta has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   8295774 : Write a test to verify that List Item selection events.
>
> test/jdk/java/awt/event/ComponentEvent/ListItemEventsTest.java line 50:
> 
>> 48:     private static final int waitDelay = 1000;
>> 49: 
>> 50:     private volatile static Frame frame;
> 
> `frame` is used only from the EDT, thus it doesn't need to be `volatile`.

Resolved .

> test/jdk/java/awt/event/ComponentEvent/ListItemEventsTest.java line 54:
> 
>> 52:     private volatile static boolean actionPerformed = false;
>> 53:     private volatile static boolean itemStateChanged = false;
>> 54:     private volatile static Robot robot;
> 
> `robot` is used from the main thread only, therefore it doesn't need to be `volatile` either.

Resolved.

> test/jdk/java/awt/event/ComponentEvent/ListItemEventsTest.java line 55:
> 
>> 53:     private volatile static boolean itemStateChanged = false;
>> 54:     private volatile static Robot robot;
>> 55:     private volatile static CountDownLatch latch;
> 
> Should rather be `final` than `volatile`.

resolved . CountDownLatch removed from code.

> test/jdk/java/awt/event/ComponentEvent/ListItemEventsTest.java line 100:
> 
>> 98:             Point listAt = list.getLocationOnScreen();
>> 99:             // get bounds of button
>> 100:             Rectangle bounds = list.getBounds();
> 
> What button? Better remove the comment.
> 
> Since you need only the size, use [`getSize()`](https://docs.oracle.com/en/java/javase/17/docs/api/java.desktop/java/awt/Component.html#getSize())

Resolved now code modified as below 
 
Dimension listDimension = list.getSize();

> test/jdk/java/awt/event/ComponentEvent/ListItemEventsTest.java line 111:
> 
>> 109:                 throw new RuntimeException(
>> 110:                     "Fail: Timed out waiting for list to gain focus, test cannot proceed!!");
>> 111:             }
> 
> Since `list` is the only focusable component in the `frame`, it gets focus before `robot.waitForIdle()` returns. So, this along with `FocusListener` could be dropped.

Resolved . FocusListener dropped.

> test/jdk/java/awt/event/ComponentEvent/ListItemEventsTest.java line 140:
> 
>> 138:             }
>> 139: 
>> 140:             robot.setAutoDelay(waitDelay);
> 
> Why is delay increased here?

This test verifies list section via mouse/keys events.
 
For mouse events, robot.setAutoDelay(100) worked well. But for key events there were a few intermittent failures which  cleared out with a higher delay via robot.setAutoDelay(1000)

> test/jdk/java/awt/event/ComponentEvent/ListItemEventsTest.java line 189:
> 
>> 187:     }
>> 188: 
>> 189:     private static void keyType(int key) throws Exception {
> 
> Suggestion:
> 
>     private static void typeKey(int key) throws Exception {
> 
> Method names usually start with verbs.

Resolved . 
renamed as suggested

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

PR: https://git.openjdk.org/jdk/pull/10899



More information about the client-libs-dev mailing list