710143 - Investigate why we fail add-ons manager tests with "Modal dialog has been found and processed" on the Mozmill side

Status: RESOLVED WORKSFORME

(Mozilla QA Graveyard :: Mozmill Tests, defect)

Description

[Maniac Vlad Florin (:vladmaniac)]

We are failing almost all add-ons manager restart tests with 

TimeoutError("Modal dialog has been found and processed")@resource://mozmill/modules/utils.js:429 waitFor([object Proxy],"Modal dialog has been found and processed",25000,100,[object Proxy])@resource://mozmill/modules/utils.js:467 

We have reproduced sometimes manually this, therefore we have bug 702266 for that. 
This bug is intended to track investigation of this failure on the Mozmill side

Please do not dupe this, since it's a general tracking bug

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

Which branches and platforms are affected? I don't think it's only Linux as given in the OS field. I really miss this information in the initial comment.

Comment 2

[Maniac Vlad Florin (:vladmaniac)]

(In reply to Henrik Skupin (:whimboo) from comment #1)
> Which branches and platforms are affected? I don't think it's only Linux as
> given in the OS field. I really miss this information in the initial comment.

Sorry for missing this important detail - all platforms are affected

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

So when has this been started exactly?

Comment 4

[u279076]

I believe it's been happening intermittently with various add-ons manager tests since we started developing them over a quarter ago. I'll let Vlad confirm that though.

Comment 5

[Maniac Vlad Florin (:vladmaniac)]

(In reply to Henrik Skupin (:whimboo) from comment #3)
> So when has this been started exactly?

From the moment we started testing add-on installation 

We are not failing modal dialog handling on other tests, so I tend to think our API is ok - the failure for add-on installation tests are caused by the hang of the download progress bar see bug 702266

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

Which means we are running into a timeout in the test, continue with the next one, and sometimes later the add-on installation dialog comes up? Hm, that would be messy.

Dave, is there a back-end API to cancel a running download/installation?

Comment 7

[Dave Townsend [:mossop]]

(In reply to Henrik Skupin (:whimboo) from comment #6)
> Which means we are running into a timeout in the test, continue with the
> next one, and sometimes later the add-on installation dialog comes up? Hm,
> that would be messy.
> 
> Dave, is there a back-end API to cancel a running download/installation?

You can call AddonManager.getAllInstalls and call cancel on each one. I think in this case that might still cause a doorhanger to appear telling the user the download was cancelled.

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

That would be totally ok. The only thing we should not end-up with for the time being is a non-handled modal dialog window.

Vlad, can you please check the proposal from Dave and how we can get this implemented? If not, I will have a look at hopefully next week.

Comment 9

[Remus Pop (:RemusPop)]

I also seem to encounter this issue in tests from testSearch where we have the Manage Search Engines modal-dialog. Tested on beta, nightly and release.

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

Remus, this is different and has to be filed as a separate bug. Please concentrate on add-on related tests here.

Comment 11

[Remus Pop (:RemusPop)]

Sorry, this was a false alarm coming from a timeout in a waitFor call.

Comment 12

[u279076]

Henrik or Vlad, is there a status update here? It's been over a month since the last meaningful comment.

Comment 13

[Maniac Vlad Florin (:vladmaniac)]

(In reply to Anthony Hughes, Mozilla QA (irc: ashughes) from comment #12)
> Henrik or Vlad, is there a status update here? It's been over a month since
> the last meaningful comment.

As I've stated in the meeting wiki noted I've backlogged this bug for the moment because I'm out of ideas for it..I can't think of a possible fix on the mozmill side.

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

Vlad, not sure what you are talking about but have you checked the feedback from Dave in comment 7? I never got a reply from you based on this proposal.

Comment 15

[Maniac Vlad Florin (:vladmaniac)]

(In reply to Henrik Skupin (:whimboo) from comment #14)
> Vlad, not sure what you are talking about but have you checked the feedback
> from Dave in comment 7? I never got a reply from you based on this proposal.

Why do I want to cancel a download/install? I did not understand that part

Comment 16

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

This would be helpful for teardown, so that we can ensure that no more download is in progress which would raise the installation dialog and block the UI.

Comment 17

[Maniac Vlad Florin (:vladmaniac)]

Attachment: wip patch1

Used testInstallTheme and update TIMEOUT in addons.js because I get 

"Selected category has been loaded." Timeout error in addonsManager.open() - this fixes it. 

Now every time the install dialog freezes, we cancel the install in teardown module

Comment 18

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

Comment on attachment 593748 [details] [diff] [review]
wip patch1

>+++ b/tests/functional/restartTests/testAddons_installTheme/test1.js
> 
>+Components.utils.import("resource://gre/modules/AddonManager.jsm");
[..]
>+function teardownModule() {
>+  AddonManager.getAllInstalls(function (aInstalls) {
>+    if (aInstalls.length == 1) {
>+      aInstalls[0].cancel();

Patch looks fine. But please don't check if it equals to 1. Instead we want to cancel all installation requests which are in the queue.

Also this code has to land in the addons.js module, where it can be used by any of our tests which attempt to install add-ons. Feel free to create a new bug under shared modules.

>-// Bug 701893 - Failure in testAddons_installTheme/test1.js
>-setupModule.__force_skip__ = "Bug 701893 - Failure in " + 
>-                             "testAddons_installTheme/test1.js";
>-teardownModule.__force_skip__ = "Bug 701893 - Failure in " + 
>-                                "testAddons_installTheme/test1.js";

I wouldn't unskip the test yet because it will still fail with the installation, or does a timeout of 7000ms change that?

Comment 19

[Maniac Vlad Florin (:vladmaniac)]

Thanks for feedback - this was a WIP to see if I properly understood what we want and I use the backend API stuff properly. The rest will be addressed in a further patch. 

the new timeout is enough for the test to pass. tested against mac and linux.

Comment 20

[Maniac Vlad Florin (:vladmaniac)]

I'm thinking on passing this code to a shared module function under bug 723470 in which we also need to handle the behavior when we actually cancel an install (new modal dialog for e.g)

Comment 21

[u279076]

(In reply to Maniac Vlad Florin (:vladmaniac) from comment #20)
> I'm thinking on passing this code to a shared module function under bug
> 723470 in which we also need to handle the behavior when we actually cancel
> an install (new modal dialog for e.g)

Sounds fine to me. Any code which can benefit multiple tests *should* go into the Shared Modules anyway, in my opinion.

Comment 22

[Maniac Vlad Florin (:vladmaniac)]

Right now we are concentrating on re-enabling addons manager tests which failed with this error some time ago. We are working on the failure bugs themselves (dependencies here)

Not sure what updates would be necessary for this bug report but I think we will not have any patches here. Should we close it?

Comment 23

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

I think that we are ok now and don't have to do any more investigation. Please concentrate on enabling skipped tests.