On Tue, 26 Aug 2025 22:41:30 GMT, Damon Nguyen <dngu...@openjdk.org> wrote:
>> When testing jtreg manual tests, some tests had unclear instructions. This
>> PR is an attempt at updating these tests for clarity.
>>
>> `MouseDraggedOriginatedByScrollBarTest.java` works as expected when compared
>> to native apps and outputs drag events even when the mouse pointer is
>> dragged off of the scrollbar and window altogether. Events should still
>> fire, but the previous instructions may make this confusing since it reads
>> as if no events should be output to the textarea at all.
>>
>> `TextAreaAppendScrollTest2.java` seems to not work when testing with the
>> previous implementation of programmatically appending strings to the
>> textarea. When I scroll down using the down arrow key, none of the text
>> below would be visible when the textarea is scrolled down. However, it would
>> show if I pressed `ENTER` to create a new line or manually modify the text
>> in the textarea first. So instead, I have implemented the original reported
>> approach to the test of adding a button to append a string to the textarea
>> to test for automatic scrolling when the word is wrapped to a new line.
>
> Damon Nguyen has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Review comment update
Now that there's only one test left in the PR, the subject doesn't correspond
to the content. You're no longer updating instructions for manual tests, you're
*automating one manual test*.
test/jdk/java/awt/List/MouseDraggedOriginatedByScrollBarTest.java line 124:
> 122: } catch (Exception ex) {
> 123: throw new RuntimeException("Can't create robot");
> 124: }
Suggestion:
Robot robot = new Robot();
The `test` method has the `throws` clause with `Exception`, just let
`AWTException` propagate.
test/jdk/java/awt/List/MouseDraggedOriginatedByScrollBarTest.java line 129:
> 127: robot.setAutoWaitForIdle(true);
> 128:
> 129: // Focus default button and wait till it gets focus
The comment is confusing, it doesn't correspond to the code below.
test/jdk/java/awt/List/MouseDraggedOriginatedByScrollBarTest.java line 132:
> 130: EventQueue.invokeAndWait(() -> {
> 131: loc = list.getLocationOnScreen();
> 132: width = list.getWidth();
Suggestion:
Point p = list.getLocationOnScreen();
p.translate(list.getWidth(), 20);
loc = p;
Then `width` becomes redundant.
test/jdk/java/awt/List/MouseDraggedOriginatedByScrollBarTest.java line 139:
> 137: Point p = MouseInfo.getPointerInfo().getLocation();
> 138: robot.mouseMove(p.x, p.y + 1);
> 139: }
Suggestion:
for (int i = 0; i < 30; i++) {
robot.mouseMove(loc.x, loc.y + 1);
}
You may copy `loc` into a non-volatile local variable (micro-optimisation).
-------------
Changes requested by aivanov (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/26636#pullrequestreview-3161585029
PR Review Comment: https://git.openjdk.org/jdk/pull/26636#discussion_r2305180802
PR Review Comment: https://git.openjdk.org/jdk/pull/26636#discussion_r2305195021
PR Review Comment: https://git.openjdk.org/jdk/pull/26636#discussion_r2305187870
PR Review Comment: https://git.openjdk.org/jdk/pull/26636#discussion_r2305193785
