RFR: 8316389: Open source few AWT applet tests [v4]

[Alexey Ivanov]

On Wed, 20 Sep 2023 16:55:30 GMT, Alexander Zvegintsev <azvegint at openjdk.org> wrote:

>> Open sourcing few tests:
>> 
>> java/awt/Frame/FrameRepackTest.java
>> java/awt/Frame/FrameResizeTest/FrameResizeTest_1.java
>> java/awt/Frame/FrameResizeTest/FrameResizeTest_2.java
>> java/awt/Frame/WindowMoveTest.java
>
> Alexander Zvegintsev has updated the pull request incrementally with one additional commit since the last revision:
> 
>   space added

Do we add `@Override` annotations?

test/jdk/java/awt/Frame/FrameRepackTest.java line 53:

> 51:             The menu has two items, labelled 'visible' and 'not visible'.
> 52:             The frame also contains a red panel that contains two line labels,
> 53:             ' This panel is always displayed' and ' it is a test'.

Suggestion:

            'This panel is always displayed' and 'it is a test'.

test/jdk/java/awt/Frame/FrameRepackTest.java line 64:

> 62:             You can repeatedly add and remove the second panel in this way.
> 63:             After such an addition or removal, the frame's location on the screen
> 64:             should not have changed, while the size changes to accommodate

Suggestion:

            should not change, while the size changes to accommodate

It's happening now, during the test.

test/jdk/java/awt/Frame/FrameRepackTest.java line 116:

> 114:         Menu flip = new Menu("Flip");
> 115:         MenuItem mi;
> 116:         flip.addSeparator();

The separator as the first item in a drop-down menu looks weird and confusing, I propose removing it.

test/jdk/java/awt/Frame/FrameRepackTest.java line 134:

> 132:         north.setLayout(new BorderLayout(2, 2));
> 133:         north.add("North", new Label(" This panel is always displayed"));
> 134:         north.add("Center", new Label(" it is a test "));

Suggestion:

        north.add("North", new Label("This panel is always displayed"));
        north.add("Center", new Label("it is a test "));

Is the space used to offset the text from the edge?

test/jdk/java/awt/Frame/FrameRepackTest.java line 169:

> 167:                 south.setVisible(true);
> 168:                 pack();
> 169:                 setVisible(true);

Nothing should change if you remove `setVisible(true)` here, the frame is never hidden, is it?

test/jdk/java/awt/Frame/FrameResizeTest/FrameResizeTest_1.java line 45:

> 43:         To the right of this frame is an all-white 200x200 frame.
> 44: 
> 45:         This is actually a white canvas component of the frame.

Suggestion:

        This is actually a white canvas component in the frame.

test/jdk/java/awt/Frame/FrameResizeTest/FrameResizeTest_1.java line 48:

> 46:         The frame itself is red.
> 47:         The red should never show.
> 48:         In particular, after you resize the frame, you should see all white and no red.

If I resize the frame quickly, I see some areas in red, I guess it's expected…

test/jdk/java/awt/Frame/FrameResizeTest/FrameResizeTest_1.java line 62:

> 60:                 .build();
> 61: 
> 62:         SwingUtilities.invokeAndWait(() -> {

Suggestion:

        EventQueue.invokeAndWait(() -> {

Using `EventQueue` is more appropriate for AWT components.

test/jdk/java/awt/Frame/FrameResizeTest/FrameResizeTest_2.java line 95:

> 93: 
> 94:         Container dumbc = new DumbC();
> 95:         add( dumbc, c );

Suggestion:

        add(dumbc, c);

test/jdk/java/awt/Frame/FrameResizeTest/FrameResizeTest_2.java line 98:

> 96: 
> 97:         Panel dump = new DumbPanel();
> 98:         add( dump, c );

Suggestion:

        add(dump, c);

test/jdk/java/awt/Frame/FrameResizeTest/FrameResizeTest_2.java line 100:

> 98:         add( dump, c );
> 99: 
> 100:         setSize( new Dimension( 300, 300));

Suggestion:

        setSize(new Dimension(300, 300));

or directly `(300, 300)` without creating the `Dimension` object.

test/jdk/java/awt/Frame/WindowMoveTest.java line 56:

> 54:                 frame.dispatchEvent(new WindowEvent(frame, WindowEvent.WINDOW_CLOSING)));
> 55: 
> 56:         WindowMove.latch.await();

Add a timeout for `await` just in case? However, jtreg will also handle timeout.

test/jdk/java/awt/Frame/WindowMoveTest.java line 69:

> 67: 
> 68:     static CountDownLatch latch = new CountDownLatch(1);
> 69:     static volatile String failMessage = null;

It does not need to be `volatile`, using the latch establishes the _happens before_ relation between the EDT and the main thread.

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

PR Review: https://git.openjdk.org/jdk/pull/15787#pullrequestreview-1642316127
PR Review Comment: https://git.openjdk.org/jdk/pull/15787#discussion_r1335980344
PR Review Comment: https://git.openjdk.org/jdk/pull/15787#discussion_r1336031064
PR Review Comment: https://git.openjdk.org/jdk/pull/15787#discussion_r1336032622
PR Review Comment: https://git.openjdk.org/jdk/pull/15787#discussion_r1336035280
PR Review Comment: https://git.openjdk.org/jdk/pull/15787#discussion_r1336038451
PR Review Comment: https://git.openjdk.org/jdk/pull/15787#discussion_r1336042208
PR Review Comment: https://git.openjdk.org/jdk/pull/15787#discussion_r1336043985
PR Review Comment: https://git.openjdk.org/jdk/pull/15787#discussion_r1336045648
PR Review Comment: https://git.openjdk.org/jdk/pull/15787#discussion_r1336047171
PR Review Comment: https://git.openjdk.org/jdk/pull/15787#discussion_r1336047580
PR Review Comment: https://git.openjdk.org/jdk/pull/15787#discussion_r1336048137
PR Review Comment: https://git.openjdk.org/jdk/pull/15787#discussion_r1336060409
PR Review Comment: https://git.openjdk.org/jdk/pull/15787#discussion_r1336057460