On Tue, 12 Sep 2023 07:07:27 GMT, Marius Hanl <mh...@openjdk.org> wrote:
>> 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.
>
> Do you a better idea? I guess it would make sense to rename it to something
> more generic. Maybe ControlLayoutTest?
ControlLayoutChildrenContractTest perhaps, if that's the only thing being
tested here (that is, if you don't plan to add anything to this test).
Also, would it make sense to move javadoc from L58 to L43 since it describes
the test (assuming no more changes)
>> 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)
>
> Will have a look, but it worked without so I did not bothered adding an
> epsilon to the tests
I would have added an EPSILON. I suspect we just got lucky that with the
default padding and a small subset of scales everything comes out ok.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1229#discussion_r1323684765
PR Review Comment: https://git.openjdk.org/jfx/pull/1229#discussion_r1323687129
