On Fri, 3 Dec 2021 10:20:43 GMT, Ajit Ghaisas <[email protected]> wrote:
>> Johan Vos has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Move functionality in the setCellCount() to the invalidated block.
>> Some hard numbers used in tests (counters for evaluations) were changed
>> because of this.
>> Instead of relying on hard values, I modified the failing was into
>> relative ones.
>
> modules/javafx.controls/src/test/java/test/javafx/scene/control/ListViewTest.java
> line 1124:
>
>> 1122: Platform.runLater(() -> {
>> 1123: Toolkit.getToolkit().firePulse();
>> 1124: assertTrue(rt_35395_counter < 7);
>
> I see that you have modified assertions to use "lesser than" some expected
> value. This may hide some incorrect test outcomes.
> Along with "lesser than" assertion, do you think we should add a "greater
> than" assertion as well? This will have a range bounded expected value.
> This is applicable for all assertion changes in this PR.
The hard values have been changed a number of times, and I believe it is not
really a good metric.
What we want to ensure is that there is no functional regression and no
performance penalty. The tests calculate the number of updateItem invocations
and require those to be a strict number. With JDK-8089589, we fixed a number of
issues that are related to the fact that the total size of the view (in case
all cells are rendered with their preferred height) is not known. This is done
by using an estimate of the total size. The estimate is 100% correct if we call
updateItem for every item, but that would lead to a huge performance penalty in
case the list contains a large amount of items with equal height.
Hence, there is a balance between minimizing the updateItem calls but still
have a fair estimate of the total dimensions. Rather than requiring that the
amount of calls should be a fixed number, I think it makes more sense to ensure
that the amount of calls stays within reasonable boundaries, and is not growing
exponentially when we add linearly more items, for example.
-------------
PR: https://git.openjdk.java.net/jfx/pull/683
