RFR: 8306137: Open source several AWT ScrollPane related tests [v4]
Tejesh R
tr at openjdk.org
Wed May 3 04:37:31 UTC 2023
On Thu, 27 Apr 2023 12:13:57 GMT, Alexey Ivanov <aivanov at openjdk.org> wrote:
>> 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.
Updated.
> 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();
Updated.
> 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.
Updated.
> 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`.
Updated.
> test/jdk/java/awt/ScrollPane/ScrollPaneRemoveAdd.java line 138:
>
>> 136: waiting--;
>> 137: }
>> 138: }
>
> This method is never used, it can safely be dropped.
Updated.
> 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.
Updated.
> 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());
Updated.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13621#discussion_r1183230264
PR Review Comment: https://git.openjdk.org/jdk/pull/13621#discussion_r1183229539
PR Review Comment: https://git.openjdk.org/jdk/pull/13621#discussion_r1183230178
PR Review Comment: https://git.openjdk.org/jdk/pull/13621#discussion_r1183230201
PR Review Comment: https://git.openjdk.org/jdk/pull/13621#discussion_r1183229514
PR Review Comment: https://git.openjdk.org/jdk/pull/13621#discussion_r1183229628
PR Review Comment: https://git.openjdk.org/jdk/pull/13621#discussion_r1183229572
More information about the client-libs-dev
mailing list