8350924: javax/swing/JMenu/4213634/bug4213634.java fails #23837
Conversation
|
👋 Welcome back psadhukhan! A progress list of the required criteria for merging this PR into |
|
❗ This change is not yet ready to be integrated. |
| test(); | ||
| } | ||
|
|
||
| public void createAndShowGUI() { | ||
| public static void createAndShowGUI() { | ||
| frame = new JFrame("TEST"); |
There was a problem hiding this comment.
Could you use a more specific title?
| } | ||
| SwingUtilities.invokeAndWait(() -> { | ||
| frame.dispose(); | ||
| if (!menu.isSelected()) { |
There was a problem hiding this comment.
Previously, the condition !menu.isSelected() was verified before the frame was disposed of.
I think you should verify whether menu is select before disposing of the frame; once the frame is disposed of, the state of components is not well-defined.
There was a problem hiding this comment.
I agree, frame.dispose() can be added within finally block instead of here.
| @@ -39,35 +39,29 @@ | |||
| * @test | |||
| * @key headful | |||
| * @bug 4213634 8017187 | |||
| * @author Scott Violet | |||
| * @library ../../regtesthelpers | |||
There was a problem hiding this comment.
| * @library ../../regtesthelpers | |
| * @library /java/awt/regtesthelpers |
| Robot robot = new Robot(); | ||
| robot.setAutoDelay(50); | ||
| robot.waitForIdle(); | ||
| private static void test() throws AWTException, InterruptedException, InvocationTargetException { |
There was a problem hiding this comment.
| private static void test() throws AWTException, InterruptedException, InvocationTargetException { | |
| private static void test() throws Exception { |
| } | ||
| SwingUtilities.invokeAndWait(() -> { | ||
| frame.dispose(); | ||
| if (!menu.isSelected()) { |
There was a problem hiding this comment.
I agree, frame.dispose() can be added within finally block instead of here.
| @@ -77,33 +71,30 @@ public void createAndShowGUI() { | |||
| frame.getContentPane().add("Center", ta); | |||
There was a problem hiding this comment.
The text area seems redundant, especially the text inside which looks like an Easter egg.
| @@ -116,8 +107,9 @@ public void actionPerformed(ActionEvent e) { | |||
| "Failed: Mnemonic activated"); | |||
| } | |||
| }); | |||
| if(bFlag) | |||
There was a problem hiding this comment.
Can also fix the for-loop above into:
for (int i = 0; i < 10; i++) {
And can make a lambda expression by using:
menuitem.addActionListener(e -> {
throw new RuntimeException(
"Failed: Mnemonic activated");
});
| SwingUtilities.invokeAndWait(() -> { | ||
| createAndShowGUI(); | ||
| }); |
There was a problem hiding this comment.
| SwingUtilities.invokeAndWait(() -> { | |
| createAndShowGUI(); | |
| }); | |
| SwingUtilities.invokeAndWait(() -> createAndShowGUI()); |
| @@ -77,33 +71,30 @@ public void createAndShowGUI() { | |||
| frame.getContentPane().add("Center", ta); | |||
| JButton button = new JButton("Test"); | |||
| frame.getContentPane().add("South", button); | |||
| frame.setBounds(100, 100, 400, 400); | |||
| frame.setSize(400, 400); | |||
There was a problem hiding this comment.
Is this the best frame size? I see the text area has text to output, but the full sentence isn't even shown and there's no word wrap. Can change the text to anything else, especially since the TextArea text exceeds the line char limit. There's also a lot of empty space in this test frame. So, this can be changed in a variety of ways.
Test fails in ubuntu OCI system..Made it more robust my adding waitForIdle/delay before commencing test..
OCI system is ok with the fix.
Progress
Issue
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/23837/head:pull/23837$ git checkout pull/23837Update a local copy of the PR:
$ git checkout pull/23837$ git pull https://git.openjdk.org/jdk.git pull/23837/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 23837View PR using the GUI difftool:
$ git pr show -t 23837Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/23837.diff
Using Webrev
Link to Webrev Comment