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

Kevin Rushforth kcr at openjdk.org
Thu May 4 12:20:24 UTC 2023


On Thu, 4 May 2023 08:21:15 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.
>
> Lukasz Kostyra has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Update javafx.web classpath to include JMemoryBuddy

The changes generally look good, although I left a comment about sleeping on the EDT.

modules/javafx.base/src/test/java/test/util/memory/JMemoryBuddy.java line 95:

> 93:     }
> 94: 
> 95:     public static void assertCollectable(WeakReference[] weakReferences) {

Can you add javadoc comments?

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

> 78:         });
> 79: 
> 80:         SwingUtilities.invokeAndWait(() -> Util.sleep(500));

Sleeping on either the JavaFX Application thread or the Swing EDT (as in this case) is not generally desirable. What is the sleep trying to achieve? Is there a better way, possibly sleeping on the test thread and then doing a `SwingUtilities.invokeAndWait` (maybe with a no-op runnable? not sure)?.

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

PR Review: https://git.openjdk.org/jfx/pull/1121#pullrequestreview-1412937420
PR Review Comment: https://git.openjdk.org/jfx/pull/1121#discussion_r1184918401
PR Review Comment: https://git.openjdk.org/jfx/pull/1121#discussion_r1184923206


More information about the openjfx-dev mailing list