RFR: JDK-8315569: Tests for the contract of SkinBase.layoutChildren(..) [v3]

[Andy Goryachev]

On Fri, 22 Sep 2023 10:11:12 GMT, Marius Hanl <mhanl at openjdk.org> wrote:

>> This PR adds a test that verifies the `SkinBase.layoutChildren(..)` method with different scales.
>> While not explicitly documented, this method will receive the snapped and correctly calculated x, y, width and height values, so that children of the Control can be layouted correctly without requiring to do many more calculations regarding padding.
>
> 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

looks good, with a few minor comments.  please let me know what you think.

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?

modules/javafx.controls/src/test/java/test/javafx/scene/control/ControlLayoutTest.java line 69:

> 67:      */
> 68:     @ParameterizedTest
> 69:     @ValueSource(doubles = { 1.0, 1.25, 1.5, 1.75, 2.0, 2.25 })

also tried 0.333333, 0.00111, still looks good :-)

modules/javafx.controls/src/test/java/test/javafx/scene/control/ControlLayoutTest.java line 128:

> 126:         assertEquals(300, control.getHeight());
> 127:     }
> 128: 

extra newline?

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

PR Review: https://git.openjdk.org/jfx/pull/1229#pullrequestreview-1640365959
PR Review Comment: https://git.openjdk.org/jfx/pull/1229#discussion_r1334641544
PR Review Comment: https://git.openjdk.org/jfx/pull/1229#discussion_r1334645567
PR Review Comment: https://git.openjdk.org/jfx/pull/1229#discussion_r1334642702