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

[Alexey Ivanov]

On Tue, 25 Apr 2023 15:43:18 GMT, Tejesh R <tr at openjdk.org> wrote:

>> Open source few AWT ScrollPane related tests.
>
> Tejesh R has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - Updated based on Review Comments
>  - Updated based on Review Comments

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

> 46:     public Frame frame;
> 47:     public int count = 0;
> 48:     public static void main(String[] args) throws InterruptedException, InvocationTargetException {

Suggestion:

    public int count = 0;

    public static void main(String[] args)
            throws InterruptedException, InvocationTargetException {

Add a blank line between fields and method, wrap `throws` clause to fit into 80-column limit.

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

> 66:                 frame.add(scrollpane);
> 67:         });
> 68:         scrollpane.getVAdjustable().addAdjustmentListener(this);

The lambda expression has incorrect indentation.

You should add the listener on EDT too.

test/jdk/java/awt/ScrollPane/ScrollPaneExtraScrollBar.java line 31:

> 29:   shown for the first time with SCROLLBARS_AS_NEEDED style
> 30:   @key headful
> 31: */

In majority of cases, the tags have a leading asterisk.

test/jdk/java/awt/ScrollPane/ScrollPaneExtraScrollBar.java line 57:

> 55:         robot = new Robot();
> 56:         EventQueue.invokeAndWait(() -> {
> 57:             f = new Frame();

Suggestion:

            f = new Frame("ScrollPaneExtraScrollBar");

As suggested in other reviews, let's add a title to the frame with the name of the test.

test/jdk/java/awt/ScrollPane/ScrollPaneExtraScrollBar.java line 71:

> 69:             robot.delay(100);
> 70:             robot.waitForIdle();
> 71:             Rectangle r = f.getBounds();

You may want to call `getBounds()` on EDT.

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

> 75:             robot.mouseRelease(InputEvent.BUTTON1_DOWN_MASK);
> 76: 
> 77:             Thread.sleep(1000);

What you want here is
Suggestion:

            Robot.waitForIdle();

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

> 78:             Properties prop = System.getProperties();
> 79:             String os = prop.getProperty("os.name", "");
> 80:             if (os != null && (os.indexOf("98") == -1)) {

Shall we drop the filter for Windows 98? Java cannot be run on Windows 9x for quite a long time, so it doesn't make sense to preserve this condition.

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

> 93:             robot.mouseRelease(InputEvent.BUTTON1_DOWN_MASK);
> 94: 
> 95:    //         Toolkit.getDefaultToolkit().sync();

Instead of commenting out the line, remove it.

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

> 118:     private static class Semaphore {
> 119:         volatile boolean state = false;
> 120:         Object lock = new Object();

Since it's a lock object, it should be `final`.

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

> 119:         volatile boolean state = false;
> 120:         Object lock = new Object();
> 121:         volatile int waiting = 0;

Why are both `state` and `waiting` marked as volatile? Either is accessed from synchronized blocks protected by `lock` except for `getState` which should also use the `lock` object.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13621#discussion_r1177959567
PR Review Comment: https://git.openjdk.org/jdk/pull/13621#discussion_r1177961343
PR Review Comment: https://git.openjdk.org/jdk/pull/13621#discussion_r1177957536
PR Review Comment: https://git.openjdk.org/jdk/pull/13621#discussion_r1177921389
PR Review Comment: https://git.openjdk.org/jdk/pull/13621#discussion_r1177922967
PR Review Comment: https://git.openjdk.org/jdk/pull/13621#discussion_r1177924261
PR Review Comment: https://git.openjdk.org/jdk/pull/13621#discussion_r1177938458
PR Review Comment: https://git.openjdk.org/jdk/pull/13621#discussion_r1177939984
PR Review Comment: https://git.openjdk.org/jdk/pull/13621#discussion_r1177944902
PR Review Comment: https://git.openjdk.org/jdk/pull/13621#discussion_r1177950769