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

[Alexey Ivanov]

On Tue, 27 Feb 2024 21:13:21 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:
> 
>   update test from feedback

Changes requested by aivanov (Reviewer).

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

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

My previous comment implies you should update the instructions to state that the test will _fail automatically_ if a failure condition detected.

After repeating clicking several times, the tester can press the _Pass_ button.

The tester should never click the _Fail_ button.

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

> 69: 
> 70:         PassFailJFrame.builder()
> 71:                 .title("Event Message Display")

Suggestion:

                .title("Dispose In ActionEvent Test")

The name of the test is good enough if you want to set a custom title.

What does "Event Message Display" mean? The instruction UI frame contain the instructions and Pass/Fail buttons.

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

> 80: 
> 81:     private static JFrame showFrameAndIcon() {
> 82:         JFrame frame = new JFrame("DisposeInActionEventTest");

Suggestion:

        JFrame frame = new JFrame("Event Message Display");

This is where events are displayed.

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

> 86:         textArea = new JTextArea();
> 87:         spane.add(textArea);
> 88:         frame.getContentPane().add(spane);

The code should look like this:


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

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

> 110:                     textArea.append(e + "\n");
> 111:                     e.printStackTrace();
> 112:                     throw new RuntimeException();

If you throw another exception after catching, pass the original exception as the `cause` parameter.

Since this is a manual test, you should use `forceFail` to exit the test gracefully:
Suggestion:

                    e.printStackTrace();
                    PassFailJFrame.forceFail("Exception caught: " + e.getMessage());


Adding a message to `textArea` makes no sense, the tester won't be able to see it — the test shuts down right after you do it.

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

> 120:             textArea.append(e + "\n");
> 121:             e.printStackTrace();
> 122:             throw new RuntimeException("Test failed.");

As above.

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

PR Review: https://git.openjdk.org/jdk/pull/17838#pullrequestreview-1905984379
PR Review Comment: https://git.openjdk.org/jdk/pull/17838#discussion_r1506066597
PR Review Comment: https://git.openjdk.org/jdk/pull/17838#discussion_r1505784352
PR Review Comment: https://git.openjdk.org/jdk/pull/17838#discussion_r1506067515
PR Review Comment: https://git.openjdk.org/jdk/pull/17838#discussion_r1505786130
PR Review Comment: https://git.openjdk.org/jdk/pull/17838#discussion_r1505790776
PR Review Comment: https://git.openjdk.org/jdk/pull/17838#discussion_r1505791483