On Wed, 18 Sep 2024 11:18:19 GMT, Alexey Ivanov <aiva...@openjdk.org> wrote:
>> Abhishek Kumar has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Test instruction update
>
> test/jdk/java/awt/MenuItem/LotsOfMenuItemsTest.java line 65:
>
>> 63: .rows((int) INSTRUCTIONS.lines().count() + 2)
>> 64: .columns(40)
>> 65: .testUI(obj.createAndShowUI())
>
> Suggestion:
>
> .testUI(obj::createAndShowUI)
>
> You should pass a method reference to call `createAndShowUI` on EDT.
>
> When you pass a method reference, you delegate the job of calling the method
> to `PassFailJFrame` and it calls it on EDT.
>
> If you use `obj.createAndShowUI`, you pass the result of calling
> `obj.createAndShowUI` — the list of windows — to `PassFailJFrame`, the method
> gets called on the main thread.
>
> It's a subtle but important difference. I believe we're trying to ensure all
> components are created and accessed on EDT, both AWT and Swing, unless the
> test specifically verifies AWT components and needs to ensure these
> components behave as they should in a concurrent environment.
Updated.
> test/jdk/java/awt/MenuItem/LotsOfMenuItemsTest.java line 83:
>
>> 81: }
>> 82:
>> 83: public void componentShown(ComponentEvent e) {
>
> Suggestion:
>
> @Override
> public void componentShown(ComponentEvent e) {
>
> Please add `@Override` annotation where applicable. It makes it explicit that
> a method implements an interface or overrides a method of a superclass.
Updated.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21029#discussion_r1764885524
PR Review Comment: https://git.openjdk.org/jdk/pull/21029#discussion_r1764885276
