Re: RFR: 8316931: [macos14] Test "java/awt/TrayIcon/ShowAfterDisposeTest/ShowAfterDisposeTest.html" throws and exception on Mac OS 14(x64, aarch64) [v2]
On Wed, 10 Jan 2024 21:08:35 GMT, Alisen Chung wrote:
>> SunToolkit.java is trying to post an event on the TrayIcon appContext, but
>> the TrayIcon was already removed by the test, causing an error. The fix is
>> to make SunToolkit skip posting the event if appContext is null. The test is
>> also updated to remove applet usage and use PassFailJFrame instead.
>
> Alisen Chung has updated the pull request incrementally with two additional
> commits since the last revision:
>
> - spacing
> - updated test title, copyright year, removed redundant check
Please update the copyright year in `SunToolkit.java` too.
I haven't looked through all the code in the test.
Note: I've edited the JBS subject to correct “and” to “an” and to remove double
space.
test/jdk/java/awt/TrayIcon/ShowAfterDisposeTest/ShowAfterDisposeTest.java line
2:
> 1: /*
> 2: * Copyright (c) 2013, 2024 Oracle and/or its affiliates. All rights
> reserved.
Suggestion:
* Copyright (c) 2013, 2024, Oracle and/or its affiliates. All rights reserved.
There must be another comma after the second copyright year.
test/jdk/java/awt/TrayIcon/ShowAfterDisposeTest/ShowAfterDisposeTest.java line
58:
> 56: System.out.println("The test cannot be run because SystemTray
> is not supported.");
> 57: return;
> 58: }
I'd rather move this entire check into the `main` method and throw
`jtreg.SkippedException` instead.
-
PR Review: https://git.openjdk.org/jdk/pull/17329#pullrequestreview-1819063454
PR Review Comment: https://git.openjdk.org/jdk/pull/17329#discussion_r1450937261
PR Review Comment: https://git.openjdk.org/jdk/pull/17329#discussion_r1450939638
Re: RFR: 8316931: [macos14] Test "java/awt/TrayIcon/ShowAfterDisposeTest/ShowAfterDisposeTest.html" throws and exception on Mac OS 14(x64, aarch64) [v2]
On Wed, 10 Jan 2024 21:08:35 GMT, Alisen Chung wrote: >> SunToolkit.java is trying to post an event on the TrayIcon appContext, but >> the TrayIcon was already removed by the test, causing an error. The fix is >> to make SunToolkit skip posting the event if appContext is null. The test is >> also updated to remove applet usage and use PassFailJFrame instead. > > Alisen Chung has updated the pull request incrementally with two additional > commits since the last revision: > > - spacing > - updated test title, copyright year, removed redundant check Looks good. - Marked as reviewed by kizune (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/17329#pullrequestreview-1819041585
Re: RFR: 8316931: [macos14] Test "java/awt/TrayIcon/ShowAfterDisposeTest/ShowAfterDisposeTest.html" throws and exception on Mac OS 14(x64, aarch64) [v2]
On Wed, 10 Jan 2024 21:08:35 GMT, Alisen Chung wrote: >> SunToolkit.java is trying to post an event on the TrayIcon appContext, but >> the TrayIcon was already removed by the test, causing an error. The fix is >> to make SunToolkit skip posting the event if appContext is null. The test is >> also updated to remove applet usage and use PassFailJFrame instead. > > Alisen Chung has updated the pull request incrementally with two additional > commits since the last revision: > > - spacing > - updated test title, copyright year, removed redundant check Extra blank lines can be removed else looks good to me. Since `html` file has been removed, I guess the java file can be taken out of `ShowAfterDisposeTest` folder. - Marked as reviewed by abhiscxk (Committer). PR Review: https://git.openjdk.org/jdk/pull/17329#pullrequestreview-1817331918 PR Comment: https://git.openjdk.org/jdk/pull/17329#issuecomment-1888406196
Re: RFR: 8316931: [macos14] Test "java/awt/TrayIcon/ShowAfterDisposeTest/ShowAfterDisposeTest.html" throws and exception on Mac OS 14(x64, aarch64) [v2]
On Wed, 10 Jan 2024 21:08:35 GMT, Alisen Chung wrote: >> SunToolkit.java is trying to post an event on the TrayIcon appContext, but >> the TrayIcon was already removed by the test, causing an error. The fix is >> to make SunToolkit skip posting the event if appContext is null. The test is >> also updated to remove applet usage and use PassFailJFrame instead. > > Alisen Chung has updated the pull request incrementally with two additional > commits since the last revision: > > - spacing > - updated test title, copyright year, removed redundant check @azuev-java please review - PR Comment: https://git.openjdk.org/jdk/pull/17329#issuecomment-1887796333
Re: RFR: 8316931: [macos14] Test "java/awt/TrayIcon/ShowAfterDisposeTest/ShowAfterDisposeTest.html" throws and exception on Mac OS 14(x64, aarch64) [v2]
> SunToolkit.java is trying to post an event on the TrayIcon appContext, but > the TrayIcon was already removed by the test, causing an error. The fix is to > make SunToolkit skip posting the event if appContext is null. The test is > also updated to remove applet usage and use PassFailJFrame instead. Alisen Chung has updated the pull request incrementally with two additional commits since the last revision: - spacing - updated test title, copyright year, removed redundant check - Changes: - all: https://git.openjdk.org/jdk/pull/17329/files - new: https://git.openjdk.org/jdk/pull/17329/files/171a1554..53629e25 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17329=01 - incr: https://webrevs.openjdk.org/?repo=jdk=17329=00-01 Stats: 8 lines in 1 file changed: 0 ins; 4 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/17329.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17329/head:pull/17329 PR: https://git.openjdk.org/jdk/pull/17329
