RFR: 8206430: Use consistent pattern for startup in FX system tests

[Kevin Rushforth]

On Tue, 15 Nov 2022 18:00:59 GMT, Andy Goryachev <angorya at openjdk.org> wrote:

> 1. Introduced the following utility methods:
> - Util.launch
> - Util.shutdown
> - Util.waitForLatch
> 2. Fixed the out-of order calls to Stage.hide() and Platform.exit() in many tests' shutdowns.
> 3. Replaced local waitForLatch copies with Util.waitForLatch

I left a couple quick comments on the new utility methods in `test.util.Util`.

Have you run a full test (including robot) on all three platforms?

> Fixed the out-of order calls to Stage.hide() and Platform.exit() in many tests' shutdowns.

I am not aware of any out of order calls. Perhaps you are referring to the following pattern:


Platform.runLater(() -> stage.hide());
Platform.exit();


That is actually valid, since `Platform.exit()` can be called on any thread. The FX runtime doesn't exit until all pending runnables are run. Doing the exit on the FX application thread is fine, too.

tests/system/src/test/java/test/util/Util.java line 317:

> 315:      * @param args - command line arguments
> 316:      */
> 317:     public static <T extends Application> void launch (

This looks like a useful utility. I think it would be helpful to either remove the timeout parameter entirely, or else have a variant of this that doesn't take the timeout value, and use a default timeout value. Most tests use 10 or 15 seconds so standardizing on 15 seems reasonable.

Also, many of the tests use `Platform.startup` since they don't need to launch an application. Have you looked at providing a similar utility method for those cases?

tests/system/src/test/java/test/util/Util.java line 338:

> 336:     public static void shutdown(Stage... stages) {
> 337:         // why isn't runAndWait() exposed as a public API?
> 338:         PlatformImpl.runAndWait(() -> {

Not providing a public `runAndWait` was a deliberate choice. In any case, tests or utilities that need to wait should use the `runAndWait` method in this Util class in preference to `PlatformImpl`. For one thing it will throw exceptions back to the caller where as the `PlatformImpl` method will not.

I think `Platform.runLater` should be sufficient here, but as long as the tests are still stable when using `runAndWait`, I don't have a strong objection.

-------------

PR: https://git.openjdk.org/jfx/pull/950