On Fri, 9 Apr 2021 11:39:34 GMT, Florian Kirmaier <fkirma...@openjdk.org> wrote:
>> Fixing leak in ProgressIndicator when treeShowing is false > > Florian Kirmaier has updated the pull request incrementally with one > additional commit since the last revision: > > 8264677 > Updated naming of the test, > reworked the old test. It now has a much smaller scope and is easier to > reason about, the initialization of JavaFX is now seperated from the test > itself. The fix looks good. So does the newly added test, and I like the refactoring of the tests that you've done. I left one question about existing test. tests/system/src/test/java/test/javafx/scene/control/ProgressIndicatorLeakTest.java line 79: > 77: indicator.setProgress(1.0); > 78: checker.assertCollectable(detIndicator); > 79: stage.show(); Now that you are no longer waiting for `Stage::onShown`, isn't it possible for the test to miss a potential leak? Can you explain why you think it's not needed? ------------- PR: https://git.openjdk.java.net/jfx/pull/455
