On Sat, 2 Sep 2023 13:09:14 GMT, Marius Hanl <mh...@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