On Sun, 30 Oct 2022 12:57:38 GMT, Alexey Ivanov <aiva...@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
