Description Paul Pazderski 2020-01-23 18:22:08 EST

Well 'some' is a little exaggerated. I found two tests opening but not closing a modal/modeless shell/dialog.

Because this text might get a bit long here is a summary:
 - fix tests leaking modal shells
 - as consequence TextHandlerTests should not fail anymore or fail for another reason
 - add protection to prevent further modal shell leaks.
 - fix testPreviousChoicesAvailable test


But the consequences of those leaks are awful. It is responsible for the test failure of TextHandlerTest testEditableText (Bug 544675) and testNonEditableText (Bug 552614). One reason I did not directly attached the patch there is that I'm very sure for testEditableText and quite sure for testNonEditableText that they will continue failing on Windows for another reason after this is fixed. (failing on another assert so we can see if my assumption is correct)

So why does a leaked modal shell fail this test and why does it not fail on Linux? The tests fail on the first enablement of the copy handler. The text widget is empty therefor the handler should be disabled. But it is not on Mac and Windows. The reason lies in the enablement update code (I do not plan to change) if there is an active text widget the selection determined the enablement (select -> enabled). If no text widget is active the actions enablement state is used which is always true for this test.

The 'active text widget' is set from focus events. And here is the failure. Due to the open modal shell the text widget cannot given focus. It does not work on Windows and I assume it does not work on Mac as well.
On Linux however the set focus works regardless of the open modal dialog.


One test missing cleanup can fail an entirely different test without clear connection and maybe hard to debug defects in the actual correct test.
To prevent such problems I'd like to add a check in UITestCase.tearDown to close any leaked modal shell and fail the offending test.
This is not perfect because not any test is using UITestCase and if such a test does not close a modal dialog the next UITestCase would report the failure. However any test failure on a gerrit build should be investigated and even if this is not always the case due to unstable tests a leaked modal shell should always considered a sever failure.


Something that might look unrelated but is probably not is QuickAccessDialogTest.testPreviousChoicesAvailable this test failed after I correctly closed the dialogs in the two bad tests. I cannot fully explain the connection. It seem that Mickael already had some trouble [1] with this test which might be related to those shell leaks.