RFR: 8325097: [macos14] DisposeInActionEventTest.html failed with message: "Event posted on wrong app contex" [v8]

[Alexey Ivanov]

On Mon, 26 Feb 2024 20:32:54 GMT, Alisen Chung <achung at openjdk.org> wrote:

>> Root cause of the test failure was fixed with https://bugs.openjdk.org/browse/JDK-8316931, updating this test since the other fix also included a test update.
>
> Alisen Chung has updated the pull request incrementally with one additional commit since the last revision:
> 
>   remove system property set

The subject of the bug needs to be updated.

The root cause has been resolved under another bug. Here, you update the test… because it uses `Applet`. You don't actually fix the test to make it pass.

Perhaps even, create a _new bug_ with clear description and close [JDK-8325097](https://bugs.openjdk.org/browse/JDK-8325097) as duplicate of [JDK-8316931](https://bugs.openjdk.org/browse/JDK-8316931) because JDK-8316931 fixed both failures. For this reason, JDK-8316931 deserves being in the `@bug` tag in both tests: `DisposeInActionEventTest` and `ShowAfterDisposeTest`.

test/jdk/java/awt/TrayIcon/DisposeInActionEventTest/DisposeInActionEventTest.java line 38:

> 36: /*
> 37:  * @test
> 38:  * @bug 6299866

Does this test serve a regression test for [JDK-8316931](https://bugs.openjdk.org/browse/JDK-8316931)?

Then 8316931 should be added to `@bug` in this test as well as in `test/jdk/java/awt/TrayIcon/ShowAfterDisposeTest/ShowAfterDisposeTest.java`.

test/jdk/java/awt/TrayIcon/DisposeInActionEventTest/DisposeInActionEventTest.java line 42:

> 40:  * handler of action event caused by clicking on this icon.
> 41:  * @library ../../regtesthelpers /test/lib
> 42:  * @build Sysout jtreg.SkippedException

Suggestion:

 * @build PassFailJFrame jtreg.SkippedException

`Sysout` isn't used in the test.

test/jdk/java/awt/TrayIcon/DisposeInActionEventTest/DisposeInActionEventTest.java line 60:

> 58:                        "  (also called Taskbar Status Area on MS Windows, Notification\n" +
> 59:                        "  Area on Gnome or System Tray on KDE) is visible.\n\n" +
> 60:                         clickInstruction + " the button on the tray icon to trigger the\n" +

_“Right-click the button”_ or _“Double-left click the button”_ doesn't make sense to me.

test/jdk/java/awt/TrayIcon/DisposeInActionEventTest/DisposeInActionEventTest.java line 67:

> 65:                        "  something is wrong the corresponding message is displayed below.\n" +
> 66:                        "  Repeat clicks several times. If no 'Test FAILED' messages\n" +
> 67:                        "  are printed, press PASS button else FAIL button.";

It's not displayed below, it's to the right, isn't it?

Fail the test automatically if you can detect the failure. You may show a message to the tester about the failure before shutting down the test.

test/jdk/java/awt/TrayIcon/DisposeInActionEventTest/DisposeInActionEventTest.java line 70:

> 68: 
> 69:         PassFailJFrame.builder()
> 70:                 .title("Test Instructions Frame")

More specific title would be better. Otherwise, you can remove `.title()` to get the same default title.

test/jdk/java/awt/TrayIcon/DisposeInActionEventTest/DisposeInActionEventTest.java line 77:

> 75:                 .testUI(DisposeInActionEventTest::showFrameAndIcon)
> 76:                 .build()
> 77:                 .awaitAndCheck();

I propose removing the icon from the system tray in a `finally` block. Otherwise, the test doesn't exit if you run it without jtreg.

test/jdk/java/awt/TrayIcon/DisposeInActionEventTest/DisposeInActionEventTest.java line 85:

> 83: 
> 84:         textArea = new JTextArea();
> 85:         frame.getContentPane().add(textArea);

Suggestion:

        frame.getContentPane().add(new JScrollPane(textArea));

Put the text area into a scroll pane to allow viewing all text even it doesn't fit. And the messages with `ActionEvent` do not fit.

test/jdk/java/awt/TrayIcon/DisposeInActionEventTest/DisposeInActionEventTest.java line 100:

> 98:         trayIcon.setImageAutoSize(true);
> 99:         trayIcon.addActionListener(ev -> {
> 100:             textArea.append(ev.toString() + "\n");

Suggestion:

            textArea.append(ev + "\n");

`.toString()` is redundant.

test/jdk/java/awt/TrayIcon/DisposeInActionEventTest/DisposeInActionEventTest.java line 109:

> 107:                     textArea.append(e.toString() + "\n");
> 108:                     textArea.append("!!! The test couldn't be performed !!!\\n");
> 109:                 }

Fail the test?

Print the stack trace of the exception with `e.printStackTrace()` — it would help analysing the failure.

test/jdk/java/awt/TrayIcon/DisposeInActionEventTest/DisposeInActionEventTest.java line 118:

> 116:             textArea.append(e.toString() + "\n");
> 117:             textArea.append("!!! The test couldn't be performed !!!\n");
> 118:         }

Also fail the test right away.

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

Changes requested by aivanov (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/17838#pullrequestreview-1903177412
PR Review Comment: https://git.openjdk.org/jdk/pull/17838#discussion_r1504041086
PR Review Comment: https://git.openjdk.org/jdk/pull/17838#discussion_r1504034589
PR Review Comment: https://git.openjdk.org/jdk/pull/17838#discussion_r1504044049
PR Review Comment: https://git.openjdk.org/jdk/pull/17838#discussion_r1504046153
PR Review Comment: https://git.openjdk.org/jdk/pull/17838#discussion_r1504047731
PR Review Comment: https://git.openjdk.org/jdk/pull/17838#discussion_r1504050445
PR Review Comment: https://git.openjdk.org/jdk/pull/17838#discussion_r1504048925
PR Review Comment: https://git.openjdk.org/jdk/pull/17838#discussion_r1504052584
PR Review Comment: https://git.openjdk.org/jdk/pull/17838#discussion_r1504054219
PR Review Comment: https://git.openjdk.org/jdk/pull/17838#discussion_r1504055513