RFR: 8300084: AquaFileChooserUI.getDefaultButton returns null [v2]

Tejesh R tr at openjdk.org
Mon Jan 16 13:49:26 UTC 2023


On Mon, 16 Jan 2023 13:33:04 GMT, Alexey Ivanov <aivanov at openjdk.org> wrote:

> > > This case is entirely covered by the test in #11901 for [JDK-8299522](https://bugs.openjdk.org/browse/JDK-8299522), which also ensures the button text isn't empty.
> > > Adding 8300084 to the `@bug` tag in that test is enough. It was my [original suggestion](https://github.com/openjdk/jdk/pull/11901#discussion_r1068659255):
> > > > Now you can fix this bug so that Aqua L&F doesn't return null and add that bugid to this test.
> > 
> > 
> > Yeah, but since #11901 handles only Approve button size except for Aqua L&F and this fix is only for Aqua L&F I thought it would be better to not to maintain any dependency between the test/bug (Though this bug was initiated as part of #11901 fix). I hope maintaining simple test case for two bug would be fine.
> 
> It's exactly the point: **include Aqua L&F in #11901**. The Aqua L&F is excluded only because `getDefaultButton` returns null, isn't it? This fix makes it return non-null, therefore Aqua L&F can be included in testing.
> 
> This fix should be integrated before [JDK-8299522](https://bugs.openjdk.org/browse/JDK-8299522). I will not approve #11901 until Aqua L&F is part of the `CustomApproveButtonTest.java` test.
The Aqua L&F is excluded not only because `getDefaultButton` returns null, but it is also excluded because the fix doesn't apply to Aqua L&F. Still if u insist to use the same test then can do that, as u said first this fix will go in and then #11901.

> test/jdk/javax/swing/JFileChooser/AquaDefaultButtonTest.java line 46:
> 
>> 44:                 JFileChooser fc = new JFileChooser();
>> 45:                 JButton defApproveBtn = fc.getUI().getDefaultButton(fc);
>> 46:                 if (defApproveBtn == null) {
> 
> Suggestion:
> 
>                 JButton approve = fc.getUI().getDefaultButton(fc);
>                 if (approve == null) {
> 
> On the scope of two lines, its type — `JButton` — as well as method name makes it obvious it's a _button_.
> 
> `approveButton` would also be good.
> 
> Yet I am for removing this test completely because it's functionality is entirely covered by the `CustomApproveButtonTest.java` in #11901.

Ok, then I'll remove this test and update https://github.com/openjdk/jdk/pull/11901.

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

PR: https://git.openjdk.org/jdk/pull/12008



More information about the client-libs-dev mailing list