On Sun, 11 Apr 2021 15:48:45 GMT, Florian Kirmaier <fkirma...@openjdk.org> wrote:
>> 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. My worry isn't the test might fail and give a false positive. The checking 10 times will be sufficient for that. My concern is that since this bug fix is fixing a leak that only happens when the stage is showing, the test might miss a potential leak if a regression in this area were ever reintroduced. I think it's best to restore the wait for onShown. ------------- PR: https://git.openjdk.java.net/jfx/pull/455
