On Fri, 9 Apr 2021 12:37:07 GMT, Kevin Rushforth <k...@openjdk.org> wrote:
>> 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. > > 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? That's a good question. OnShown is usually called in about 1 Frame. The memory-leak test takes some times, usually longer as 1 Frame. In the current configuration, it takes up to 1 seconds, with up to 10 checks. Because the check is so much longer, waiting for onShown doesn't really make a difference. ------------- PR: https://git.openjdk.java.net/jfx/pull/455
