Module: testInstallMultipleExtensions Test: /restartTests/testAddons_installMultipleExtensions/test1.js Failure: Notification popup state has been opened Branches: default Platforms: OSX, Linux Sometimes this message is recorded first: "Panel is in the correct state - 'open' should equal 'closed'" http://mozmill-daily.blargon7.com/#/functional/failure?app=Firefox&branch=All&platform=All&from=2014-11-04&test=%2FrestartTests%2FtestAddons_installMultipleExtensions%2Ftest1.js&func=testInstallMultipleExtensions Looks like a regression. I can't make any pattern out of it. Of the 4th and on the 6th it failed on all OSX machines. And then on the 8th and on the 9th it failed on Ubuntu. (We switched on mozmill 2.0.9 on the 7th, maybe that has something to do with the failures switching from OSX to Linux?)

Reproduced this on Windows http://mozmill-crowd.blargon7.com/#/functional/report/43b0604030afed1afa485902878463ac

(In reply to Teodor Druta from comment #1) > Reproduced this on Windows > http://mozmill-crowd.blargon7.com/#/functional/report/ > 43b0604030afed1afa485902878463ac I also saw this on OSX. Once (out of 12+ runs). Are you able to reproduce it constantly? Try running the affected test in a loop. Maybe even reducing the executed code to get a testcase.

I replicated this issue on the staging XP machine today. It is reproducible on any Firefox version and with any mozmill branch, so I highly doubt that this is a Firefox/Mozmill issue, rather a system issue. The test failed in my investigation because the Firefox window didn't gain focus on start and opened in the foreground, the issue was to an opened system/installed application notification, unfortunately due to slow experience with the vnc connection I couldn't get to see what it was, and clicking it closed it. I will continue to look more into this bug.

(In reply to Teodor Druta from comment #3) > I replicated this issue on the staging XP machine today. > It is reproducible on any Firefox version and with any mozmill branch, so I > highly doubt that this is a Firefox/Mozmill issue, rather a system issue. > The test failed in my investigation because the Firefox window didn't gain > focus on start and opened in the foreground, the issue was to an opened > system/installed application notification, unfortunately due to slow > experience with the vnc connection I couldn't get to see what it was, and > clicking it closed it. > I will continue to look more into this bug. I investigated this thoroughly on some Ubuntu machines and I came to the conclusion that the failures for Ubuntu and Mac OS X is of different nature than those from the Windows XP. For Ubuntu it is indeed a regression range. https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=ffa0f66db411&tochange=b6cd2dd85b26

I found the pushlog on fx-team: https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=8047f8b92715&tochange=ea6f00d19dca The regressor is Bug 1084558. Bug 1082764, Bug 1094312 also seems to be related with this issue. The problem in our test is that when we click on the install button from the "Software Installation" dialog the "Restart firefox now" notification popup fails to open.

How can I reproduce this?

(In reply to Dave Townsend [:mossop] from comment #6) > How can I reproduce this? 1. Clone the test repository from http://hg.mozilla.org/qa/mozmill-tests 2. Dowload latest mozmill-env for your system from https://mozqa.com/mozmill-env/ 3. Activate the environment (./run or . bin/activate) 4. Run the test: > mozmill -m firefox/tests/functional/restartTests/testAddons_installMultipleExtensions/manifest.ini -b /Applications/FirefoxNightly.app/

I meant outside of the test. Is this just a test bug or a real bug that users can hit?

(In reply to Dave Townsend [:mossop] from comment #8) > I meant outside of the test. Is this just a test bug or a real bug that > users can hit? This currently fails in our test. We don't yet have manual steps, but from the looks of how this fails we should probably be able to reproduce manually (unless there's some kind of race condition). Teodor, please check if you are able to manually reproduce the issue, and include steps if so. If not please attach a minimized testcase for mozmill. Also from the available reports it looks like it's not failing intermittently: http://mozmill-daily.blargon7.com/#/functional/failure?app=Firefox&branch=All&platform=All&from=2014-11-04&test=%2FrestartTests%2FtestAddons_installMultipleExtensions%2Ftest1.js&func=testInstallMultipleExtensions We should probably also disable the affected test on Nightly.

Teodor, please add a skip patch, it's failing a lot. This will also go in for next week to check.

Added skip patch.

Comment on attachment 8524594 [details] [diff] [review] skipinstallmultipleext.patch Review of attachment 8524594 [details] [diff] [review]: ----------------------------------------------------------------- Disabled: http://hg.mozilla.org/qa/mozmill-tests/rev/1e04124c084e (default)

There's another test affected with the same error, we should check if it's related: testAddons_InstallAddonWithoutEULA/test1.js http://mozmill-daily.blargon7.com/#/remote/report/635fcffb06b246be47416301bd43d549

We have a race condition here, but it's a real bug. The latest notification that we miss is shown in background, because at the moment notification get's instantiated, the focused window is not the one in which notification must be displayed, I added some dumps and I've got to isActiveWindow. http://mxr.mozilla.org/mozilla-central/source/toolkit/modules/PopupNotifications.jsm?rev=297245ad4574#319 Now I've got this far, but I don't know what would be the best approach to fix this. Personally I think we should get the notification even if the is not on the active window, otherwise it should be shown when we focus the window, but it doesn't. Also if the notification failed to open, it prevents us from opening new tabs. Considering we show the notification on browser, and this is active, we shouldn't care so much about the window and show the notification anyway: 321 if (isActiveBrowser) { 322 // show panel now 323 this._update(notifications, notification.anchorElement, true); 324 325 if (!isActiveWindow) { 326 // indicate attention and update the icon if necessary 327 if (!notification.dismissed) { 328 this.window.getAttention(); 329 } 330 } 331 } Dave, what do you think? To reproduce this please follow the steps from comment 7.

So the pop-up should be updated right away, when the window gets the focus, but hit doesn't happen because it's in a setTimeout which get's called when the browser closes, even if it should execute right away. http://mxr.mozilla.org/mozilla-central/source/toolkit/modules/PopupNotifications.jsm?rev=297245ad4574#429

I'll resume again what happens: After we finish the installing of the addon, the notification which will require a restart should appear. Because the modal dialog is still focused(now it's closing), the notification is updated in background and it should appear right away when the window gets focused. This call happens in a setTimeout(... ,0) this should happen right away, but instead it get's called only when we close the browser. The solution here(which will not change the behavior) would be to use Services.tm.currentThread to dispatch the event. We saw this in other tests too, so I expect this to fix all the "Notification state open" failures in our tests. I saw Gavin implemented this code, so I'll ask him for review.

Thanks Cosmin for this proposed fix. Can you please create a specific bug under toolkit for this patch? This bug is for the failing Mozmill test. Thanks.

Comment on attachment 8528870 [details] [diff] [review] patch v1.0 I don't understand how this would fix anything - these should be functionally equivalent, as you say. Why do you want to make this change?

The code as it's right(inside of setTimeout call) executes only when the window closes, I don't know exactly why but that's what I see when I log dumps and visually, the notification appears only when the window closes. This will allow us to re-enable lots of skipped test, due to intermittent notifications failures. Gavin, do you agree with this change? Should I file a new bug or it will be pointless?

(In reply to Cosmin Malutan from comment #19) > The code as it's right(inside of setTimeout call) executes only when the > window closes When what window closes?

The browser-chrome window, it appears just before the browser shuts down, that's after our test failed. https://www.youtube.com/watch?v=v85M-fiDk4A&feature=youtu.be

It seeems that there is an issue skipping manifest.ini directly, updated the patch to skip directly the tests.

After removing the changeset, there are rejections, fixed the patch.

After landing the new patch from Bug 1088561, the patch is not applying anymore, fixed rejections.

Comment on attachment 8535643 [details] [diff] [review] skipinstallmulext.patch Review of attachment 8535643 [details] [diff] [review]: ----------------------------------------------------------------- http://hg.mozilla.org/qa/mozmill-tests/rev/cffd6c8a8fba (default) This will cause bug 1110837 to happen on aurora too if we move testChangeTheme from there and this test remains first.

I have created an unskip patch for these tests because the Bug 1108994 will fix this issue.

Comment on attachment 8536497 [details] [diff] [review] unskipInstallMultipleExtensions Review of attachment 8536497 [details] [diff] [review]: ----------------------------------------------------------------- This is blocked by the other bug, please update the Depends on here. Also the disabled will be at that point in the testAddons folder manifest, so you need to unblock from that bug first.

To resume what was discussed here, we fail in waiting for a notification after we install two extensions, please see the video from comment 21. The waitFor is inside of dialog handler, if I move it outside of the handler the test passes, as discussed on irc on 19/12/2014 we interfere with the browser in our tests and our waitFor function enforce the Firefox to run synchronously, that's why we see the notification only when the Firefox closes. As a solution I just had to move the waitForNotification call outside of modal handler. http://mozmill-crowd.blargon7.com/#/functional/report/c922e10f6ac9f80d9cffdca9cf057e72

Here is the fix patch, see the report above, also this was failing 100% on ubuntu, so this definitely fixes the test.

This is the correct patch

I fixed a nit, I call callback directly in waitForNotification

I re-arranged a bit the code. http://mozmill-crowd.blargon7.com/#/functional/report/c922e10f6ac9f80d9cffdca9cf31aeef

Comment on attachment 8558483 [details] [diff] [review] patch v2.1 Andreea this is the working patch, the latest one has intermittent failures because we register the listeners when the popoup is already opened

Comment on attachment 8558483 [details] [diff] [review] patch v2.1 Review of attachment 8558483 [details] [diff] [review]: ----------------------------------------------------------------- It no longer applies in the manifest, please update. Otherwise tests well.

Thanks

Comment on attachment 8559601 [details] [diff] [review] patch v2.2 Review of attachment 8559601 [details] [diff] [review]: ----------------------------------------------------------------- https://hg.mozilla.org/qa/mozmill-tests/rev/7d68f2abf7c3 (default)

Andreea, this ran perfectly on default for 4 days, let's backport it down to release.

The previous patch applies on aurora, this is the patch for beta.

https://hg.mozilla.org/qa/mozmill-tests/rev/c76cfa6a90d2 (aurora)

Comment on attachment 8561278 [details] [diff] [review] patch v2.2 beta Review of attachment 8561278 [details] [diff] [review]: ----------------------------------------------------------------- https://hg.mozilla.org/qa/mozmill-tests/rev/3ed735e6f581 (beta)

I see release is skipped too, the patch on beta doesn't apply to it.

This is patch for release.

Comment on attachment 8561902 [details] [diff] [review] patch v2.2 release Review of attachment 8561902 [details] [diff] [review]: ----------------------------------------------------------------- https://hg.mozilla.org/qa/mozmill-tests/rev/f49441edc75c (release)

ESR31 is also affected: http://mozmill-daily.blargon7.com/#/functional/failure?app=Firefox&branch=31&platform=All&from=2014-11-04&to=2015-02-10&test=%2FrestartTests%2FtestAddons_installMultipleExtensions%2Ftest1.js&func=testInstallMultipleExtensions

For esr we should reopen bug 1088551 and backport that fix too.

Lets do that please.

Andreea the esr is not affected this bug was 100% reproducible, but I can't reproduce it neither locally or on a production node. Also it failed only on Ubuntu 14.04, this bug affected all both Ubuntu versions and also OSX. I expect to be a display size issue. If it fails again I suggest we open a new bug for that, the issue for this specific bug was clearly fixed.