It's a new test failure as introduced with the implementation bug 1144873 of notifications to Puppeteer. I will work on a fix together with the improvements on bug 1270953. 07:23:23 INFO - TEST-UNEXPECTED-FAIL | test_notifications.py TestNotifications.test_add_on_failed_notification | AssertionError: <firefox_puppeteer.ui.browser.notifications.AddOnProgressNotification object at 0x02804310> is not an instance of <class 'firefox_puppeteer.ui.browser.notifications.AddOnInstallFailedNotification'> 07:23:23 INFO - Traceback (most recent call last): 07:23:23 INFO - File "c:\jenkins\workspace\mozilla-central_functional\build\venv\lib\site-packages\marionette\marionette_test.py", line 351, in run 07:23:23 INFO - testMethod() 07:23:23 INFO - File "c:\jenkins\workspace\mozilla-central_functional\build\tests\firefox-ui\tests\puppeteer\test_notifications.py", line 52, in test_add_on_failed_notification 07:23:23 INFO - AddOnInstallFailedNotification)

It looks like we briefly get an AddOnProgressNotification before the AddOnInstallFailedNotification.

That's not the problem here. With my fix on bug 1270953 I can see that it will be working for nightly builds on mozilla-central and mozilla-aurora with strict signing of extensions enabled. But it does not work for inbound builds!? There extensions are getting installed without the blocked notification. Dave, do you have an idea what's different for those builds?

Review commit: https://reviewboard.mozilla.org/r/53234/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/53234/

Review commit: https://reviewboard.mozilla.org/r/53236/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/53236/

(In reply to Henrik Skupin (:whimboo) from comment #3) > That's not the problem here. With my fix on bug 1270953 I can see that it > will be working for nightly builds on mozilla-central and mozilla-aurora > with strict signing of extensions enabled. But it does not work for inbound > builds!? There extensions are getting installed without the blocked > notification. > > Dave, do you have an idea what's different for those builds? Inbound should be building exactly the same as mozilla-central, there shouldn't be any difference at all.

Interesting. So far I tested with a local fragment build. Maybe there is a problem given that a downloaded build from inbound works. I will check what the problem could be.

There was clearly something wrong with the fragment build. I did a fresh pull from inbound and rebuilt, now it is working. So my patch on bug 1270953 will definitely fix the failure on mozilla-central now.

The refactoring didn't fix it completely. It's a Windows only failure now.

I would definitely need help with this. Dave, would be great if you can have a look.

Review commit: https://reviewboard.mozilla.org/r/56612/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/56612/

It looks like the test is catching the progress notification popup, which appears briefly when installing a signed add-on. I've created a patch that waits for the expected notification.

Dave, the try results show test failures for linux64 non-e10s. I retriggered once but it keeps showing up. Can you please rebase your patch against latest inbound and upload again? We should trigger a complete new try build. Thanks.

Chris, I have an e10s related question for you which is related to the test changes in that mozreview request. As it looks like Marionette (and in detail our firefox-ui-tests) seem to be the only testsuite which has to set the "browser.tabs.remote.force-enable" preference to True. As what we have seen it at least prevents an additional notification (partially disabled accessibility support) shown on OS X. On Linux I do not see it at all. Is that a good way to go?

Review commit: https://reviewboard.mozilla.org/r/57218/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/57218/

(In reply to Henrik Skupin (:whimboo) from comment #19) > Dave, the try results show test failures for linux64 non-e10s. I retriggered > once but it keeps showing up. Can you please rebase your patch against > latest inbound and upload again? We should trigger a complete new try build. > Thanks. This is likely because I was setting a preference that effectively forced e10s to be enabled. I've moved this in my latest commit to only be set when we're testing against e10s. I've triggered a new try run to see if this passes.

Comment on attachment 8758349 [details] Bug 1271911 - Wait for the expected notifications in test_notifications.py to avoid transient popups. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/56612/diff/1-2/

Comment on attachment 8759186 [details] Bug 1271911 - Enable the browser.tabs.remote.force-enable preference when using e10s. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57218/diff/1-2/

Comment on attachment 8759186 [details] Bug 1271911 - Enable the browser.tabs.remote.force-enable preference when using e10s. https://reviewboard.mozilla.org/r/57218/#review53994 ::: testing/marionette/harness/marionette/runner/base.py:440 (Diff revision 2) > args.prefs = self._get_preferences(args.prefs_files, args.prefs_args) > > if args.e10s: > args.prefs.update({ > 'browser.tabs.remote.autostart': True, > + 'browser.tabs.remote.force-enable': True, Please request review from Maja or David here. I'm not really a Marionette peer. ::: testing/puppeteer/firefox/firefox_puppeteer/testcases/base.py (Diff revision 2) > def setUp(self, *args, **kwargs): > super(BaseFirefoxTestCase, self).setUp(*args, **kwargs) > > - # setting this pref prevents a notification from being displayed > - # when e10s is enabled. see http://mzl.la/1TVNAXh > - self.prefs.set_pref('browser.tabs.remote.force-enable', True) Lets get those lines removed in your first commit, so that we have a clean change for Marionette here.

(In reply to Henrik Skupin (:whimboo) from comment #20) > Chris, I have an e10s related question for you which is related to the test > changes in that mozreview request. As it looks like Marionette (and in > detail our firefox-ui-tests) seem to be the only testsuite which has to set > the "browser.tabs.remote.force-enable" preference to True. As what we have > seen it at least prevents an additional notification (partially disabled > accessibility support) shown on OS X. On Linux I do not see it at all. Is > that a good way to go? Felipe, do you know the answer to Henrik's question about forcing the "browser.tabs.remote.force-enable" pref in the Marionette tests?

(In reply to Chris Peterson [:cpeterson] from comment #26) > Felipe, do you know the answer to Henrik's question about forcing the > "browser.tabs.remote.force-enable" pref in the Marionette tests? What I noticed is that Marionette has a11y enabled by default because it relies on some of its features for working with elements. IMO we should get this stopped so I will file a bug for it. But due to that reason and having e10s enabled, we get this extra notification on OS X. If there is another pref we could use to not actually force-enable e10s I would happily use that.

https://reviewboard.mozilla.org/r/56612/#review54282 The test changes look fine to me and try is finally happy! Sadly I won't have the time to get the patch tested on Windows myself. But when you do the commit fixes as I mentioned earlier you will get my r+.

Comment on attachment 8759186 [details] Bug 1271911 - Enable the browser.tabs.remote.force-enable preference when using e10s. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57218/diff/2-3/

Comment on attachment 8758349 [details] Bug 1271911 - Wait for the expected notifications in test_notifications.py to avoid transient popups. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/56612/diff/2-3/

Comment on attachment 8758349 [details] Bug 1271911 - Wait for the expected notifications in test_notifications.py to avoid transient popups. https://reviewboard.mozilla.org/r/56612/#review54304 As mentioned earlier the test changes look fine to me. Thanks for your help Dave!

Comment on attachment 8759186 [details] Bug 1271911 - Enable the browser.tabs.remote.force-enable preference when using e10s. https://reviewboard.mozilla.org/r/57218/#review54306 Having to duplicate all the code now makes me iiiik. It will add a lot of additional maintainance.

Comment on attachment 8759186 [details] Bug 1271911 - Enable the browser.tabs.remote.force-enable preference when using e10s. https://reviewboard.mozilla.org/r/57218/#review54388

Pushed by mjzffr@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/e845e5efd230 Enable the browser.tabs.remote.force-enable preference when using e10s. r=maja_zf https://hg.mozilla.org/integration/mozilla-inbound/rev/465fe1502bf4 Wait for the expected notifications in test_notifications.py to avoid transient popups. r=whimboo

https://hg.mozilla.org/mozilla-central/rev/e845e5efd230 https://hg.mozilla.org/mozilla-central/rev/465fe1502bf4

Maja, seems this is already on aurora. Like https://hg.mozilla.org/releases/mozilla-aurora/rev/e845e5efd230 so removing flag :)

This would need a checkin for beta and not aurora. I think Maja missed to account the last merge.

https://hg.mozilla.org/releases/mozilla-beta/rev/b8a9cd794b94 https://hg.mozilla.org/releases/mozilla-beta/rev/1dcf48bac874

Looks like the question asked here about e10s force prefs was understood. If there are any remaining issues about how Marionette and e10s/a11y interacts, then it's probably a good idea to file a new bug for it