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

[Tejesh R]

On Wed, 26 Apr 2023 14:25:42 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 ScrollPaneWindowsTest.java
>
> test/jdk/java/awt/ScrollPane/ComponentScrollTest.java line 80:
> 
>> 78:                     Thread.sleep(5000);
>> 79:                 } catch (InterruptedException ie) {
>> 80:                 }
> 
> This `sleep` blocks EDT. Is it intended?

Yeah, I guess 5000 might not be required 1000 might be fine, but either delay or sleep is required I feel.

> test/jdk/java/awt/ScrollPane/ScrollPaneRemoveAdd.java line 87:
> 
>> 85: 
>> 86:             Point buttonLoc = button.getLocationOnScreen();
>> 87:             Dimension buttonSize = button.getSize();
> 
> Both `getLocationOnScreen` and `getSize` should also be called on EDT?

Updated.

> test/jdk/java/awt/ScrollPane/ScrollPaneWindowsTest.java line 53:
> 
>> 51:     Frame frame;
>> 52:     Insets paneInsets;
>> 53:     public static Object LOCK = new Object();
> 
> The lock object should be `final`.

Updated.

> test/jdk/java/awt/ScrollPane/ScrollPaneWindowsTest.java line 62:
> 
>> 60:             System.out.println("This test is for Windows 2000 and above.");
>> 61:             return;
>> 62:         }
> 
> Shall this be dropped too? Java won't start on Windows versions before Vista (6.0), so does it serve a purpose other than a historical reasons?

Updated.

> test/jdk/java/awt/ScrollPane/ScrollPaneWindowsTest.java line 70:
> 
>> 68:     public void init() throws Exception {
>> 69:         EventQueue.invokeAndWait(() -> {
>> 70:             frame = new Frame();
> 
> Suggestion:
> 
>             frame = new Frame("ScrollPaneWindowsTest");

Updated.

> test/jdk/java/awt/ScrollPane/ScrollPaneWindowsTest.java line 81:
> 
>> 79:         hScroll = (ScrollPaneAdjustable) sp.getHAdjustable();
>> 80:         vScroll.addAdjustmentListener(this);
>> 81:         hScroll.addAdjustmentListener(this);
> 
> ScrollPane should be created inside `invokeAndWait` block, listeners should be added on EDT too.

Updated.

> test/jdk/java/awt/ScrollPane/ScrollPositionIntact.java line 31:
> 
>> 29: */
>> 30: 
>> 31: import java.awt.*;
> 
> Avoid wildcard imports.

Updated.

> test/jdk/java/awt/ScrollPane/ScrollPositionIntact.java line 61:
> 
>> 59:                 pa.add("East", new Label("East", Label.RIGHT));
>> 60:                 sp.add(pa);
>> 61:                 frame = new Frame();
> 
> Suggestion:
> 
>                 frame = new Frame("ScrollPositionIntact");

Updated.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13621#discussion_r1178149780
PR Review Comment: https://git.openjdk.org/jdk/pull/13621#discussion_r1178149893
PR Review Comment: https://git.openjdk.org/jdk/pull/13621#discussion_r1178149988
PR Review Comment: https://git.openjdk.org/jdk/pull/13621#discussion_r1178150098
PR Review Comment: https://git.openjdk.org/jdk/pull/13621#discussion_r1178150275
PR Review Comment: https://git.openjdk.org/jdk/pull/13621#discussion_r1178150178
PR Review Comment: https://git.openjdk.org/jdk/pull/13621#discussion_r1178150376
PR Review Comment: https://git.openjdk.org/jdk/pull/13621#discussion_r1178150724