On Fri, 5 Jul 2024 17:13:47 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
>> Marius Hanl has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> add many more unit tests for Tooltip
>
> tests/system/src/test/java/test/robot/javafx/scene/TooltipTest.java line 81:
>
>> 79:
>> 80: @AfterAll
>> 81: static void exit() {
>
> I still not convinced I like the concept of non-public methods in tests,
> since they are annotated.
>
> Why do we need to do this?
Please check:
https://stackoverflow.com/questions/55215949/why-junit-5-default-access-modifier-changed-to-package-private
It is essentially the good old 'why make something public when you do not need
it?'.
It follows the way encapsulation is meant to be, by lowering the visibility as
far as you can for something that is not meant to be called from the outside.
Also static code analysis will check that and issue a code smell, which makes
sense. Quoting Sonarlint here:
"
JUnit5 is more tolerant regarding the visibility of test classes and methods
than JUnit4, which required everything to be public. Test classes and methods
can have any visibility except private. It is however recommended to use the
default package visibility to improve readability.
Test classes, test methods, and lifecycle methods are not required to be
public, but they must not be private.
It is generally recommended to omit the public modifier for test classes, test
methods, and lifecycle methods unless there is a technical reason for doing so
– for example, when a test class is extended by a test class in another
package. Another technical reason for making classes and methods public is to
simplify testing on the module path when using the Java Module System.
— JUnit5 User Guide
"
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1394#discussion_r1667351090
