RFR: 8296919: Make system tests that detect memory leaks more robust by using JMemoryBuddy utility

[Andy Goryachev]

On Mon, 1 May 2023 14:48:32 GMT, Lukasz Kostyra <lkostyra at openjdk.org> wrote:

> Verified these changes on all platforms and saw no abnormalities in test execution.
> 
> This change is based on a preliminary patch done by @aghaisas , with some minor changes and the addition of one Leak-related web test. EventListenerLeak test was not touched, as it is a separate manual test app instead of a JUnit test.

LG, with minor suggestions.  What do you think?

build.gradle line 3468:

> 3466:         implementation project(":controls")
> 3467:         implementation project(":media")
> 3468:         testImplementation project(":base").sourceSets.test.output

does the header year need to be changed?

modules/javafx.web/src/test/java/test/javafx/scene/web/LeakTest.java line 50:

> 48: import org.w3c.dom.NodeList;
> 49: import static org.junit.Assert.*;
> 50: import test.util.memory.JMemoryBuddy;

same comment - year in the header?

tests/system/src/test/java/test/javafx/embed/swing/SwingNodeDnDMemoryLeakTest.java line 74:

> 72:         });
> 73: 
> 74:         for (WeakReference<SwingNode> ref : weakRefArrSN) {

I wonder if we should add a utility method to JMemoryBuddy to operate on an array/collection, so we don't have to repeat this code again and again?

tests/system/src/test/java/test/javafx/embed/swing/SwingNodeMemoryLeakTest.java line 82:

> 80:         SwingUtilities.invokeAndWait(() -> Util.sleep(500));
> 81: 
> 82:         for (WeakReference<SwingNode> ref : weakRefArrSN) {

same comment as before about a new utility method

tests/system/src/test/java/test/javafx/scene/control/TabPaneHeaderLeakTest.java line 98:

> 96: 
> 97:         JMemoryBuddy.assertCollectable(tabWeakRef);
> 98:         JMemoryBuddy.assertCollectable(textFieldWeakRef);

perhaps the utility method operating on an array should accept varargs, like so:

`assertCollectable(WeakRef a, WeakRef ... b)`

(to differentiate it from a single arg method, and to make sure there are 2+ args)

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

PR Review: https://git.openjdk.org/jfx/pull/1121#pullrequestreview-1407777336
PR Review Comment: https://git.openjdk.org/jfx/pull/1121#discussion_r1181683614
PR Review Comment: https://git.openjdk.org/jfx/pull/1121#discussion_r1181683783
PR Review Comment: https://git.openjdk.org/jfx/pull/1121#discussion_r1181685233
PR Review Comment: https://git.openjdk.org/jfx/pull/1121#discussion_r1181686826
PR Review Comment: https://git.openjdk.org/jfx/pull/1121#discussion_r1181687839