973641 - Add test for the functionality of addon button in Australis

Status: RESOLVED FIXED

(Firefox :: Toolbars and Customization, defect)

Description

[Mihaela Velimiroviciu (:mihaelav)]

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.

Comment 1

[Mihaela Velimiroviciu (:mihaelav)]

Attachment: v1

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.

Comment 2

[:Gijs (he/him)]

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?

Comment 3

[Mihaela Velimiroviciu (:mihaelav)]

Attachment: v2

Comment 4

[:Gijs (he/him)]

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?

Comment 5

[Mihaela Velimiroviciu (:mihaelav)]

Attachment: v2.1

Try runs: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=97ea4d0fd6cf

Comment 6

[:Gijs (he/him)]

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 :-)

Comment 7

[Mihaela Velimiroviciu (:mihaelav)]

Attachment: v2.2

Try run: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=734e85c0940d

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/cd771a5cdfde

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/cd771a5cdfde