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

Alexander Zvegintsev azvegint at openjdk.org
Mon Sep 25 17:21:18 UTC 2023


On Mon, 25 Sep 2023 15:12:27 GMT, Alexey Ivanov <aivanov at openjdk.org> wrote:

>> Alexander Zvegintsev has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   space added
>
> 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?

No idea. So removed.

> 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?

Sure, removed.

> 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…

Updated the instructions slightly to reflect this.

> 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.

This particular case also uses the JFrame from PassFailJFrame.
So I used the Swing variant here (which uses EventQueue.invokeAndWait internally).

> 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.

Yes, the original idea was that jtreg was responsible for the timeout.

Changed it to a smaller timeout.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/15787#discussion_r1336155065
PR Review Comment: https://git.openjdk.org/jdk/pull/15787#discussion_r1336153641
PR Review Comment: https://git.openjdk.org/jdk/pull/15787#discussion_r1336179241
PR Review Comment: https://git.openjdk.org/jdk/pull/15787#discussion_r1336152307
PR Review Comment: https://git.openjdk.org/jdk/pull/15787#discussion_r1336142471


More information about the client-libs-dev mailing list