RFR: 8263322: Calling Application.launch on FX thread should throw IllegalStateException, but causes deadlock [v7]

Florian Kirmaier fkirmaier at openjdk.java.net
Sun Mar 21 22:43:40 UTC 2021


On Fri, 12 Mar 2021 16:29:35 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:

>> Florian Kirmaier has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   JDK-8263322
>>   Seperated the unit-tests into seperate files
>
> Thanks for splitting up the tests. I ran them with the fix and all pass, I then ran them without the fix and only the one I expected to fail did.
> 
> Before approving this, can you change the title of the JBS bug and CSR issue to match (after doing that you will also need to update this PR title) as recommended in the CSR? My suggestion is something like:
> 
> Calling Application.launch on FX thread should throw IllegalStateException, but causes deadlock
> 
> Another thing I recommend is to comment out or remove the print statements in the tests. They are useful while debugging, but in production we prefer tests to only print something as a warning or error. 
> 
> I also left a few inline comments.

I've made all the changes in the comments, and also changed the names of the tickets and the PR.
For some reason, I can't interact with the two unclosed comments.
I've removed the unreachable code, and the print-statements are also removed.

> tests/system/src/test/java/test/com/sun/javafx/application/InitializeJavaFXLaunch1Test.java line 41:
> 
>> 39: 
>> 40:     @Test (timeout = 15000)
>> 41:     public void testStartupThenLaunchInFX() throws Exception {
> 
> Would something like `testLaunchThenLaunchInFX` be a better name? As it is, it uses the same name as the test that really does use `Platform.startup` which I found confusing.

Yes, I've fixed the method names!

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

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


More information about the openjfx-dev mailing list