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

[Andy Goryachev]

On Sat, 2 Sep 2023 13:09:14 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: Set a min size

modules/javafx.controls/src/test/java/test/javafx/scene/control/ControlContractTest.java line 44:

> 42: import static org.junit.jupiter.api.Assertions.assertTrue;
> 43: 
> 44: class ControlContractTest {

Do you plan to add more tests to this class?  The name does not specify which contract is being tested.

modules/javafx.controls/src/test/java/test/javafx/scene/control/ControlContractTest.java line 46:

> 44: class ControlContractTest {
> 45: 
> 46:     private ControlStub control;

Do you think it's worth to run this test against all of the existing Controls?

modules/javafx.controls/src/test/java/test/javafx/scene/control/ControlContractTest.java line 67:

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

Could you please add 2.25 (I can see this one with the secondary monitor attached to my win11 box)

modules/javafx.controls/src/test/java/test/javafx/scene/control/ControlContractTest.java line 101:

> 99: 
> 100:                 double expectedWidth = control.snapSizeX(control.getWidth()) - control.snappedLeftInset() - control.snappedRightInset();
> 101:                 assertEquals(expectedWidth, w);

since we are performing floating point operations, we might accumulate small error.  
So this and 3 subsequent assertEquals() would need an EPSILON = 0.0000001 (an arbitrary value)

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1229#discussion_r1319158123
PR Review Comment: https://git.openjdk.org/jfx/pull/1229#discussion_r1319159680
PR Review Comment: https://git.openjdk.org/jfx/pull/1229#discussion_r1319150932
PR Review Comment: https://git.openjdk.org/jfx/pull/1229#discussion_r1319155511