On Wed, 26 Apr 2023 14:40:53 GMT, Tejesh R <t...@openjdk.org> wrote:
>> Open source few AWT ScrollPane related tests.
>
> Tejesh R has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Updated ScrollPaneWindowsTest.java
test/jdk/java/awt/ScrollPane/ComponentScrollTest.java line 80:
> 78: Thread.sleep(5000);
> 79: } catch (InterruptedException ie) {
> 80: }
This `sleep` blocks EDT. Is it intended?
test/jdk/java/awt/ScrollPane/ScrollPaneRemoveAdd.java line 87:
> 85:
> 86: Point buttonLoc = button.getLocationOnScreen();
> 87: Dimension buttonSize = button.getSize();
Both `getLocationOnScreen` and `getSize` should also be called on EDT?
test/jdk/java/awt/ScrollPane/ScrollPaneWindowsTest.java line 53:
> 51: Frame frame;
> 52: Insets paneInsets;
> 53: public static Object LOCK = new Object();
The lock object should be `final`.
test/jdk/java/awt/ScrollPane/ScrollPaneWindowsTest.java line 62:
> 60: System.out.println("This test is for Windows 2000 and
> above.");
> 61: return;
> 62: }
Shall this be dropped too? Java won't start on Windows versions before Vista
(6.0), so does it serve a purpose other than a historical reasons?
test/jdk/java/awt/ScrollPane/ScrollPaneWindowsTest.java line 70:
> 68: public void init() throws Exception {
> 69: EventQueue.invokeAndWait(() -> {
> 70: frame = new Frame();
Suggestion:
frame = new Frame("ScrollPaneWindowsTest");
test/jdk/java/awt/ScrollPane/ScrollPaneWindowsTest.java line 81:
> 79: hScroll = (ScrollPaneAdjustable) sp.getHAdjustable();
> 80: vScroll.addAdjustmentListener(this);
> 81: hScroll.addAdjustmentListener(this);
ScrollPane should be created inside `invokeAndWait` block, listeners should be
added on EDT too.
test/jdk/java/awt/ScrollPane/ScrollPositionIntact.java line 31:
> 29: */
> 30:
> 31: import java.awt.*;
Avoid wildcard imports.
test/jdk/java/awt/ScrollPane/ScrollPositionIntact.java line 61:
> 59: pa.add("East", new Label("East", Label.RIGHT));
> 60: sp.add(pa);
> 61: frame = new Frame();
Suggestion:
frame = new Frame("ScrollPositionIntact");
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13621#discussion_r1177963376
PR Review Comment: https://git.openjdk.org/jdk/pull/13621#discussion_r1177969918
PR Review Comment: https://git.openjdk.org/jdk/pull/13621#discussion_r1177971770
PR Review Comment: https://git.openjdk.org/jdk/pull/13621#discussion_r1177974445
PR Review Comment: https://git.openjdk.org/jdk/pull/13621#discussion_r1177977077
PR Review Comment: https://git.openjdk.org/jdk/pull/13621#discussion_r1177976182
PR Review Comment: https://git.openjdk.org/jdk/pull/13621#discussion_r1177979671
PR Review Comment: https://git.openjdk.org/jdk/pull/13621#discussion_r1177980405
