On Tue, 23 Jun 2026 11:10:56 GMT, Marius Hanl <[email protected]> wrote:
>> Florian Kirmaier has updated the pull request incrementally with two
>> additional commits since the last revision:
>>
>> - JDK-8386590
>> Removed some magic values.
>> - JDK-8386590
>> Small code simplification.
>
> modules/javafx.graphics/src/test/java/test/javafx/scene/layout/GridPaneTest.java
> line 3198:
>
>> 3196: void checkSize() {
>> 3197: // This should work when the Node is laid out properly.
>> 3198: assertEquals(AREA, getWidth() * getHeight(), 20);
>
> From my testig, a `delta` of 10 was enough. Thats why I was a bit confused
> and also suggested to use precise assertions instead.
>
> I understand you want to keep that for future tests, which is fine for me.
> But I'm a bit afraid that a high delta may masks some bugs where the test
> should fail but will pass because it fits into the delta of 20. Especially
> since v/hgap is usually set to 4 or 5, and we only have 2 cols/rows, a delta
> of 20 can be enough to e.g. hide the fact that a gap was not calculated in
> correctly or similar situations.
>
> Does that makes sense?
Thank you for the quick response.
It only looks high because it compares the area - this basically
represents 0.2 pixels. I'm not sure whether the snap-to-pixel logic
will require a higher value in some other cases. A value up to
1.0 pixels (100) would be in a range that makes sense to me.
I could do something like the following, if that's considered more readable:
AverageSide = Math.sqrt(getWidth() * getHeight());
assertEquals(100, AverageSide, 0.2);
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/2187#discussion_r3459505496
