On Tue, 3 Mar 2020 22:51:43 GMT, Kevin Rushforth <k...@openjdk.org> wrote:
> While testing JavaFX using gradle 6.3-nightly and JDK 14, I ran into what
> turned out to be a latent test bug in the
> AppJSCallback* apps that are launched by ModuleLauncherTest. The launched
> apps construct a WebView instance, obtain a
> WebEngine instance from the WebView, add a state listener to the WebEngine,
> load the web content, and then return from
> the Application::start method. The WebView instance is held a local variable,
> and so is subject to garbage collection
> once it goes out of scope when the start method returns. In addition, the
> test apps did not check for successful
> launching of the application or successful loading of the web content, which
> led to the test suite hanging (else it
> would have been a simple test failure). Note that I don't know (nor care)
> what changed in JDK 14 to make this more
> likely to occur, but since the test itself is demonstrably wrong, it needs to
> be fixed. The main part of the fix was
> to make the WebEngine an instance variable. In order to make the test more
> robust, I also modified it to launch the
> application in another thread, and having the main thread check that the
> application launched successfully and that the
> web content was loaded successfully, after which I did the assertion check
> for the correct number of callbacks. The 5
> different apps only differ from each other in the name of the class, the name
> of the package from which MyCallback is
> imported, and possibly the expected number of callbacks. I diffed
> AppJSCallbackExported.java against the other 4 test
> files and the diffs are essentially the same as they were before this change.
> Reviewers thus only need to review one of
> the tests in detail. For example, here are the diffs between
> AppJSCallbackExported and AppJSCallbackUnexported: $
> diff .../AppJSCallbackExported.java .../AppJSCallbackUnexported.java 37c37 <
> import myapp5.pkg2.MyCallback;
> ---
>> import myapp5.pkg1.MyCallback;
> 45c45
> < public class AppJSCallbackExported extends Application {
> ---
>> public class AppJSCallbackUnexported extends Application {
> 77c77
> < Util.assertEquals(1, callbackCount);
> ---
>> Util.assertEquals(0, callbackCount);
Marked as reviewed by aghaisas (Reviewer).
-------------
PR: https://git.openjdk.java.net/jfx/pull/134
