On Mon, 1 May 2023 16:05:01 GMT, Andy Goryachev <ango...@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.
>
> 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?
Done
> 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?
Done
> 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?
Done
> 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
Done
> 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)
Done
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1121#discussion_r1182334803
PR Review Comment: https://git.openjdk.org/jfx/pull/1121#discussion_r1182334700
PR Review Comment: https://git.openjdk.org/jfx/pull/1121#discussion_r1182334626
PR Review Comment: https://git.openjdk.org/jfx/pull/1121#discussion_r1182334517
PR Review Comment: https://git.openjdk.org/jfx/pull/1121#discussion_r1182334449
