On Thu, 27 Jan 2022 10:18:31 GMT, Jeanette Winzenburg <faste...@openjdk.org> wrote:
>> Hm, is this really needed? Not sure, are there any side effects by the >> `StageLoader` like this when a test failed? >> Just asking since the `StageLoader` is used a lot like this. >> And since the tests normally run green unless you may change something >> locally (on the JavaFX Pipeline it should never be red), it would probably >> never affect anything (or maybe it does?). > >> Hm, is this really needed? > > yes, IMO, we want the exact same cleanup for passing/failing tests. So either > dispose is required always (then need to make sure it's called on failure) or > not required always (then all its calls would be noise). > >> Not sure, are there any side effects by the `StageLoader` like this when a >> test failed? Just asking since the `StageLoader` is used a lot like this. > > don't now (and doesn't matter, what matters is the guaranteed cleanup) - and > aware of those slightly fishy patterns, we all learn :) Faintly remember > having discussed the point in a PR (can't find it right now, though), and > just as faintly remember the outcome was to guarantee the cleanup in new > tests. ah okay. Was just confusing for me since I never read that and I think some recent PRs still had this pattern, e.g. also the touch table scrolling PR I had a look at yesterday. Maybe for future it makes sense to have an abstract test class with the stage loader setup already in place. ------------- PR: https://git.openjdk.java.net/jfx/pull/669
