Closed
Bug 1000832
Opened 10 years ago
Closed 9 years ago
Update "notification_popup" case in LocationBar.getElements() methods to get the specific notifications
Categories
(Mozilla QA Graveyard :: Mozmill Tests, defect)
Mozilla QA Graveyard
Mozmill Tests
Tracking
(Not tracked)
RESOLVED
WONTFIX
People
(Reporter: AndreeaMatei, Unassigned)
References
()
Details
(Whiteboard: [enhancement][lib][lang=js][good first bug])
Attachments
(2 files)
2.28 KB,
patch
|
AndreeaMatei
:
review-
|
Details | Diff | Splinter Review |
7.71 KB,
patch
|
AndreeaMatei
:
review-
|
Details | Diff | Splinter Review |
One of this cases is "notification_popup" from toolbars.js in locationBar class. The popup is for notifications like after you install an addon, you get the small puzzle icon near the glob/lock icon and informs you about the action (the addon has been installed/needs restart). Same notification popup for login information, to save the password (you get a key icon). We might have other cases as well.
Reporter | ||
Comment 1•10 years ago
|
||
You can find in the URL the documentation to get started with Mozmill and mozmill tests. The case I was talking about: http://hg.mozilla.org/qa/mozmill-tests/file/default/firefox/lib/toolbars.js#l533 Please add other suggestions if you see some unclear cases.
Updated•10 years ago
|
Summary: Update cases in getElements() methods to be more suggestive → Update "notification_popup" case in LocationBar.getElements() methods to be more suggestive
Comment 2•10 years ago
|
||
Hi Andreea, I would like to work on this bug, am a newbie here can you please help me get started off?
Reporter | ||
Comment 3•10 years ago
|
||
Hi and welcome, I will assign you to this bug then. Please see the link in the URL field to start: https://developer.mozilla.org/en-US/docs/Mozilla/QA/Mozmill_tests You have there steps to get our environment with Mozmill installed, all you have to do then is to clone the repository: hg clone http://hg.mozilla.org/qa/mozmill-tests And start making changes for this bug as explained in the first comments. When you're done, you create a mercurial patch that you can attach here and ask review from myself. Thanks!
Assignee: nobody → adityamagadi
Status: NEW → ASSIGNED
Comment 4•10 years ago
|
||
Hi, I will cloning the repo today and start my work on it meanwhile i would like to if there anymore information on this bug that would come in handy?
Comment 5•10 years ago
|
||
Hi Andreea, i am kind of stuck here, When bug summary says "more suggestive" does it mean am suppose to alter the browser code for the notification popup? if it is the browser code that needs to be updated can you please help me locate the area where it needs to be fixed? or should the mozmill-tests javascript should be updated? Excuse me if am sounding lame.
Reporter | ||
Comment 6•10 years ago
|
||
It's just the case name from this link in comment 1: http://hg.mozilla.org/qa/mozmill-tests/file/default/firefox/lib/toolbars.js#l533 No Firefox code involved, just mozmill-tests repository.
Comment 7•10 years ago
|
||
Comment 8•10 years ago
|
||
Hi Andreea, Please review my patch.
Reporter | ||
Comment 9•10 years ago
|
||
Comment on attachment 8423257 [details] [diff] [review] BUG1000832.patch Review of attachment 8423257 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for your patch! When you attach the next one, please at the review field, put '?' and add my name (andreea.matei) so I'm notified of your requested review. Also, can you please update the commit message? hg qrefresh -m "Bug number - description of what your patch does. r=amatei" https://developer.mozilla.org/en-US/docs/Mozilla/QA/Mozmill_tests#Commit_message You will also need to modify all instances in the tests that use "notification_popup" with the changed case name. To be easier, you could search with grep through the whole repo where is used: grep -r "notification_popup" Then, your patch should be tested by running testruns of the specific test types you change. If you changed tests in functional and remote for example, you'll run those testruns: https://developer.mozilla.org/en-US/docs/Mozilla/QA/Mozmill_tests#Run_via_the_automation_scripts The mozmill-crowd reports can be added as a comment along with your patch here. ::: firefox/lib/toolbars.js @@ +29,5 @@ > /** > * AutoComplete Result class > */ > autoCompleteResults.prototype = { > + /**$ Please remove the $ symbol, must have been introduced by mistake. @@ +529,5 @@ > nodeCollector.queryNodes("#" + spec.subtype); > break; > case "notificationIcon": > return [new elementslib.ID(root, spec.subtype + "-notification-icon")]; > + case "notifyOnCompletion_popup": On a second thought, we'd like to divide this to multiple cases, as this one is general for addons or login forms etc., but they also have a "popupid" attribute which differentiate them. For login forms is "password-save", for addons is "addon-install-complete".
Attachment #8423257 -
Flags: review-
Reporter | ||
Updated•10 years ago
|
Summary: Update "notification_popup" case in LocationBar.getElements() methods to be more suggestive → Update "notification_popup" case in LocationBar.getElements() methods to get the specific notifications
Comment 10•10 years ago
|
||
Thank you for the review. I will work on the changes mentioned and get back to you.
Comment 11•10 years ago
|
||
Hi Andreea, i have done the changes as mentioned, however i could not run mozmill crowd tests as am running into this conflict "pkg_resources.VersionConflict: (mozinfo 0.6 (/usr/local/lib/python2.7/dist-packages), Requirement.parse('mozinfo>=0.7')) ", i was not able to update my mozinfo file to resolve this conflict.
Attachment #8426362 -
Flags: review?(andreea.matei)
Comment 12•10 years ago
|
||
Hi Andreea, any update on this?
Reporter | ||
Comment 13•10 years ago
|
||
Comment on attachment 8426362 [details] [diff] [review] 3882.patch Review of attachment 8426362 [details] [diff] [review]: ----------------------------------------------------------------- Hi, Unfortunately I don't see any change in toolbars.js, just one to a comment. You can check the patch before uploading it to be sure it included all the changes. Regarding the error you've got, have you used the latest env in http:// mozqa.com/mozmill-env? It should work fine to run a testrun.
Attachment #8426362 -
Flags: review?(andreea.matei) → review-
Comment 14•10 years ago
|
||
Hi, the first patch that i have submitted here contains the changes to the toolbars.js file.
Reporter | ||
Comment 15•10 years ago
|
||
Yes, but I have reviewed that in comment 9 and mentioned we need other cases for each notification type. Also we work with one patch only, so we mark as obsolete the previous ones and just qrefresh the patch in your repository to get all the changes.
Reporter | ||
Comment 16•10 years ago
|
||
Hi Adi, do you have trouble working on this, is something unclear? If so, please ask here or on the #automation channel on IRC. You could also join our training day there, tomorrow and next Wednesday, we learn stuff from scratch about mozmill and automation in general :)
Comment 17•10 years ago
|
||
Hi Andreea, Am not having any trouble working, i was tied up with busy schedule in the organisation that i work for. I will work this, this weekend and get back to you. And yes i will join the training session. Thank you.
Updated•10 years ago
|
Mentor: andreea.matei
Whiteboard: [enhancement][lib][mentor=andreea.matei][lang=js][good first bug] → [enhancement][lib][lang=js][good first bug]
Comment 18•10 years ago
|
||
Hi Andreea, I am sorry i will not be able to continue working on this bug, i have been travelling around a lot for my organisation product launch and not finding enough time to work on this bug. You can assign to someone else as i do not want to cause any further delay to your organisation.
Updated•10 years ago
|
Assignee: adityamagadi → nobody
Status: ASSIGNED → NEW
Reporter | ||
Comment 19•10 years ago
|
||
No problem Adityamagadi, feel free to join us whenever you have time in the future. Thanks for the work so far!
Comment 20•10 years ago
|
||
Along with this we should also update tests that opens a notification popup to check that the correct one has been opened (maybe create a new method for that).
Comment 21•10 years ago
|
||
Examples of this tests are in the geolocation tests (remote/testGeolocation & functional/testGeolocation) where we test that the propper notification is opened.
Comment 22•10 years ago
|
||
Hi Andreea, I'm out of time today, but I'll look over this tomorrow and probably have a few questions for you. Just wanted to let you know I'm on board.
Comment 23•10 years ago
|
||
Hi, sorry for disappearing, some pressing matters came up. I'll get back to you on this by Monday. Again, sorry for the delay.
Comment 24•10 years ago
|
||
I should probably have said this sooner, but I can't work on this bug right now. Sorry. If I have time later and nobody's taken this on, I'll come back to this. Again, sorry for any inconvenience.
Comment 25•9 years ago
|
||
We are in a transition to a new test framework. So this bug we don't want to fix anymore.
Mentor: matei.andreea89
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → WONTFIX
Updated•5 years ago
|
Product: Mozilla QA → Mozilla QA Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•