On Fri, 15 Nov 2024 14:35:25 GMT, Alisen Chung <ach...@openjdk.org> wrote:
>> Cleaning up tests building ExtendedRobot that shouldn't be.
>
> Alisen Chung has updated the pull request incrementally with two additional
> commits since the last revision:
>
> - EOF newline ActionEventTest
> - revert ActionEventTest
LGTM apart from the following minor inline comments.
Since @aivanov-jdk reviewed previously please wait to see in case he wants to
add anything additional.
test/jdk/javax/swing/JInternalFrame/6725409/bug6725409.java line 49:
> 47: private JInternalFrame iFrame;
> 48: private static TestTitlePane testTitlePane;
> 49: private boolean passed;
Suggestion:
private static volatile boolean passed;
test/jdk/javax/swing/JInternalFrame/6725409/bug6725409.java line 50:
> 48: private static TestTitlePane testTitlePane;
> 49: private boolean passed;
> 50: private static final Robot robot = createRobot();
Suggestion but not a must change: Since createRobot() is just creating an
instance of Robot, the extra method can be removed and robot can be initialized
in main method.
test/jdk/javax/swing/JInternalFrame/6725409/bug6725409.java line 54:
> 52: public static void main(String[] args) throws Exception {
> 53: UIManager.setLookAndFeel(
> 54: new
> com.sun.java.swing.plaf.windows.WindowsClassicLookAndFeel());
Is fully qualified name required? I think we can shorten it by adding required
imports.
test/jdk/javax/swing/JInternalFrame/6725409/bug6725409.java line 138:
> 136:
> 137: private static void sync() {
> 138: robot.waitForIdle();
Since sync() is called after UI setup better to add `robot.delay(500)` for
stability.
test/jdk/javax/swing/JInternalFrame/6725409/bug6725409.java line 146:
> 144: } catch (Exception ex) {
> 145: throw new Error("Can't create Robot", ex);
> 146: }
Same with com.sun.java.swing.plaf.windows.WindowsInternalFrameTitlePane , fully
qualified name can be replaced.
-------------
Marked as reviewed by honkar (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/20846#pullrequestreview-2439644547
PR Review Comment: https://git.openjdk.org/jdk/pull/20846#discussion_r1844486162
PR Review Comment: https://git.openjdk.org/jdk/pull/20846#discussion_r1844487898
PR Review Comment: https://git.openjdk.org/jdk/pull/20846#discussion_r1844481698
PR Review Comment: https://git.openjdk.org/jdk/pull/20846#discussion_r1844482837
PR Review Comment: https://git.openjdk.org/jdk/pull/20846#discussion_r1844483763
