[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-sound-dev
Subject:    Re: RFR: 8295774 : Write a test to verify that List Item selection events.
From:       Alexey Ivanov <aivanov () openjdk ! org>
Date:       2022-10-30 13:06:35
Message-ID: gPnzVicNzZjmhmaahUgdcFi-aA0uAUVdK4ktC7fAUo0=.d5e33610-b2d3-4f76-a2d7-e0de54ec3386 () github ! com
[Download RAW message or body]

On Fri, 28 Oct 2022 09:01:53 GMT, ravi gupta <duke@openjdk.org> wrote:

> This testcase verify that List Item selection via mouse/keys generates \
> ItemEvent/ActionEvent appropriately. 
> a. Single click on the list generate ItemEvent and double click on item generates \
> ActionEvent. b. UP/DOWN keys on the list generate ItemEvent and enter key on item \
> generates ActionEvent. 
> Testing:
> Tested using Mach5(20 times per platform) in macos,linux and windows and got all \
> pass.

Changes requested by aivanov (Reviewer).

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`.

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.

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`.

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())


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.

test/jdk/java/awt/event/ComponentEvent/ListItemEventsTest.java line 140:

> 138:             }
> 139: 
> 140:             robot.setAutoDelay(waitDelay);

Why is delay increased here?

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.

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

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


[prev in list] [next in list] [prev in thread] [next in thread]