Closed Bug 578580 Opened 12 years ago Closed 12 years ago

Add category utilities for browser tests

Categories

(Toolkit :: Add-ons Manager, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla2.0b2

People

(Reporter: bparr, Assigned: bparr)

References

Details

(Whiteboard: [AddonsRewrite])

Attachments

(1 file, 2 obsolete files)

Add category utilities to test/browser/head.js so that tests don't have to implement the same thing over and over again (e.g. opening a category).
Summary: Add category utilities tests for browser tests → Add category utilities for browser tests
Attached patch Patch (obsolete) — Splinter Review
Assignee: nobody → bparr
Status: NEW → ASSIGNED
Attachment #457254 - Flags: review?(dtownsend)
Blocks: 577990
Blocks: 558287
Comment on attachment 457254 [details] [diff] [review]
Patch

This looks good. Will take another spin after the proposed changes.

>diff --git a/toolkit/mozapps/extensions/test/browser/browser_bug572561.js b/toolkit/mozapps/extensions/test/browser/browser_bug572561.js

> // Test that restarting the add-on manager with a non-active install
> // does not cause the locale category to show
> add_test(function() {
>   restart_manager(gManagerWindow, null, function(aWindow) {
>     gManagerWindow = aWindow;
>+    gCategoryUtilities = new CategoryUtilities(gManagerWindow);

You don't seem to use this after this point so probably no point in creating it.

>     check_hidden(true);
>     run_next_test();
>   });
> });
> 
> // Test that installing the install shows the locale category
> add_test(function() {
>   gInstall.install();
> });
> 
> // Test that restarting the add-on manager does not cause the locale category
> // to become hidden
> add_test(function() {
>   restart_manager(gManagerWindow, null, function(aWindow) {
>     gManagerWindow = aWindow;
>+    gCategoryUtilities = new CategoryUtilities(gManagerWindow);

Ditto

>diff --git a/toolkit/mozapps/extensions/test/browser/head.js b/toolkit/mozapps/extensions/test/browser/head.js

>+CategoryUtilities.prototype = {
>+  window: null,
>+
>+  get currentCategory() {
>+    isnot(this.window, null, "Should not get current when manager window is not loaded");
>+    var currentViewId = this.window.gViewController.currentViewId;
>+    var view = this.window.gViewController.parseViewId(currentViewId);
>+    return (view.type == "list") ? view.param : view.type;
>+  },

I think I would prefer this to be called selectedCategory and to get it from the selected richlistitem. These are UI tests after all so it's more important that the UI state is correct than the internal state.

>+
>+  getViewId: function(aCategoryType) {
>+    var LIST_CATEGORIES = ["locale", "searchengine", "extension" , "theme", "plugin"];
>+    if (LIST_CATEGORIES.indexOf(aCategoryType) >= 0)
>+      return "addons://list/" + aCategoryType;

To avoid the hardcoded list here you could just search the document for elements with attribute value="addons://list/<aCategoryType>". That would save us altering this if we change anything.

>+
>+    return "addons://" + aCategoryType + "/";
>+  },
>+
>+  get: function(aCategoryType) {
>+    isnot(this.window, null, "Should not get category when manager window is not loaded");
>+    return this.window.gCategories.get(this.getViewId(aCategoryType));
>+  },
>+
>+  isShowing: function(aCategory) {

Name this isVisible.

>+    isnot(this.window, null, "Should not check showing state when manager window is not loaded");
>+    if (aCategory.hasAttribute("disabled") &&
>+        aCategory.getAttribute("disabled") == "true")
>+      return false;
>+
>+    var style = this.window.document.defaultView.getComputedStyle(aCategory, "");
>+    return style.display != "none" && style.visibility == "visible";
>+  },
>+
>+  isTypeShowing: function(aCategoryType) {
>+    return this.isShowing(this.get(aCategoryType));
>+  },

isTypeVisible.

>+
>+  open: function(aCategory, aCallback) {
>+    isnot(this.window, null, "Should not open category when manager window is not loaded");
>+    ok(this.isShowing(aCategory), "Category should be showing if attempting to open it");
>+
>+    EventUtils.synthesizeMouse(aCategory, 2, 2, { }, this.window);
>+
>+    if (aCallback)
>+      wait_for_view_load(this.window, aCallback);
>+  },
>+
>+  openType: function(aCategoryType, aCallback) {
>+    this.open(this.get(aCategoryType), aCallback);
>+  },
Attachment #457254 - Flags: review?(dtownsend) → review-
Attached patch Patch v2 (obsolete) — Splinter Review
Kept the "gCategoryUtilities = new CategoryUtilities(gManagerWindow);" lines in browser_bug572561.js because gCategoryUtilities is used by the check_hidden() function. Besides that, I made the rest of Dave's changes.
Attachment #457254 - Attachment is obsolete: true
Attachment #457448 - Flags: review?(dtownsend)
Comment on attachment 457448 [details] [diff] [review]
Patch v2

>+  get: function(aCategoryType) {
>+    isnot(this.window, null, "Should not get category when manager window is not loaded");
>+    var viewId = "addons://list/" + aCategoryType;
>+    var category = this.window.gCategories.get(viewId);
>+    if (category)
>+      return category;
>+
>+    viewId = "addons://" + aCategoryType + "/";
>+    var category = this.window.gCategories.get(viewId);
>+    if (category)
>+      return category;

I want to avoid using the add-ons manager objects where possible and focus on the raw DOM. You should be able to get the richlist for the categories and use list.getElementsByAttribute("value", viewId) to get the category here.
Attached patch Patch v3Splinter Review
Made Dave's changes.
Attachment #457448 - Attachment is obsolete: true
Attachment #457646 - Flags: review?(dtownsend)
Attachment #457448 - Flags: review?(dtownsend)
Comment on attachment 457646 [details] [diff] [review]
Patch v3

Cool, thanks
Attachment #457646 - Flags: review?(dtownsend) → review+
Landed: http://hg.mozilla.org/mozilla-central/rev/5fd0b1867f79
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Flags: in-testsuite+
Flags: in-litmus-
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b2
Marking as verified fixed based on its landing and green test results.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.