942711 - Test failure 'Add-ons Manager has been closed.' across several tests

Status: RESOLVED FIXED

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

Description

[Andrei Eftimie]

Module:    teardownModule
Test:      /restartTests/testAddons_enableDisableExtension/test4.js    
Failure:   Add-ons Manager has been closed.
Branches:  default
Platforms: Windows XP

Sample report: http://mozmill-daily.blargon7.com/#/functional/report/456bebe92845279408c15c03e871330b

This has failed 4 times over the weekend, on Windows XP, with Nightly.

Comment 1

[Andrei Eftimie]

Interestingly we have the same failure in some endurance tests, again windows:

XP: http://mozmill-daily.blargon7.com/#/endurance/report/456bebe92845279408c15c03e88b1ad5
Vista: http://mozmill-daily.blargon7.com/#/endurance/report/456bebe92845279408c15c03e8746e9a
http://mozmill-daily.blargon7.com/#/endurance/report/456bebe92845279408c15c03e88b2442

Comment 2

[Andrei Eftimie]

I've failed to reproduce any of the failures above on a WinXP box

Comment 3

[mario garbi]

Happened again on Windows NT 6.0.6002 Service Pack 2 (x86) with Firefox 28.0a1 en-US:
http://mozmill-daily.blargon7.com/#/endurance/report/b99421c0f132c68dec1548288a019035

and
Windows NT 5.1.2600 Service Pack 3 (x86):
http://mozmill-daily.blargon7.com/#/endurance/report/b99421c0f132c68dec1548288a0191d5

Comment 4

[Cosmin Malutan, [:cosmin-malutan]]

Happened again on windows xp x86 with Nightly en-US
http://mozmill-daily.blargon7.com/#/endurance/report/b99421c0f132c68dec1548288a6cdc90

and on windows vista x86 with Nightly en-US
http://mozmill-daily.blargon7.com/#/endurance/report/b99421c0f132c68dec1548288a6ca034

Comment 5

[mario garbi]

I managed to reproduce it locally, will look into it.
http://mozmill-crowd.blargon7.com/#/endurance/report/8ff37f5518a39f6af2983cfe44028edb

Comment 6

[Andrei Eftimie]

Just to reiterate which tests are affected, we've seen this fail in:

functional/restartTests/testAddons_enableDisableExtension/test4.js
functional/testAddons/testManagerKeyboardShortcut.js
endurance/testAddons_OpenAndCloseExtensionList/test1.js	
endurance/testAddons_OpenAndCloseGetAddonsPane/test1.js	

I think I've only seen this on 28

Comment 7

[mario garbi]

Attachment: Screenshot from 2013-12-10 11:07:54.png

This happens due to us setting "about:home" as the value for "extensions.webservice.discoverURL" preference. I have tested this using a minimized test and it happens 2/3 time on winXP. 

As can be seen from the attached screenshot we open a third tab when we open the addonManager.

I will further look into why this is happening and how to fix the issue while keeping the preference that speeds up our addon tests considerably.

Comment 8

[mario garbi]

Attachment: addonManagerTabAnimation.patch

The issue here was that we were clicking the middle mouse button to close the addon manager before the tab was fully loaded and therefore hitting the tab bar, opening a new about:blank tab. 
The fix here was removes the open tab animation and therefore solves our problem as well as adding to the test speed.

http://mozmill-crowd.blargon7.com/#/endurance/report/ca1869364f98dd0de71202e01e1b3e10

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Comment on attachment 8346557 [details] [diff] [review]
addonManagerTabAnimation.patch

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

This is just a bad workaround. We generally have the tab animation turned on now, so tests which make use of the addon manager should properly handle the animation.

Comment 10

[Andreea Matei [:AndreeaMatei]]

Failed on endurance with Nightly, Windows XP
Tests:
testAddons_OpenAndCloseExtensionList/test1.js and 
testAddons_OpenAndCloseGetAddonsPane/test1.js:

http://mozmill-daily.blargon7.com/#/endurance/report/ca1869364f98dd0de71202e01e254b0f

Comment 11

[Andrei Eftimie]

This has failed several times during the weekend.
Tempted to raise the priority.

Mario any update?

Comment 12

[mario garbi]

Attachment: waitForTabAnimation.patch

Updated the patch to wait for the "transitionend" event, which also fixes the problem.
Linux:
http://mozmill-crowd.blargon7.com/#/endurance/report/ca1869364f98dd0de71202e01ea84a4a
Windows:
http://mozmill-crowd.blargon7.com/#/endurance/report/ca1869364f98dd0de71202e01ea850ab
Mac:
http://mozmill-crowd.blargon7.com/#/endurance/report/ca1869364f98dd0de71202e01ea84b1c

Comment 13

[Andreea Matei [:AndreeaMatei]]

Comment on attachment 8348101 [details] [diff] [review]
waitForTabAnimation.patch

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

Landed:
http://hg.mozilla.org/qa/mozmill-tests/rev/b63a467a043c (default)

Comment 14

[mario garbi]

Attachment: waitForTabAnimation_Beta.patch

Backport for Beta and Release. We can use the initial patch for Aurora.

Comment 15

[mario garbi]

Attachment: waitForTabAnimation_ESR24.patch

Backport patch for ESR24

Comment 16

[Andrei Eftimie]

Comment on attachment 8348626 [details] [diff] [review]
waitForTabAnimation_Beta.patch

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

::: firefox/lib/addons.js
@@ +173,5 @@
>        }
>      }
>      finally {
> +      this._tabBrowser._tabs.getNode().removeEventListener("transitionend",
> +                                                           checkTabTransitioned);

Please leave this in the same order as in the default & aurora patch.
We'll get merge conflicts later on if we have a different order.

Comment 17

[mario garbi]

Attachment: waitForTabAnimation_Beta.patch

Updated the patch with a nit.

Comment 18

[mario garbi]

Attachment: waitForTabAnimation_ESR24.patch

Updated the patch.

Comment 19

[Andrei Eftimie]

Comment on attachment 8348101 [details] [diff] [review]
waitForTabAnimation.patch

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

::: firefox/lib/addons.js
@@ +157,5 @@
>        }
>  
> +      assert.waitFor(function () {
> +        return self.transitioned;
> +      }, "Add-ons Manager has been opened", timeout, undefined, this);

`timeout` is only declared later in the code.

Please remove the last 3 parameters here. 
We don't need them.

We should clean this up.
Please make a followup patch to have this fixed.

We'll land backports with all final changes.

Comment 20

[mario garbi]

Attachment: waitForTabAnimation_FollowUp.patch

Followup patch for Default Branch, cleaning up parts of the code that shouldn't be there in the first place.

Comment 21

[Andreea Matei [:AndreeaMatei]]

Comment on attachment 8348702 [details] [diff] [review]
waitForTabAnimation_FollowUp.patch

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

http://hg.mozilla.org/qa/mozmill-tests/rev/39901d638d4a (default)

The other patches need an update.

Comment 22

[mario garbi]

Attachment: waitForTabAnimation_Aurora.patch

Aurora backport patch.

Comment 23

[mario garbi]

Attachment: waitForTabAnimation_Beta.patch

Fixed Beta backport patch.

Comment 24

[mario garbi]

Attachment: waitForTabAnimation_ESR24.patch

Fixed ESR24 backport patch.

Comment 25

[Andreea Matei [:AndreeaMatei]]

Comment on attachment 8349388 [details] [diff] [review]
waitForTabAnimation_Aurora.patch

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

http://hg.mozilla.org/qa/mozmill-tests/rev/465d259ac182 (aurora)

Comment 26

[Andreea Matei [:AndreeaMatei]]

Comment on attachment 8349393 [details] [diff] [review]
waitForTabAnimation_Beta.patch

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

http://hg.mozilla.org/qa/mozmill-tests/rev/fe6b65f6cf81 (beta)

Comment 27

[Andreea Matei [:AndreeaMatei]]

Comment on attachment 8349396 [details] [diff] [review]
waitForTabAnimation_ESR24.patch

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

http://hg.mozilla.org/qa/mozmill-tests/rev/13b716c6f589 (release)
http://hg.mozilla.org/qa/mozmill-tests/rev/6b5eee9025f6 (esr24)

We're done :) Thanks!