<Swing Dev> RFR: JDK-8258884: [TEST_BUG] Convert applet-based test open/test/jdk/javax/swing/JMenuItem/8031573/bug8031573.java to a regular java test [v2]

[Alexey Ivanov]

On Mon, 4 Jan 2021 09:15:14 GMT, K Suman Rajkumaar <github.com+70650887+skodanda at openjdk.org> wrote:

>> Hi All, Could you please review this fix for JDK16?
>> 
>> Problem Description: The test open/test/jdk/javax/swing/JMenuItem/8031573/bug8031573.java is applet based.
>> 
>> Fix: Rewritten the above applet based test to a regular java test.
>> 
>> Best Regards,
>> K Suman Rajkumaar
>
> K Suman Rajkumaar has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Update bug8031573.java
>   
>   Corrected the jtreg tag from @run main bug8031573 to @run main/manual bug8031573 to make this test as a manual test.

Changes requested by aivanov (Reviewer).

test/jdk/javax/swing/JMenuItem/8031573/bug8031573.java line 60:

> 58:             + "If the display does not support HiDPI mode press PASS.\n"
> 59:             + "1. Run the test on HiDPI Display.\n"
> 60:             + "2. Press the Menu in the applet.\n"

There's no applet. Shall we say “Open the menu” instead?

test/jdk/javax/swing/JMenuItem/8031573/bug8031573.java line 61:

> 59:             + "1. Run the test on HiDPI Display.\n"
> 60:             + "2. Press the Menu in the applet.\n"
> 61:             + "3. Check that the icon on the JCheckBoxMenuItem is smooth If so, press PASS, else press FAIL.\n";

Suggestion:

            + "3. Check that the icon on the JCheckBoxMenuItem is smooth.\n"
            + "   If so, press PASS, otherwise press FAIL.";

test/jdk/javax/swing/JMenuItem/8031573/bug8031573.java line 68:

> 66: 
> 67:         if (!latch.await(5, TimeUnit.MINUTES)) {
> 68:             frame.dispose();

The `dispose()` method should be called on EDT.

test/jdk/javax/swing/JMenuItem/8031573/bug8031573.java line 78:

> 76:     private static void createTestGUI() {
> 77:         frame = new JFrame("bug8031573");
> 78:         frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);

I propose to omit this line: the test should never call `System.exit()`.

test/jdk/javax/swing/JMenuItem/8031573/bug8031573.java line 105:

> 103:             latch.countDown();
> 104:             frame.dispose();
> 105:             throw new RuntimeException("Test Failed!");

Throwing the exception on EDT is redundant and does not make the test fail; moreover you throw the exception in `main()` if `passed` is `false`.

test/jdk/javax/swing/JMenuItem/8031573/bug8031573.java line 118:

> 116:             public void windowClosing(WindowEvent e) {
> 117:                 latch.countDown();
> 118:             }

Add here `frame.dispose();`  and then the frame will close without `frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);` and the app will exit even when launched as a stand-alone app.

test/jdk/javax/swing/JMenuItem/8031573/bug8031573.java line 52:

> 50: public class bug8031573 {
> 51: 
> 52:     private static JFrame frame;

`frame` should also be `volatile` as it's accessed from multiple threads: main thread and EDT.

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

PR: https://git.openjdk.java.net/jdk/pull/1878