On Mon, 16 Jan 2023 13:33:04 GMT, Alexey Ivanov <[email protected]> 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
