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

[Andy Goryachev]

On Tue, 12 Sep 2023 07:07:27 GMT, Marius Hanl <mhanl at 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