RFR: 8286620: Create regression test for verifying setMargin() of JRadioButton [v3]

Alexey Ivanov aivanov at openjdk.java.net
Thu Jun 2 08:10:35 UTC 2022


On Thu, 2 Jun 2022 07:51:00 GMT, Alexey Ivanov <aivanov at openjdk.org> wrote:

>>> > > > I wonder why the test is Windows-specific if it allows changing Look and Feels.
>>> > > > The only Look and Feel which ignores the background is Nimbus, other L&Fs on Windows at least paint the yellow and green background. As such, the test can be run on other platforms.
>>> > > > Was the original bug in Windows Look and Feel? If so, you may want to make it the default L&F.
>>> > > > The instructions don't mention anything about other Look and Feels. Does the tester have to verify each presented L&F?
>>> > > 
>>> > > 
>>> > > I think the test was in windows which I might have retained the os type, will check and confirm once. The tester have to verify with each L&F for left margin check.
>>> > 
>>> > 
>>> > Yeah, it was only for Motif and Windows before, so I have retained it for only windows os.
>>> 
>>> I can see no reason why the test is for Windows only.
>>> 
>>> The original bug [JDK-4380543](https://bugs.openjdk.java.net/browse/JDK-4380543) lists all the operating systems as affected. Then [JDK-8134640](https://bugs.openjdk.java.net/browse/JDK-8134640) asked to support all Look-and-Feels, which somewhat implies other platforms should also be tested.
>> 
>> Ok. Since this PR is about the movement of test from closed to open, I would like to retain it for Windows alone @aivanov-jdk as the bug fix [JDK-8134640](https://bugs.openjdk.java.net/browse/JDK-8134640) has been taken care for windows. Will surely plan for other L&F in sometime.
>
>> > I can see no reason why the test is for Windows only.
>> >
>> > The original bug [JDK-4380543](https://bugs.openjdk.java.net/browse/JDK-4380543) lists all the operating systems as affected. Then [JDK-8134640](https://bugs.openjdk.java.net/browse/JDK-8134640) asked to support all Look-and-Feels, which somewhat implies other platforms should also be tested.
>> 
>> Ok. Since this PR is about the movement of test from closed to open, I would like to retain it for Windows alone @aivanov-jdk as the bug fix [JDK-8134640](https://bugs.openjdk.java.net/browse/JDK-8134640) has been taken care for windows. Will surely plan for other L&F in sometime.
> 
> Why can't we make it run on other platforms here?
> 
> All it takes is removing `@requires (os.family == "windows")`. Well, obviously, you have to run it on macOS and Linux to make sure it works as expected.
> 
> I think doing it right now is more than reasonable compared to submitting a new issue just to remove the OS restriction and going through the code review.

> > > @aivanov-jdk Recently we have started to use the latter and remove the extra folder (`/4380543/`)
> > 
> > Thank you for clarification, Harshitha.
> 
> Yes, blame (?) me :-)
> We may not always remember to point it out but it is what we want.

I see. I noticed it in recent code reviews, so I wanted to clarify.

> I've never seen the point in an extra level of folder except when the test is composed of multiple files all unique to the test.

I agree to some extent… When using an IDE to compile and run a test, a separate folder is quite convenient: add it as test source, and you're done. When there are many tests in one folder, it could result in compilation errors. Some tests declare the same class names, a number of manual tests have `Sysout` class, and thus it causes duplicate class names, which makes it impossible to run a test right from IDE unless you take additional steps to resolve or prevent compilation errors.

On the other hand, the shorter filesystem tree is easier to navigate. As such, flatter structure is the way to go.

> I also request that new tests not be given names like bug87654321.java but instead be named in a way that you can tell what they are supposed to be testing like in this case something like RadioButtonMarginTest.java

Descriptive names are easier to remember and to refer to, they give you an idea of what test does when you see a failure. With a bugid, you have to open the test source or the bug in JBS for getting that info.

I fully support meaningful, descriptive names for new tests.

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

PR: https://git.openjdk.java.net/jdk/pull/8721



More information about the client-libs-dev mailing list