Closed Bug 783484 Opened 8 years ago Closed 7 years ago

Test failure 'Shutdown expected but none detected before end of test ' in /restartTests/testAddons_uninstallExtension/test4.js

Categories

(Mozilla QA Graveyard :: Mozmill Tests, defect, P2)

defect

Tracking

(firefox20 wontfix, firefox21 wontfix, firefox22 wontfix, firefox23 wontfix, firefox27 wontfix, firefox28 fixed, firefox29 fixed, firefox30 fixed, firefox31 fixed, firefox-esr17 wontfix, firefox-esr24 fixed)

RESOLVED FIXED
Tracking Status
firefox20 --- wontfix
firefox21 --- wontfix
firefox22 --- wontfix
firefox23 --- wontfix
firefox27 --- wontfix
firefox28 --- fixed
firefox29 --- fixed
firefox30 --- fixed
firefox31 --- fixed
firefox-esr17 --- wontfix
firefox-esr24 --- fixed

People

(Reporter: whimboo, Assigned: andrei)

References

()

Details

(Whiteboard: [mozmill-test-failure])

Attachments

(7 files, 2 obsolete files)

The test has been started to fail recently. Can someone please check if this is a regression?

Failure message: "Shutdown expected but none detected before end of test"
OS: All → Mac OS X
Yes - seen this yesterday evening happening. I'll take it. Thanks for submitting the bug
Assignee: nobody → vlad.mozbugs
Status: NEW → ASSIGNED
Looks like I had no luck running into this on my testing box. 

http://mozmill-crowd.blargon7.com/#/functional/reports?branch=All&platform=All&from=2012-08-17&to=2012-08-17

I was trying more with the fr locale which is likely more to fail, but still no luck. 
Judging by the error it seems like we never get the shutdown command, which in our case would be clicking on the restart link in add-ons manager
So this happens no more. If we check the URL section in the bug, it has aprox one month of clean results. Therefore WFM until further notice
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → WORKSFORME
I saw this failure today with the 17rc build on Mac OSX 10.8.2:
http://mozmill-ci.blargon7.com/#/functional/report/674977957b923f4905160d1b9a3a43ef
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
Putting back into the pool for watching those results.
Assignee: vlad.mozbugs → nobody
Status: REOPENED → ASSIGNED
QA Contact: mozmill-tests → hskupin
Given that it fails for Firefox 17 we have to take care about it because we will get a lot of esr17 releases. Lets work on this next week.
Whiteboard: [mozmill-test-failure] → [mozmill-test-failure] s=121126 u=failure c=addons p=1
Assignee: nobody → alex.lakatos
Reproduced on Mac OS X 10.8.2 (x86_64) and Aurora branch:
http://mozmill-ci.blargon7.com/#/functional/report/352218d7e3125c857fc1d371671e455d
Can someone try if it is reproducible?
FWIW, reproduced with the 17.0.1 candidate builds:
http://mozmill-ci.blargon7.com/#/functional/report/352218d7e3125c857fc1d3716716fa4b
Has happened again on Aurora, with Windows Vista:

http://mozmill-ci.blargon7.com/#/functional/report/9e41582ed5e806fa373351d9d339c5d2

I'll try today to reproduce it.
Priority: P2 → P3
OS: Mac OS X → All
Assignee: alex.lakatos.dev → andrei.eftimie
Priority: P3 → P2
Have not been able to reproduce it locally yet.

Happened again (Vista, 21.0a1, fr):
http://mozmill-ci.blargon7.com/#/functional/report/72e15dc943833b8fcba70aeb51cbc4ea
Same issue as bug 780957
Unable to click on an element if outside the visible viewport.
Thanks Andrei. So lets get bug 780957 fixed and rechecked this one afterward.
Depends on: 780957
Depends on: 837008
No longer depends on: 780957
Depends on: 780957
Has not happened in almost 3 weeks (last recorded failure was in the 29th January)

We might mark it WFM, but:

since it is related to bug 780957 which has been fixed (fix only applied to 17, 18 and 19)
we should keep an eye out if it resurfaces (especially see if it is on a branch with the previously mentioned fix applied, or on a newer branch).

Depending on the case bug 780957 fix might also fix this (if it only shows up on unpatched branches) or it might require a slightly different approach.
Marking as fixed as the dependant bug was fixed. We can reopen if it turns out this did not in fact fix this issue.
Status: ASSIGNED → RESOLVED
Closed: 8 years ago8 years ago
Resolution: --- → FIXED
Happened again today on Mac 10.8.2 with FF 17.0.4:
http://mozmill-ci.blargon7.com/#/functional/report/172b2945b09ec259eb6e92f6441f9459
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Has happened again this weekend, with nightly fr locale, Windows Vista:
http://mozmill-ci.blargon7.com/#/functional/report/2a6536e9db9f5f44ed48c58510c222ff
Happened again on Nightly with Mac OS X 10.6.8 (x86_64): http://mozmill-ci.blargon7.com/#/functional/report/2a6536e9db9f5f44ed48c58510fd1f85
Also on Nightly, en-US, Win7:
http://mozmill-ci.blargon7.com/#/functional/report/db924715258381629c26ddbad698c9f7

Lots of failures in the last weeks, I will give this higher priority on my todo list to get it fixed.

*Still not sure if we fail to click on the restart link (simplest explanation, have not been able to make it fail this way manually). Or something else entirely which hangs the restart.
This is failing a lot these days. Andrei, please try to get it reproduced so we can fix it.
Status: REOPENED → ASSIGNED
I've managed to reproduce this locally.
Fails about 1 in 10 runs.

I dumped all properties of the restartLink, attached is a diff of those properties. Based on the dimensions and positions (0) I might assert that we try to click on the link before it is actually displayed.

If this is the case, waiting for the element before clicking it should solve the problem. Running tests now to verify (will probably leave them overnight running to make sure no failures).

But I am a bit concerned about the difference on the markup, mainly:

Pass
> command="cmd_restartApp"

Fail
> oncommand="document.getBindingParent(this).restart();"

This might not mean anything, and it might change as the element loads into view.
(In reply to Andrei Eftimie from comment #26)
> But I am a bit concerned about the difference on the markup, mainly:
> 
> Pass
> > command="cmd_restartApp"
> 
> Fail
> > oncommand="document.getBindingParent(this).restart();"

Blair, do you have an explanation for this behavior of the restart button in the extension pane?
Flags: needinfo?(bmcbride)
Attached patch Patch v1 (obsolete) — Splinter Review
Attachment #731081 - Flags: review?(andreea.matei)
Missing comment for the previously attached patch.

Ran tests overnight. More then 1000 runs without incident.
It should fix the problem.
Comment on attachment 731081 [details] [diff] [review]
Patch v1

Review of attachment 731081 [details] [diff] [review]:
-----------------------------------------------------------------

I see we are using this restartLink element in several tests, I wonder why only this one failed. 
Anyway, it would be good as a precaution to have the fix on all of them.

::: tests/functional/restartTests/testAddons_uninstallExtension/test4.js
@@ +35,5 @@
>  
>    // Restart the browser using restart prompt
>    var restartLink = addonsManager.getElement({type: "listView_restartLink",
>                                                parent: enabledExtension});
> +  controller.waitForElement(restartLink);

Won't waitThenClick() work, instead of this and click()?
Attachment #731081 - Flags: review?(andreea.matei) → review-
(In reply to Henrik Skupin (:whimboo) from comment #27)
> Blair, do you have an explanation for this behavior of the restart button in
> the extension pane?

No idea, sorry :\
Flags: needinfo?(bmcbride)
Attached patch Patch v2 (obsolete) — Splinter Review
We are now waiting for every instance of the restartLink.

> Won't waitThenClick() work, instead of this and click()?
Not really, since we want to wait for the element to be available, and only then do startUserShutdown()
Attachment #731081 - Attachment is obsolete: true
Attachment #731788 - Flags: review?(andreea.matei)
Comment on attachment 731788 [details] [diff] [review]
Patch v2

Review of attachment 731788 [details] [diff] [review]:
-----------------------------------------------------------------

::: tests/functional/restartTests/testAddons_changeTheme/test1.js
@@ +78,5 @@
>    // Restart the browser using restart prompt
>    var restartLink = addonsManager.getElement({type: "listView_restartLink",
>                                                parent: plainTheme});
> +  controller.waitForElement(restartLink);
> +  

Trailing whitespaces.

::: tests/functional/restartTests/testAddons_changeTheme/test2.js
@@ +45,5 @@
>    // Restart the browser using restart prompt
>    var restartLink = addonsManager.getElement({type: "listView_restartLink",
>                                                parent: defaultTheme});
> +  controller.waitForElement(restartLink);
> +  

Same here

::: tests/functional/restartTests/testAddons_enableDisableExtension/test2.js
@@ +41,5 @@
>    // Click on the list view restart link
>    var restartLink = addonsManager.getElement({type: "listView_restartLink",
>                                                parent: addon});
> +  controller.waitForElement(restartLink);
> +  

Whitespaces

::: tests/functional/restartTests/testAddons_uninstallExtension/test2.js
@@ +53,5 @@
>    // Restart the browser using restart prompt
>    var restartLink = addonsManager.getElement({type: "listView_restartLink",
>                                                parent: toDisableExtension});
> +  controller.waitForElement(restartLink);
> +  

Trailing whitespaces
Attachment #731788 - Flags: review?(andreea.matei) → review-
Attached patch Patch v2.1Splinter Review
Sorry about that.
Fix trailing whitespaces.
Attachment #731788 - Attachment is obsolete: true
Attachment #732224 - Flags: review?(andreea.matei)
Happened again today with Firefox 23.0a1 on Mac OS X 10.7.5 (x86_64):
http://mozmill-ci.blargon7.com/#/functional/report/25ad365ca7bcf4905e9b700b4fbb635b
Comment on attachment 732224 [details] [diff] [review]
Patch v2.1

Review of attachment 732224 [details] [diff] [review]:
-----------------------------------------------------------------

Landed as:
http://hg.mozilla.org/qa/mozmill-tests/rev/c866a17f277c (default)
Attachment #732224 - Flags: review?(andreea.matei) → review+
Unfortunately this has not fixed the issue:
http://mozmill-ci.blargon7.com/#/functional/report/740ab23e062758ae8a240b263d27da26

Henrik, should we leave this change as an improvement though?
I think we should backout the patch. Given that we don't get a failure that the restartLink element doesn't exist. So it exists even without the extra waitForElement() call. We simply don't click on it.

Most likely it has something to do with the visibility state, and we click on it before it has been made visible.
Can we please get this test disabled given that we can't find a solution that quickly? This failure is bouncing us for days now.
Attached patch skip patchSplinter Review
Patch to skip the test.

*also needed to skip test5 because it relies on test4
Attachment #734532 - Flags: review?(andreea.matei)
Comment on attachment 734532 [details] [diff] [review]
skip patch

Review of attachment 734532 [details] [diff] [review]:
-----------------------------------------------------------------

Landed skip:
http://hg.mozilla.org/qa/mozmill-tests/rev/89cdb2dc35db (default)
Attachment #734532 - Flags: review?(andreea.matei) → review+
I might have had the right idea to "wait" for the restartLink based on previous evidence that when it failed it wasn't properly initialised.

But the implementation was not good. I used controller.waitFor() which, as I've now looked more deeply into mozmill's code, only checks that the element is available in the DOM. And the link was there, it only seemed not to be initialised.

The correct approach might be to wait for the element not to be disabled.
But alas, I am unable to reproduce it anymore. (Not locally, and not on our CI VM's).

Will try further to reproduce it.
There is a CSS transformation going on for the restart button link and the width is changing from auto to a specific value. The link is not disabled in any way. That might help you but better would be to have a look at the source what's happening for real when clicking on enable or disable.
I'm not sure if this is related but testAddons_UninstallExtension test1 through test3 failed today for Firefox 23.0b1 pl on Windows 8 64-bit:
http://mozmill-ondemand.blargon7.com/#/functional/report/91511d5536e962e0de7f57ed1ed83fec
Attached patch skipTests.patchSplinter Review
Disabling tests, manifest file wasn't changed.
Attachment #806603 - Flags: review?(andrei.eftimie)
Comment on attachment 806603 [details] [diff] [review]
skipTests.patch

Review of attachment 806603 [details] [diff] [review]:
-----------------------------------------------------------------

Yes, these 2 tests shouldn't have been unskipped.
The manifest.ini skip is still there.

Disabled:
http://hg.mozilla.org/qa/mozmill-tests/rev/678d05fab339 (default)
http://hg.mozilla.org/qa/mozmill-tests/rev/d56aac198f68 (mozilla-aurora)
Attachment #806603 - Flags: review?(andrei.eftimie)
Attachment #806603 - Flags: review+
Attachment #806603 - Flags: checkin+
Attached patch unskip_1.patchSplinter Review
These have been skipped for a long time now.
We've had numerous changes, including the move to mozmill 2.0

I've ran this 50 times in a loop. No failures.
It _was_ intermittent before. So this is not failproof.

I suggest we reenable tests 4 & 5 and monitor the outcome.
Attached is an unskip patch for default
Attachment #8388514 - Flags: review?(andreea.matei)
Comment on attachment 8388514 [details] [diff] [review]
unskip_1.patch

Review of attachment 8388514 [details] [diff] [review]:
-----------------------------------------------------------------

Re-enabled:
http://hg.mozilla.org/qa/mozmill-tests/rev/8fa4cef860a5 (default)
Attachment #8388514 - Flags: review?(andreea.matei) → review+
Andrei, could you please check if everything is fine on the rest of the branches to unskip the test?
Unskip for release.
Attachment #8394651 - Flags: review?(andreea.matei)
Unskip for ESR24
Attachment #8394652 - Flags: review?(andreea.matei)
Comment on attachment 8394651 [details] [diff] [review]
unskip_release.patch

Review of attachment 8394651 [details] [diff] [review]:
-----------------------------------------------------------------

http://hg.mozilla.org/qa/mozmill-tests/rev/1a452afb4700 (release)
http://hg.mozilla.org/qa/mozmill-tests/rev/4bb20a4ae08f (esr24)

Thanks!
Attachment #8394651 - Flags: review?(andreea.matei) → review+
Attachment #8394652 - Flags: review?(andreea.matei) → review+
Status: ASSIGNED → RESOLVED
Closed: 8 years ago7 years ago
Resolution: --- → FIXED
Whiteboard: [mozmill-test-failure] s=121126 u=failure c=addons p=1 → [mozmill-test-failure]
Product: Mozilla QA → Mozilla QA Graveyard
You need to log in before you can comment on or make changes to this bug.