RFR: 8306996: Open source Swing MenuItem related tests [v2]

Abhishek Kumar abhiscxk at openjdk.org
Mon May 8 09:33:43 UTC 2023


On Fri, 5 May 2023 18:03:12 GMT, Phil Race <prr at openjdk.org> wrote:

>> Abhishek Kumar has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Review comment update
>
> test/jdk/javax/swing/JMenuItem/bug4304129.java line 47:
> 
>> 45:     public static void main(String args[]) throws Exception {
>> 46:         bug4304129 test = new bug4304129();
>> 47:         test.init();
> 
> I don't see why we need an instance here.
> The body of init() can be moved into main() and mnemonic and accelerator made static

Updated.

> test/jdk/javax/swing/JMenuItem/bug4839464.java line 79:
> 
>> 77:     public static JButton changeShortDescButton;
>> 78: 
>> 79:     public JMenuItem item;
> 
> it seems a bit odd that this and some below are instance vars  and the ones above are static
> 
> Should you make them all static  ?.. and then you don't need an instance of the test class either
> Or make them all instance .. either way ...
> 
> I also find it odd that this test creates TWO Robots - one AWT Robot and one JRobot.
> Can you consolidate these ?

Updated.

> test/jdk/javax/swing/JMenuItem/bug4966168.java line 46:
> 
>> 44:     public static void main(String args[]) throws Exception {
>> 45:         bug4966168 test = new bug4966168();
>> 46:         test.init();
> 
> No need I can see for the instance. Collapse init() into main()

Updated.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13822#discussion_r1187232377
PR Review Comment: https://git.openjdk.org/jdk/pull/13822#discussion_r1187232672
PR Review Comment: https://git.openjdk.org/jdk/pull/13822#discussion_r1187232953



More information about the client-libs-dev mailing list