On Tue, 20 May 2025 17:07:59 GMT, Alexey Ivanov <aiva...@openjdk.org> wrote:
>> Manukumar V S has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Review comments fixed : Rearranged and reused code by creating a new
>> Helper MenuItemTestHelper.java
>
> test/jdk/javax/swing/JMenuItem/MenuItemTest/MenuItemTestHelper.java line 53:
>
>> 51: } catch (Exception e) {
>> 52: e.printStackTrace();
>> 53: }
>
> I wonder if we should actually wrap the exception into `RuntimeException` and
> let it propagate to fail the test. I think we should; if Look and Feel wasn't
> set as expected, the test won't test that part of the functionality.
I'm for propagating the exception if setting L&F fails.
> test/jdk/javax/swing/JMenuItem/MenuItemTest/MenuItemTestHelper.java line 100:
>
>> 98: frame.setSize(300, 300);
>> 99: frame.setLocation(isLeft ? 0 : 600, frameY);
>> 100: frame.setVisible(true);
>
> `setLocation` shouldn't be need if you use `PassFailJFrame` layouts for
> positioning. Then `frameY` becomes redundant.
>
> `setVisible(true)` isn't need either.
Drop `setLocation` and `setVisible` as well as `frameY` parameter.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25319#discussion_r2100508682
PR Review Comment: https://git.openjdk.org/jdk/pull/25319#discussion_r2100510915
