RFR: 8307538: Memory leak in TreeTableView when calling refresh [v2]
Kevin Rushforth
kcr at openjdk.org
Tue May 9 22:56:21 UTC 2023
On Mon, 8 May 2023 18:55:38 GMT, Andy Goryachev <angorya at openjdk.org> wrote:
>> Fixed a memory leak in TreeTableView by using WeakInvalidationListener (which is the right approach in this particular situation) - the leak is specific to TreeTableRowSkin.
>>
>> Added a unit test.
>>
>> Added Refresh buttons and Skin menu to the Monkey Tester (will expand these to other controls as a part next MT update).
>
> Andy Goryachev has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 10 additional commits since the last revision:
>
> - removed monkey tester changes
> - Merge remote-tracking branch 'origin/master' into 8307538.refresh
> - whitespace
> - updated test
> - variable tree cell fix
> - fixed size selector
> - list view page
> - fix and test
> - skin menu
> - refresh button
If I run just the `TreeTableRowSkinTest` test class, I now get a test failure:
$ gradle --info -PTEST_ONLY=true :controls:test --tests TreeTableRowSkinTest
TreeTableRowSkinTest > treeTableRowWithFixedCellSizeShouldIgnoreVerticalPadding() FAILED
org.opentest4j.AssertionFailedError: expected: <18.0> but was: <23.0>
at app//org.junit.jupiter.api.AssertionUtils.fail(AssertionUtils.java:55)
at app//org.junit.jupiter.api.AssertionUtils.failNotEqual(AssertionUtils.java:62)
at app//org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:86)
at app//org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:81)
at app//org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:1010)
at app//test.javafx.scene.control.skin.TreeTableRowSkinTest.treeTableRowWithFixedCellSizeShouldIgnoreVerticalPadding(TreeTableRowSkinTest.java:251)
Given that the GHA run passes, as does a full controls test run on my system, that suggests something odd is going on with that test, possibly it is affected by tests in another class. If so, then it could fail randomly anyway, since the tests are run in an unspecified (and unpredictable) order.
@aghaisas can you review this?
modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/TreeTableRowSkinTest.java line 215:
> 213: int right = 23;
> 214: int bottom = 37;
> 215: int left = 49;
After the fix and test change, this test now fails for me.
-------------
PR Review: https://git.openjdk.org/jfx/pull/1129#pullrequestreview-1419545337
PR Comment: https://git.openjdk.org/jfx/pull/1129#issuecomment-1540988869
PR Review Comment: https://git.openjdk.org/jfx/pull/1129#discussion_r1189195101
More information about the openjfx-dev
mailing list