RFR: 8364363: Modify some manual test instructions [v3]

Alexey Ivanov aivanov at openjdk.org
Wed Aug 27 20:15:46 UTC 2025


On Tue, 26 Aug 2025 22:41:30 GMT, Damon Nguyen <dnguyen at 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


More information about the client-libs-dev mailing list