JDK-8176884 : Window-based unit tests need to be more robust
  • Type: Bug
  • Component: javafx
  • Sub-Component: window-toolkit
  • Affected Version: 9
  • Priority: P4
  • Status: Open
  • Resolution: Unresolved
  • Submitted: 2017-03-16
  • Updated: 2018-12-01
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
Other
tbdUnresolved
Related Reports
Blocks :  
JDK-8176902 - Add waitForIdle support to FX Robot
Relates :  
JDK-8192800 - Table auto resize ignores column resize policy
Relates :  
JDK-8179980 - Reenable RestoreStagePositionTest for Mac
Description
The following new unit tests (at least) have some general stability issues that should be addressed:

test/javafx/scene/NewSceneSizeTest.java
test/javafx/scene/RestoreSceneSizeTest.java
test/javafx/stage/DeiconifiedWithChildTest.java
test/javafx/stage/RestoreStagePositionTest.java
test/robot/javafx/scene/AfterModalClosedTest.java

Most of the problems stem from the asynchronous nature of operations that happen when doing various window-related operations such as show(), hide(), etc. This problem is made worse by submitting the operations via a runLater and then not waiting for the runLater to be executed before the main thread proceeds (in many cases using a short sleep in place of a latch or a runAndWait).

Some of the issues would be helped by enhancements to glass Robot to allow for waiting until an operation is done, but in the mean time we can and should still make the tests as robust as possible.

I have two specific recommendations regarding the above tests.

1. Replace all uses of Platform.runLater in the main test thread with Util.runAndWait if they don't already wait on a latch that is directly counted down in the target (Runnable) of that runLater. This has two benefits: first we will be assured that the commands in the target of the runLater have actually executed before we continue; second, any exceptions that occur in the target of the runLater will be captured and re-thrown to the caller.

2. Any place where we are sleeping to wait for a window to be shown, hidden, minimized, maximized, restored, made full-screen, or made non-full-screen should sleep for 1 second rather than a shorter amount. This isn't a fool-proof number, but many of our other system tests use this number and it has been proven over time to make those tests robust enough for use across a wide range of machines.
Comments
Kevin, I agree that after runLater() + 200ms delay there is a small chance that the action has not been yet executed (a very very small chance). But this is not the root cause of the issue and runAndWait() won't improve the situation a lot. The delay was added not to wait until the scheduled runnable is executed (delay can be much less for that purpose) but to wait until the native platform really maximizes the window. If animation is enabled it may take a while. I afraid, without resolving JDK-8179980 it is impossible to make this test stable on Mac platform. Why not wait for JDK-8179980? Of cause, your recommendations will be taken into account as well. Another way to fix the test is to extend the binding API to enable receiving of the real state change notification coming from the native platform.
09-05-2017
I still think the recommendations in this JBS issue might be worth looking at for 10, even before JDK-8176902 is fixed. Specifically, it would help with test robustness if we increased the sleep values for places where we show windows (see Dave's comment in JDK-8179980). Also, independent of the fix for JDK-8176902, we should use runAndWait or runLater + CountDownLatch rather than runLater + sleep everywhere it makes sense to do so. The latter is not robust since even a medium-size sleep can finish before the target of the runLater executes.
09-05-2017
I could not reproduce instability on Linux for 10 runs. I suggest to postpone the bug until JDK-8176902 is fixed.
04-04-2017
test/robot/javafx/scene/AfterModalClosedTest.java Cannot reproduce any issues with this test on Mac.
30-03-2017
test/javafx/stage/RestoreStagePositionTest.java Both cases of the test fails on Mac because of long transition animation delay and because it is not possible to determine time when window is really in full-sreeen/maximized state on Mac.
30-03-2017
test/javafx/stage/DeiconifiedWithChildTest.java Could not get failures of this test for 20 runs on Mac. But I think it also should be improved to receive actual Iconified/Deiconified notifications.
30-03-2017
test/javafx/scene/RestoreSceneSizeTest.java the failure of the test take place on Mac if windows animation is on. The test initially makes a window full-screen and then restores its size but the moment when window is actually made full-screen cannot be determined and the second step of the test is executed too early. By default Mac has very long transitional process to make a window full-screen because of animation effects. So, this is the same issue as described for previous test.
29-03-2017
Mac platform: test/javafx/scene/NewSceneSizeTest.java The test fails on Mac because of two reasons: 1. showing a new window takes on Mac much more time than on other platforms (5 seconds is not enough to create 100 windows). 2. it is not possible to receive the window change notification when it is really changed in the native platform. The latter is a general FX issue. When a window property gets a value from FX level the FX bound property receives the value and notify the attached property change listeners about the change, then the value is sent to the native platform. The native platform makes the real change of the window property (or it may reject it) and notify FX back about the change. But if the change was successful then FX property is notified about the same value it already has and the FX property change listeners are not triggered because of the next generic code: protected void fireValueChangedEvent() { final T oldValue = currentValue; currentValue = observable.getValue(); final boolean changed = (currentValue == null)? (oldValue != null) : !currentValue.equals(oldValue); if (changed) { try { listener.changed(observable, oldValue, currentValue); } catch (Exception e) { Thread.currentThread().getUncaughtExceptionHandler().uncaughtException(Thread.currentThread(), e); } } } So, if changing the native window property takes time (for example because of animation) it is not possible on FX level to catch the moment when the native window state is really changed. This may also affect FX application development because FX developer cannot be sure that the FX property he set is actually set to the window. This issue can be avoided only by adding a big delay which is bad practice because it makes execution time too big, or it can be reduced by disabling window animation in native system. I suggest to file a separate bug to extend the API to allow to receive native notifications on FX level about the actual window property changes. Then the test may be corrected to use this API to invoke testing steps at the right time.
29-03-2017
I'll take a look at the results, but the problem will not always show up depending on your machine and what else it is doing at the time. One way to see the problem is to add a sleep into a Runnable that is passed to runLater. This will simulate the problem that can happen if the runnable isn't scheduled right away. If it fails (which it will for many of the tests), then there is a problem. We used this technique when debugging the JSMemoryCallBackTest to make it more robust (see JDK-8172694). Another thing to do is to run the tests on different platforms and see whether there are any failures there (e.g., the timing for window operations is different on Mac than on Linux).
22-03-2017
Kevin, I ran the system tests suite on windows platform for 10 times. There were number of tests that failed but no one from the list you provided. The attached file test_runs.zip contains test reports for all runs.
22-03-2017