Closed Bug 1270953 Opened 9 years ago Closed 9 years ago

Enhancements for notification popups

Categories

(Testing :: Firefox UI Tests, defect)

Version 3
defect
Not set
normal

Tracking

(firefox48 fixed, firefox49 fixed)

RESOLVED FIXED
mozilla49
Tracking Status
firefox48 --- fixed
firefox49 --- fixed

People

(Reporter: whimboo, Assigned: whimboo)

References

Details

Attachments

(3 files)

Follow-up bug for my last comment on bug 1144873. For ease of discovery here again: https://reviewboard.mozilla.org/r/45279/#review47709 Hey Dave! Thanks a ton for working on the implementation for doorhanger notifications! It's great to see that those have been landed now. Even this bug is fixed I checked the code and made some comments. There is nothing we have to work on this bug, but given the comment Bugzilla will create, I will file new bug for enhancements to this patch. ::: testing/firefox-ui/tests/puppeteer/test_notifications.py:75 (Diff revision 2) > + with self.assertRaisesRegexp(TimeoutException, message): > + self.browser.wait_for_notification(None) > + > + def trigger_add_on_notification(self, add_on): > + with self.marionette.using_context('content'): > + self.marionette.navigate('file://{0}'.format(here)) For resources we have a separate folder under http://mxr.mozilla.org/mozilla-central/source/testing/firefox-ui/resources/. It should have been the place where the XPI is stored. We can fix that in a follow-up bug. Also we should not install via file:// but localhost:// ::: testing/puppeteer/firefox/firefox_puppeteer/ui/browser/notifications.py:5 (Diff revision 2) > +# This Source Code Form is subject to the terms of the Mozilla Public > +# License, v. 2.0. If a copy of the MPL was not distributed with this file, > +# You can obtain one at http://mozilla.org/MPL/2.0/. > + > +from abc import ABCMeta We haven't used the meta classes anywhere else yet. Introducing them somewhere in the middle of the class hierachy is a bit irritating. But lets get a new bug filed to get the basic classes updated. ::: testing/puppeteer/firefox/firefox_puppeteer/ui/browser/notifications.py:32 (Diff revision 2) > + > + :returns: The notification origin. > + """ > + return self.element.get_attribute('origin') > + > + def close(self): We need a `force` option here, so we can ensure at least in teardown that the notification gets definitely closed. ::: testing/puppeteer/firefox/firefox_puppeteer/ui/browser/notifications.py:35 (Diff revision 2) > + return self.element.get_attribute('origin') > + > + def close(self): > + """Close the notification.""" > + self.element.find_element( > + By.ANON_ATTRIBUTE, {'anonid': 'closebutton'}).click() Usually we have a separate property for elements, so we can act on them directly and not via helper methods. ::: testing/puppeteer/firefox/firefox_puppeteer/ui/browser/notifications.py:45 (Diff revision 2) > + > + def allow(self): > + """Allow the add-on to be installed.""" > + self.element.find_element( > + By.ANON_ATTRIBUTE, {'anonid': 'button'}).find_element( > + By.ANON_ATTRIBUTE, {'anonid': 'button'}).click() Same here for the element, including other instances below. ::: testing/puppeteer/firefox/firefox_puppeteer/ui/browser/window.py:97 (Diff revision 2) > self._navbar = NavBar(lambda: self.marionette, self, navbar) > > return self._navbar > > @property > + def notification(self): Also we might have to think about if the notification directly under the browser is correct here. UI wise it is connected to the toolbar, and even only available for the tab where it has been raised. But maybe we should keep the door hanger notifications as is and add the tab-specific notifications under the tab. ::: testing/puppeteer/firefox/firefox_puppeteer/ui/browser/window.py:115 (Diff revision 2) > + By.CSS_SELECTOR, '#notification-popup popupnotification') > + except NoSuchElementException: > + return None # no notification is displayed > + notification_id = notification.get_attribute('id') > + return notifications_map[notification_id]( > + lambda: self.marionette, self, notification) I would propose that we return a base notification in case the current type is not being supported yet. ::: testing/puppeteer/firefox/firefox_puppeteer/ui/browser/window.py:118 (Diff revision 2) > + notification_id = notification.get_attribute('id') > + return notifications_map[notification_id]( > + lambda: self.marionette, self, notification) > + > + def wait_for_notification(self, notification_class=BaseNotification): > + """Waits for the specified notification to be displayed.""" nit: we should always document the parameters for the right API documentation. ::: testing/puppeteer/firefox/firefox_puppeteer/ui/browser/window.py:126 (Diff revision 2) > + lambda _: self.notification is None, > + message='Unexpected notification shown.') > + else: > + message = '{0} was not shown.'.format(notification_class.__name__) > + if notification_class is BaseNotification: > + message = 'No notification was shown.' Not sure why we have to set the message again. The `__name__` property should also be existent for the BaseNotification class.
I have started to work on this bug but run into a problem with the add-on installation. Now that the test tries to install it via the local webserver from Marionette I do not get the notification that the install is blocked due to no verification. Instead the addon gets installed successfully. I tried the same add-on with mozqa.com and there I get the blocked notification. Is there something special about localhost? Dave, could you help us? Thanks.
Flags: needinfo?(dtownsend)
Assignee: nobody → hskupin
Status: NEW → ASSIGNED
Blocks: 1271911
I'm not sure how to fix the test failure on bug 1271911 which only happens with inbound builds but not nightlies. I will move the request from Dave forward to the other bug, and skip the test for now.
No longer blocks: 1271911
Flags: needinfo?(dtownsend)
Comment on attachment 8753373 [details] MozReview Request: Bug 1270953 - Enhance utils module for permissions. r?maja_zf Review request updated; see interdiff: https://reviewboard.mozilla.org/r/53240/diff/1-2/
Comment on attachment 8753374 [details] MozReview Request: Bug 1270953 - Enhance notification class for Firefox Puppeteer. r?maja_zf Review request updated; see interdiff: https://reviewboard.mozilla.org/r/53242/diff/1-2/
Blocks: 1271911
The before mentioned test failure as a bustage locally. A rebuilt of my fragment build fixed it. So we do not have to skip it!
Blocks: 1272710
Blocks: 1272709
Comment on attachment 8753373 [details] MozReview Request: Bug 1270953 - Enhance utils module for permissions. r?maja_zf https://reviewboard.mozilla.org/r/53240/#review50088
Attachment #8753373 - Flags: review?(mjzffr) → review+
Comment on attachment 8753374 [details] MozReview Request: Bug 1270953 - Enhance notification class for Firefox Puppeteer. r?maja_zf https://reviewboard.mozilla.org/r/53242/#review50094
Attachment #8753374 - Flags: review?(mjzffr) → review+
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
No longer blocks: 1271911
Since the patch landed on mozilla-central the number of different failures went down to only 2. So I would like to see a backport of those test changes to mozilla-aurora too.
Whiteboard: [checkin-needed-aurora]
Hi Henrik this fails to apply on aurora: Tomcats-MacBook-Pro-2:mozilla-central Tomcat$ hg graft --edit -r 90cad97e0b06 grafting 345230:90cad97e0b06 "Bug 1270953 - Enhance utils module for permissions. r=maja_zf" merging testing/puppeteer/firefox/firefox_puppeteer/api/utils.py warning: conflicts while merging testing/puppeteer/firefox/firefox_puppeteer/api/utils.py! (edit, then use 'hg resolve --mark') abort: unresolved conflicts, can't continue (use 'hg resolve' and 'hg graft --continue') Tomcats-MacBook-Pro-2:mozilla-central Tomcat$ vim testing/puppeteer/firefox/firefox_puppeteer/api/utils.py could you take a look ?
Flags: needinfo?(hskupin)
Whiteboard: [checkin-needed-aurora]
Backport patch for aurora without any code changes. Just an update of the lines because a method from mozilla-central is not available on mozilla-aurora.
Flags: needinfo?(hskupin)
Attachment #8755823 - Flags: review+
Carsten, the conflict has been fixed. What we would need to land are attachment 8755823 [details] [diff] [review] and changeset d1d706fd85f4. Thanks.
Whiteboard: [checkin-needed-aurora]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: