The default bug view has changed. See this FAQ.

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

RESOLVED WONTFIX

Status

Mozilla QA
Mozmill Tests
RESOLVED WONTFIX
3 years ago
2 years ago

People

(Reporter: AndreeaMatei, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

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

Attachments

(2 attachments)

(Reporter)

Description

3 years ago
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

3 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.
Summary: Update cases in getElements() methods to be more suggestive → Update "notification_popup" case in LocationBar.getElements() methods to be more suggestive

Comment 2

3 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

3 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

3 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

3 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

3 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

3 years ago
Created attachment 8423257 [details] [diff] [review]
BUG1000832.patch

Comment 8

3 years ago
Hi Andreea,
Please review my patch.
(Reporter)

Comment 9

3 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

3 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

3 years ago
Thank you for the review. I will work on the changes mentioned and get back to you.

Comment 11

3 years ago
Created attachment 8426362 [details] [diff] [review]
3882.patch

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

3 years ago
Hi Andreea,
any update on this?
(Reporter)

Comment 13

3 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

3 years ago
Hi,
the first patch that i have submitted here contains the changes to the toolbars.js file.
(Reporter)

Comment 15

3 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

3 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

3 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

3 years ago
Mentor: andreea.matei@softvision.ro
Whiteboard: [enhancement][lib][mentor=andreea.matei][lang=js][good first bug] → [enhancement][lib][lang=js][good first bug]

Comment 18

3 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

3 years ago
Assignee: adityamagadi → nobody
Status: ASSIGNED → NEW
(Reporter)

Comment 19

3 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

3 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

3 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

3 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

3 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

3 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.
We are in a transition to a new test framework. So this bug we don't want to fix anymore.
Mentor: matei.andreea89@gmail.com
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.