On Mon, 4 Jan 2021 09:15:14 GMT, K Suman Rajkumaar
<github.com+70650887+skoda...@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
