-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add documentation on how to test JavaFX app #5
Comments
I've added a lot of real world test samples to the Readme. |
Hi Florian, Thank you very much for your work in getting rid of memory leaks! I did see your links about real word examples (FYI some links are broken like the one for SpaceFX) but what I still fail to do is a very simple JavaFX test. Let's assume one creates a JavaFX Pane and after a while the pane gets invisible (i.e. not used anymore). I wanted to create a very simple test to check whether it is possible to detect whether the node (and maybe also its FXML controller) can be properly garbage collected. Attached a very simple SSCCE example (without FXML) that tries to check based on import de.sandec.jmemorybuddy.JMemoryBuddy;
import javafx.application.Platform;
import javafx.scene.Node;
import javafx.scene.Scene;
import javafx.scene.control.Button;
import javafx.scene.layout.BorderPane;
import javafx.scene.layout.Pane;
import javafx.stage.Stage;
import org.junit.jupiter.api.Test;
import org.testfx.framework.junit5.ApplicationTest;
import static org.junit.jupiter.api.Assertions.assertNotNull;
public class MemoryPaneTest extends ApplicationTest {
Scene scene;
BorderPane bp;
Node centerNode;
@Override
public void start(Stage stage) throws Exception {
this.bp = new BorderPane();
bp.setCenter(this.centerNode = getCenterNode());
scene = new Scene(bp, 800, 600);
stage.setScene(scene);
stage.show();
}
protected Node getCenterNode() throws Exception {
BorderPane p = new BorderPane();
bp.setCenter(new Button("ABC"));
return p;
}
@Test
public void check1() {
assertNotNull(scene);
assertNotNull(centerNode);
JMemoryBuddy.memoryTest(checker -> {
checker.assertNotCollectable(centerNode); // should not be collectable, still visible
});
// remove pane from scene
Platform.runLater(() -> {
this.bp.getChildren().remove(this.centerNode);
this.bp.setCenter(new Pane());
// checking here fails with --- Exception in Async Thread ---
});
try {
Thread.sleep(1000);
} catch (InterruptedException e) {
}
JMemoryBuddy.memoryTest(checker -> {
checker.assertCollectable(centerNode); // fails always
});
}
} |
The test doesn't work, because as long as the Test is running, all it's classmembers cannot be collected, so checking whether they are collectable always fails. I wouldn't use class members, when writing a memory test. It's better to put all variables in the Lambda of the memoryTest, this way they don't interfere with the test. If you do so anyways, then you would have to set them to null. Maybe this test is a better reference? https://github.com/openjdk/jfx/blob/fdc88341f1df8fb9c99356ada54b25124b77ea6e/tests/system/src/test/java/test/javafx/scene/control/ProgressIndicatorLeakTest.java |
Yes, this really helped a lot 👍 I followed that approach (i.e., the spirit of The surprising part to me was that I easily created a memory leak when adding a @Test
public void stageLeakWithMenuButton() throws Exception {
JMemoryBuddy.memoryTest((checker) -> {
CountDownLatch showingLatch = new CountDownLatch(1);
AtomicReference<Stage> stage = new AtomicReference<>();
TestUtil.runAndWait(() -> {
stage.set(new Stage());
BorderPane root = new BorderPane();
ToolBar tb = new ToolBar();
root.setTop(tb);
tb.getItems().add(new Button("A"));
tb.getItems().add(new Button("B"));
tb.getItems().add(new Button("C"));
// causes memory leak !!!
tb.getItems().add(new MenuButton("MB"));
stage.get().setScene(new Scene(root));
stage.get().setOnShown(l -> {
Platform.runLater(() -> showingLatch.countDown());
});
stage.get().show();
});
try {
assertTrue(showingLatch.await(15, TimeUnit.SECONDS), "Timeout waiting test stage");
} catch (InterruptedException e) {
throw new RuntimeException(e);
}
TestUtil.runAndWait(() -> {
stage.get().close();
});
checker.assertCollectable(stage.get());
});
} |
Before investigating it, which version of JavaFX are you using? It's worth mentioning, that many of these leaks are only noticed when writing such precise tests. In production, many leaks don't cause big issues because they don't build up over time. |
If i would guess - populating the native menu-bar might have a leak and remembers the last stage it was used with. Not a big problem in production - but with this kind of tests such bugs are easily found, fixed, and secured against regression. |
FYI: I am using 17-ea+16
I agree. The only problem is with such minor memory leaks it is more difficult to find the more crucial ones.. |
This test works for me:
So I guess MenuBar is fine. |
Mhh, interesting. In my case the test you provided also fails.
Question: Do you have any other boiler-code in the file since you removed I am using |
FYI: I put the test how I use it on GitHub for your convenience. Since GH is "Unable to open DISPLAY" it fails. Locally it fails with the afore mentioned message. |
I've added the whole test. Test should be green. But these tests might also be platform-dependent. I've run it on Mac. Yes, being Unable to open DISPLAY is a common error on CI with JavaFX. The test you've provided works on my machine. So the next step would be to look into the Heapdump and check the GC path. |
I see. Having said hat, this seems to be somewhat operating system dependent which is weird. I am seeing the message (I am on Windows) and GitHub actions on Windows does see the same message.
see https://github.com/danielpeintner/Java11Test/runs/3496191290?check_suite_focus=true#step:6:540
I used VisualVM but I am not very experienced... I don't really understand why the AtomicReference is there twice and the value is null which should be fine I think. |
No, this issue is not known. Many those issues are not known - they are close to invisible without JMemoryBuddy. To know what's happening, you will have to show the Path the "GC Root". There is a button for this in the upper right. |
I hope this is what you mean. To me it doesn't say a whole lot but maybe you can see the problem... What surprises me is that there are 2 |
The Atomic Reference is not really related to the leak. The two instances you are seeing, are internal implementation details of JavaFX.
|
I followed a related thread, see https://mail.openjdk.java.net/pipermail/openjfx-dev/2021-September/031925.html Having said that, the test provided works with JavaFX 16 (also on Windows) but fails with version 17. |
Can you provide an example on how to setup a test for a JavaFX app / control? Especially one that also works with the module system.
The text was updated successfully, but these errors were encountered: