Closed Bug 1000832 Opened 7 years ago Closed 6 years ago

Update "notification_popup" case in LocationBar.getElements() methods to get the specific notifications

Categories

(Mozilla QA Graveyard :: Mozmill Tests, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: AndreeaMatei, Unassigned)

References

()

Details

(Whiteboard: [enhancement][lib][lang=js][good first bug])

Attachments

(2 files)

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.
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.
Summary: Update cases in getElements() methods to be more suggestive → Update "notification_popup" case in LocationBar.getElements() methods to be more suggestive
Hi Andreea,
    I would like to work on this bug, am a newbie here can you please help me get started off?
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
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?
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.
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.
Attached patch BUG1000832.patchSplinter Review
Hi Andreea,
Please review my patch.
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-
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
Thank you for the review. I will work on the changes mentioned and get back to you.
Attached patch 3882.patchSplinter Review
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)
Hi Andreea,
any update on this?
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-
Hi,
the first patch that i have submitted here contains the changes to the toolbars.js file.
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.
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 :)
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.
Mentor: andreea.matei
Whiteboard: [enhancement][lib][mentor=andreea.matei][lang=js][good first bug] → [enhancement][lib][lang=js][good first bug]
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.
Assignee: adityamagadi → nobody
Status: ASSIGNED → NEW
No problem Adityamagadi, feel free to join us whenever you have time in the future. Thanks for the work so far!
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).
Examples of this tests are in the geolocation tests (remote/testGeolocation & functional/testGeolocation) where we test that the propper notification is opened.
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.
Hi, sorry for disappearing, some pressing matters came up. I'll get back to you on this by Monday.

Again, sorry for the delay.
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.
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: 6 years ago
Resolution: --- → WONTFIX
Product: Mozilla QA → Mozilla QA Graveyard
You need to log in before you can comment on or make changes to this bug.