On Fri, 22 Sep 2023 17:14:30 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
>> Marius Hanl has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> JDK-8315569: Improve test name and add 2.25 scale
>
> modules/javafx.controls/src/test/java/test/javafx/scene/control/ControlLayoutTest.java
> line 44:
>
>> 42: import static org.junit.jupiter.api.Assertions.assertTrue;
>> 43:
>> 44: class ControlLayoutTest {
>
> 1. should this class be public?
> 2. since no more tests are being planned here, perhaps we could rename it to
> be more specific (ControlLayoutTest still seems to be too generic to me),
> ControlLayoutChildrenContractTest or something like that?
> 3. could you please add a javadoc at the class level to explain what generate
> area this class tests?
1. No, JUnit5 classes can be package private as well and it is actually
recommended
2. Well, we don't know. Maybe we will add some at one point, right now I don't
have any idea what to add. Therefore I would keep the more generic name so we
can reuse it at one point.
3. Same reason as 2., if we add more tests the Javadoc at class level may not
be accurate anymore.
+ There is Javadoc at method level already, which is more than 'necessary' as
there is no policy that specifies that javadoc should be added to tests, I just
do it voluntarily.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1229#discussion_r1357363874
