RFR: 8306137: Open source several AWT ScrollPane related tests [v3]

Alexey Ivanov aivanov at openjdk.org
Wed Apr 26 14:46:58 UTC 2023


On Wed, 26 Apr 2023 14:40:53 GMT, Tejesh R <tr at 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



More information about the client-libs-dev mailing list