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