On Wed, 4 Mar 2020 14:02:51 GMT, Erik Helin <ehe...@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);
>
> Please ignore this comment, this is for debugging Skara that might have some 
> mailing list mirroring issues 😄

Please ignore this comment as well, it is also for debugging issues with Skara 
and mailman :email: 😠

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

PR: https://git.openjdk.java.net/jfx/pull/134

Reply via email to