604327 - Update Mozmill tests to make use of the new Menu API

Status: RESOLVED FIXED

(Mozilla QA Graveyard :: Mozmill Tests, defect)

Description

[Henrik Skupin [:whimboo][⌚️UTC+1]]

With the new feature from bug 597330 in place we will have to update our tests to make use of the new API. This will cover:

* Replace all controller.menus.* calls
* Identify all tests which make use of context menus and replace that code with the new API

All those changes will make our tests more robust, especially in situations when the context menu hasn't been closed.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Move of Mozmill Test related project bugs to newly created components. You can
filter out those emails by using "Mozmill-Tests-to-MozillaQA" as criteria.

Comment 2

[u279076]

What's the status of this bug?

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+1]]

If we can find an assignee it's kinda easy to fix.

Comment 4

[Remus Pop (:RemusPop)]

I don't see it failing any more.
I think we can call this a WONTFIX.

Comment 5

[Remus Pop (:RemusPop)]

Sorry, wrong bug.

Comment 6

[Antonina]

I would like to work on this bug.

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Thanks Antonina. Lets talk more and about the first steps on IRC early next week. Otherwise don't hesitate to ask if you have other questions.

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Antonina, have you had time to look at this? Last time we talked on IRC you wanted to send an updated version of the documentation on MDN. Have you had time for it?

Comment 10

[Alex Lakatos[:AlexLakatos]]

Antonina, I have a patch for this issue, from the duplicated bug. Is it ok if I take over?

Comment 11

[Antonina]

Yes, thank you!

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Antonina, do you want to still update the documentation for this API? I would kinda appreciate it.

Comment 13

[Alex Lakatos[:AlexLakatos]]

Attachment: patch v1.0

Refactored all controller.menus

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 612192 [details] [diff] [review]
patch v1.0

>+++ b/lib/private-browsing.js
>@@ -29,17 +29,17 @@ const gTimeout = 5000;
[..]
>-  this._pbMenuItem = new elementslib.Elem(this._controller.menus['tools-menu'].privateBrowsingItem);
>+  this._pbMenuItem = "#privateBrowsingItem";
[..]
>-      this._controller.click(this._pbMenuItem);
>+      this._controller.mainMenu.click(this._pbMenuItem);

I would say we get rid of the _pbMenuItem now. It exists for caching purposes and doesn't make sense anymore.

Otherwise I really like that change. It makes the code way cleaner. Just do the above change and we can land it.

Comment 15

[Dave Hunt [:davehunt] [he/him] ⌚BST]

The patch is missing at least one instance where there is a line break between "controller." and "menus[]". See http://hg.mozilla.org/qa/mozmill-tests/file/3b8476081653/lib/addons.js#l142

Comment 16

[Alex Lakatos[:AlexLakatos]]

Attachment: patch v1.1 [checked-in]

(In reply to Dave Hunt (:davehunt) from comment #15)
> The patch is missing at least one instance where there is a line break
> between "controller." and "menus[]". See
> http://hg.mozilla.org/qa/mozmill-tests/file/3b8476081653/lib/addons.js#l142

That was the only one. grep -r "menus" ./ returns nothing.

Comment 17

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Alex, given that Antonina hasn't replied back yet, would you mind updating the documentation on MDN based on bug 597330? I don't want to wait longer and forget about it again.

Comment 18

[Alex Lakatos[:AlexLakatos]]

(In reply to Henrik Skupin (:whimboo) from comment #17)
> Alex, given that Antonina hasn't replied back yet, would you mind updating
> the documentation on MDN based on bug 597330? I don't want to wait longer
> and forget about it again.

I don't mind. I'll update the documentation as well.

Comment 19

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 612570 [details] [diff] [review]
patch v1.1 [checked-in]

Patch has been landed:
http://hg.mozilla.org/qa/mozmill-tests/rev/db45e531d2d8 (default)
http://hg.mozilla.org/qa/mozmill-tests/rev/b04aa73f3114 (aurora)
http://hg.mozilla.org/qa/mozmill-tests/rev/183fdb44ee73 (beta)
http://hg.mozilla.org/qa/mozmill-tests/rev/88635eb190a5 (release)

Alex, please come up with a patch for esr10 because it doesn't apply cleanly:

$ hg transplant db45e531d2d8
applying db45e531d2d8
patching file tests/functional/testBookmarks/testAddBookmarkToMenu.js
Hunk #1 FAILED at 28
1 out of 1 hunks FAILED -- saving rejects to file tests/functional/testBookmarks/testAddBookmarkToMenu.js.rej
patch failed to apply

Comment 20

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Patch v1.0 (esr10) [checked-in]

Quick fix for the esr10 branch so we can close this bug.

Comment 21

[u279076]

Comment on attachment 614211 [details] [diff] [review]
Patch v1.0 (esr10) [checked-in]

Looks good to me -- go ahead and land.

Comment 22

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 614211 [details] [diff] [review]
Patch v1.0 (esr10) [checked-in]

http://hg.mozilla.org/qa/mozmill-tests/rev/bfbb3b6691a8 (esr)