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