On Mon, 18 Nov 2024 20:11:50 GMT, Marius Hanl <mh...@openjdk.org> wrote:
>> modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/TableViewTableHeaderRowTest.java >> line 261: >> >>> 259: /** >>> 260: * Tests that toggling the column visibility does not cause memory >>> leaks. >>> 261: * See also: <a >>> href="https://bugs.openjdk.org/browse/JDK-8341687";>JDK-8341687</a>. >> >> same comment - does not test for memory leak. the memory leak test might >> involve `JMemoryBuddy` - see for example `SystemMenuBarTest`. > > I tried, but it isn't that easy - since the memory is not leaked because we > keep some references we should not keep - rather we are adding listener again > and again and again. So we can not assert things to be collectable, as they > should not be, still they increase the memory footprint instead. > > So I did not manage to make a test with `JMemoryBuddy`, but can prove the > point with the amount of listener instead. > > I wrote a test that checked, that the memory is similar high as before, that > worked as well but I was afraid that is might be flaky, which I want to avoid. I think this is an acceptable explanation as it tests the root cause of the leak. Do you think changing the skin would allow us to create a test for the memory leak specifically. Maybe some variation of `SkinMemoryLeakTest` ? ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1640#discussion_r1847219724
