RFR: 8296330: Tests: Add layout container snapping tests

Marius Hanl mhanl at openjdk.org
Sat Nov 5 13:11:59 UTC 2022


On Fri, 4 Nov 2022 20:42:07 GMT, Andy Goryachev <angorya at openjdk.org> wrote:

>> While checking https://bugs.openjdk.org/browse/JDK-8295078 I found much more layout container which do not snap their children correctly.
>> 
>> The goal of this PR is to add snapping tests for all layout container.
>> After that we can create issues for all broken layout container, fix them and comment in the corresponding line.
>
> modules/javafx.graphics/src/test/java/test/javafx/scene/layout/SnappingTest.java line 46:
> 
>> 44: import static org.junit.jupiter.api.Assertions.*;
>> 45: 
>> 46: class SnappingTest {
> 
> should it be `public class SnappingTest`?
> 
> Also, perhaps a brief javadoc explaining the purpose of this test might help.

JUnit5 classes do not need to be public.
Sure, I will add javadoc. :)

> modules/javafx.graphics/src/test/java/test/javafx/scene/layout/SnappingTest.java line 85:
> 
>> 83:     @MethodSource(value = { "getContainerInstruction" })
>> 84:     void testContainerSnappingScale200(ContainerInstruction<Region> containerInstruction) {
>> 85:         testContainerSnappingImpl(containerInstruction, 2);
> 
> minor: do you think there is a need for other scale factors?

I don't think so. We could add more but I don't see how this will give us any more information/benefits.

> modules/javafx.graphics/src/test/java/test/javafx/scene/layout/SnappingTest.java line 127:
> 
>> 125: 
>> 126:         assertEquals(widthHeight + snappedPaddingX, container.prefWidth(-1), 0.00001, className);
>> 127:         assertEquals(widthHeight + snappedPaddingY, container.prefHeight(-1), 0.00001, className);
> 
> perhaps this should be a constant?

Sure, will do.

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

PR: https://git.openjdk.org/jfx/pull/936


More information about the openjfx-dev mailing list