Add test for the functionality of addon button in Australis

RESOLVED FIXED in Firefox 35

Status

()

RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: mihaelav, Assigned: mihaelav)

Tracking

(Blocks: 1 bug)

Trunk
Firefox 35
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [Australis:P-])

Attachments

(1 attachment, 3 obsolete attachments)

Create automated tests for the following:

Steps:
1. Start Firefox
2. Install an add-on which adds a button to the browser interface
3. Click on the button of the add-on added at step 2.

Expected result:
The appropriate action related to the add-on's button is performed.
Whiteboard: [Australis:P-]
(Assignee)

Updated

5 years ago
Depends on: 985332
No longer depends on: 985332
(Assignee)

Comment 1

5 years ago
Posted patch v1 (obsolete) — Splinter Review
Extended the initial steps of the test a little: 

1. Start Firefox
2. Install an add-on which adds a button to the browser interface
3. Click on the button of the add-on added at step 2.
>> The appropriate action related to the add-on's button is performed.
4. Move the add-on button to the panel menu
5. Click on the add-on button in the panel menu
>> The appropriate action related to the add-on's button is performed.
Attachment #8497462 - Flags: review?(gijskruitbosch+bugs)

Comment 2

5 years ago
Comment on attachment 8497462 [details] [diff] [review]
v1

Review of attachment 8497462 [details] [diff] [review]:
-----------------------------------------------------------------

Although I'm really impressed with the add-on install stuff... it also sounds like a recipe for intermittent failures. :-(

Can you extract the add-on code and add/remove the button from inside the test itself?
Attachment #8497462 - Flags: review?(gijskruitbosch+bugs)
(Assignee)

Comment 3

5 years ago
Posted patch v2 (obsolete) — Splinter Review
Attachment #8497462 - Attachment is obsolete: true
Attachment #8497513 - Flags: review?(gijskruitbosch+bugs)

Comment 4

5 years ago
Comment on attachment 8497513 [details] [diff] [review]
v2

Review of attachment 8497513 [details] [diff] [review]:
-----------------------------------------------------------------

Close, but this should also call destroyWidget (see some of the other tests in this dir). See also the other notes below. :-)

A try push is probably a good idea.

::: browser/components/customizableui/test/browser_973641_button_addon.js
@@ +22,5 @@
> +  CustomizableUI.addWidgetToArea(kButton, CustomizableUI.AREA_NAVBAR);
> +
> +  // check the button's functionality in navigation bar
> +  let addonButton = document.getElementById(kButton);
> +  ok(addonButton, "Addon button was added to the navigation bar");

The message here should be more like "exists", and you can add another test for:

ok(navBar.contains(addonButton), "Addon button is in the navbar");

@@ +31,5 @@
> +  //move the add-on button in the Panel Menu
> +  CustomizableUI.addWidgetToArea(kButton, CustomizableUI.AREA_PANEL);
> +  let navBar = document.getElementById("nav-bar");
> +  let addonButtonInNavbar = navBar.getElementsByAttribute("id", kButton);
> +  is(addonButtonInNavbar.length, 0, "Addon button was removed from the browser bar");

nit: ok(!navBar.contains(addonButton), ...);

@@ +37,5 @@
> +  // check the addon button's functionality in the Panel Menu
> +  yield PanelUI.show();
> +  var panelMenu = document.getElementById("PanelUI-mainView");
> +  let addonButtonInPanel = panelMenu.getElementsByAttribute("id", kButton);
> +  is(addonButtonInPanel.length, 1, "Addon button was added to the Panel Menu");

Nit: ok(panelMenu.contains(addonButton), ...);

@@ +45,5 @@
> +add_task(function asyncCleanup() {
> +  resetTabs();
> +
> +  // reset the UI to the default state
> +  yield resetCustomization();

You should also destroy the widget you've added.

@@ +63,5 @@
> +
> +function checkButtonFunctionality(aButton) {
> +  aButton.click();
> +  yield waitForCondition(function() gBrowser.currentURI &&
> +                                    gBrowser.currentURI.spec == "about:addons");

Nit: use a fat arrow function:

yield waitForCondition(() => ...)

@@ +66,5 @@
> +  yield waitForCondition(function() gBrowser.currentURI &&
> +                                    gBrowser.currentURI.spec == "about:addons");
> +
> +  let addonsPage = gBrowser.selectedBrowser.contentWindow.document.
> +                           getElementById("addons-page");

Nit: split this up, e.g.:

let contentDoc = gBrowser.selectedBrowser.contentWindow.document;
let addonsPage = contentDoc.getElementById("addons-page");

@@ +67,5 @@
> +                                    gBrowser.currentURI.spec == "about:addons");
> +
> +  let addonsPage = gBrowser.selectedBrowser.contentWindow.document.
> +                           getElementById("addons-page");
> +  ok(addonsPage, "The button performs the expected action");

I'm a little scared about this relying on the internals of about:addons. We could just leave it at yielding the condition for the URL?
Attachment #8497513 - Flags: review?(gijskruitbosch+bugs) → review-
(Assignee)

Comment 5

5 years ago
Posted patch v2.1 (obsolete) — Splinter Review
Try runs: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=97ea4d0fd6cf
Attachment #8497513 - Attachment is obsolete: true
Attachment #8497983 - Flags: review?(gijskruitbosch+bugs)

Comment 6

5 years ago
Comment on attachment 8497983 [details] [diff] [review]
v2.1

Review of attachment 8497983 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/components/customizableui/test/browser_973641_button_addon.js
@@ +67,5 @@
> +
> +function checkButtonFunctionality(aButton) {
> +  aButton.click();
> +  yield waitForCondition(() => gBrowser.currentURI &&
> +                                    gBrowser.currentURI.spec == "about:addons");

Nit: wonky indenting :-)
Attachment #8497983 - Flags: review?(gijskruitbosch+bugs) → review+
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
https://hg.mozilla.org/integration/fx-team/rev/cd771a5cdfde
Flags: in-testsuite+
Keywords: checkin-needed
Whiteboard: [Australis:P-] → [Australis:P-][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/cd771a5cdfde
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Whiteboard: [Australis:P-][fixed-in-fx-team] → [Australis:P-]
Target Milestone: --- → Firefox 35
You need to log in before you can comment on or make changes to this bug.