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

Alexey Ivanov aivanov at openjdk.org
Thu Apr 27 12:52:23 UTC 2023


On Thu, 27 Apr 2023 06:28:23 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 based on review comments

test/jdk/java/awt/ScrollPane/ComponentScrollTest.java line 47:

> 45:     public ScrollPane scrollpane;
> 46:     public Frame frame;
> 47:     public int count = 0;

Suggestion:

    public volatile int count = 0;

`count` is accessed from two threads, it should be `volatile` or you should use `AtomicInteger` instead.

test/jdk/java/awt/ScrollPane/ScrollPaneLimitation.java line 53:

> 51:     public static boolean mouseWasPressed = false;
> 52:     public static Component child = null;
> 53:     private Object lock = new Object();

Suggestion:

    private final Object lock = new Object();

test/jdk/java/awt/ScrollPane/ScrollPaneLimitation.java line 83:

> 81:                         if (e.getID() == MouseEvent.MOUSE_PRESSED) {
> 82:                             if (e.getSource() == ScrollPaneLimitation.child
> 83:                                     && e.getY() > SCROLL_POS) {

The two `if` blocks could be merged together into one.

test/jdk/java/awt/ScrollPane/ScrollPaneLimitation.java line 145:

> 143:                     throw new RuntimeException("test was interrupted");
> 144:                 }
> 145:             }

A `CountDownLatch` would be better rather than plain `lock`.

test/jdk/java/awt/ScrollPane/ScrollPaneRemoveAdd.java line 138:

> 136:                 waiting--;
> 137:             }
> 138:         }

This method is never used, it can safely be dropped.

test/jdk/java/awt/ScrollPane/ScrollPaneWindowsTest.java line 77:

> 75:         hScroll = (ScrollPaneAdjustable) sp.getHAdjustable();
> 76:         vScroll.addAdjustmentListener(this);
> 77:         hScroll.addAdjustmentListener(this);

`vScroll` and `hScroll` should also go into `invokeAndWait` above.

test/jdk/java/awt/ScrollPane/ScrollPositionIntact.java line 85:

> 83: 
> 84:                 int i;
> 85:                 i = (int) (sp.getScrollPosition().getX());

Suggestion:

                int i = (int) (sp.getScrollPosition().getX());

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13621#discussion_r1179060375
PR Review Comment: https://git.openjdk.org/jdk/pull/13621#discussion_r1179087661
PR Review Comment: https://git.openjdk.org/jdk/pull/13621#discussion_r1179062469
PR Review Comment: https://git.openjdk.org/jdk/pull/13621#discussion_r1179065937
PR Review Comment: https://git.openjdk.org/jdk/pull/13621#discussion_r1179096166
PR Review Comment: https://git.openjdk.org/jdk/pull/13621#discussion_r1179076672
PR Review Comment: https://git.openjdk.org/jdk/pull/13621#discussion_r1179084512



More information about the client-libs-dev mailing list