On Mon, 25 Sep 2023 15:12:27 GMT, Alexey Ivanov <aiva...@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