[mozmill] Test Theme Change

RESOLVED FIXED

Status

Mozilla QA
Mozmill Tests
RESOLVED FIXED
8 years ago
7 years ago

People

(Reporter: aakashd, Assigned: aakashd)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [MozMill1.3Testday][mozmill-test-failure])

Attachments

(2 attachments, 2 obsolete attachments)

(Assignee)

Description

8 years ago
Created attachment 409215 [details] [diff] [review]
patch

The test case uses the Theme Installation folder in restartTests/ to change the theme from Walnut for Firefox back to the default theme via the Addons Manager. After switching back to default, it goes back to the Addons Manager --> Themes Pane, selects the default theme entry and verifies it's the current theme.


Note: I didn't change the name of the folder as I'm not sure if it should be changed or not. It's up to you, Henrik.

Litmus Testcases Represented:
 * Litmus test #8168: Change theme
 * Litmus test #6126: Change theme
Attachment #409215 - Flags: review?(hskupin)
(Assignee)

Updated

8 years ago
Whiteboard: [MozMill1.3Testday]
Comment on attachment 409215 [details] [diff] [review]
patch

Somehow the installation of the theme fails in Shiretoko on OS X 10.6. The countdown doesn't start in the modal installation dialog so we run into a timeout. Does that also happen on 10.5? 

>+++ b/firefox/restartTests/testThemeInstallUninstall/test3.js	Thu Oct 29 

Please put the test function from this module into test2.js. No need to have another restart.

>+  var window = mozmill.wm.getMostRecentWindow('Extension:Manager');
>+  var addonsController = new mozmill.controller.MozMillController(window);

Drop that like in the other tests.

>+  var useThemeButton = new elementslib.Lookup(addonsController.window.document, '/id("extensionsManager")/id("addonsMsg")/id("extensionsBox")/[1]/id("extensionsView")/id("urn:mozilla:item:'+ gThemeId +'")/anon({"flex":"1"})/{"class":"addonTextBox"}/anon({"anonid":"selectedButtons"})/{"class":"uninstallHide themeButton useThemeButton"}');

Can you please use the command property instead of class for the last element? 

>+  var themeDescription = new elementslib.Lookup(addonsController.window.document, '/id("extensionsManager")/id("addonsMsg")/id("extensionsBox")/[1]/id("extensionsView")/id("urn:mozilla:item:'+ gThemeId +'")/anon({"flex":"1"})/{"class":"addonTextBox"}/anon({"anonid":"addonDescriptionWrap"})');

Instead of the description we have to check that the url of the preview image changes when you select the other theme. You can use the persisted element too (like proposed in the other reviews) to carry values from one test module to the other.

>+  addonsController.waitForElement(restartButton, gTimeout);  

Please move this up to its declaration so its next to each other.

>+  UtilsAPI.assertElementVisible(addonsController, useThemeButton, false);

This test fails for me.

>+  var changedTheme = new elementslib.Lookup(addonsController.window.document, '/id("extensionsManager")/id("addonsMsg")/id("extensionsBox")/[1]/id("extensionsView")/id("urn:mozilla:item:'+ gThemeId +'")');
>+  addonsController.waitThenClick(changedTheme, gTimeout);
>+
>+  // Verify the gTheme is the current theme
>+  if (!changedTheme.getNode().getAttribute('current')) {
>+    throw gThemeName + " is not the currently enabled theme."
>+  }

Really? After the installation the new theme is automatically selected. That means in your test3.js you don't select the other theme.

I think that we should move this test to its own folder and just replicate some code. Before that I would really like to have a shared module which will help us a lot in extension and theme tests. Please file a bug on that.
Attachment #409215 - Flags: review?(hskupin) → review-
(Assignee)

Comment 2

8 years ago
Created attachment 413176 [details] [diff] [review]
patch with changes done like uninstall extension changes

Changes made to install theme and change theme (i.e. checking prefs thanks to mossop's help) as well as the comments made in the last review. The only thing that hasn't been fixed is to test the url of the preview image. I'm not sure how to do that after playing with DOM inspector. So, if you can offer some help there, then i'd like it.
Assignee: nobody → adesai
Attachment #409215 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #413176 - Flags: review?(hskupin)
Comment on attachment 413176 [details] [diff] [review]
patch with changes done like uninstall extension changes

While running this test on the different platforms I have seen that some tests are still produce failures. I have updated the tests.

>+  module.persisted.gThemeName = "Walnut for Firefox";

The 'g' prefix is only used for global variables. I have removed it.

>   // Make sure the Get Add-ons pane is visible
>   var getAddonsPane = new elementslib.ID(addonsController.window.document, "search-view");
>   addonsController.waitThenClick(getAddonsPane, gTimeout);
>@@ -83,7 +89,7 @@
>   addonsController.waitForEval("subject.selected == true", 10000, 100, installPane.getNode());

That only checks for the visibility of the installation pane but doesn't wait until the theme has been downloaded. That was causing the failure on Windows as what have been reported a couple of times. I have fixed that too now.

>   // Check if the correct theme name is shown
>-  if (itemElem.childNodes[0].name != gThemeName) {
>+  if (itemElem.childNodes[0].name != persisted.gThemeName) {
>     throw "Visible theme name doesn't match target theme";
>   }

I have moved those checks to assertJS.

>   // The installed theme should be the current theme in the list
>   // XXX: Use the add-on uuid to access the entry directly until we can pass the info
>   // between restart test files (bug 500987)

That 'XXX' is not valid anymore. I have removed that comment.

>   // Check if the Walnut Theme is the current theme
>   if (!item.getNode().getAttribute('current')) {
>-    throw gThemeName + " is not the currently enabled theme."
>+    throw persisted.gThemeName + " is not the currently enabled theme."
>   }

That should be an assertJS call.

controller.assertJS(persisted.gThemeName.toLowerCase().indexOf(currentTheme) != -1);  

Please always use the subject way as best as you can in the future.

>+  if (!changedTheme.getNode().getAttribute('selected')) 
>+  {
>+    throw "The default theme is not the currently highlighted theme."
>+  }

That should be an assertJS too.
Attachment #413176 - Flags: review?(hskupin) → review-
Created attachment 413323 [details] [diff] [review]
Patch (corrected)
Attachment #413323 - Flags: review?(adesai)
Blocks: 527983
(Assignee)

Comment 5

8 years ago
Created attachment 413751 [details] [diff] [review]
patch after your changes

With MozMill 1.3 the testscript fails on test3 when performing the waitForEval command. I've added a waitForElement for changedTheme on test3.js before that line which fixes it for me.
Attachment #413323 - Attachment is obsolete: true
Attachment #413751 - Flags: review?(hskupin)
Attachment #413323 - Flags: review?(adesai)
Comment on attachment 413751 [details] [diff] [review]
patch after your changes

Makes sense. Thanks.
Attachment #413751 - Flags: review?(hskupin) → review+
Landed as:
http://hg.mozilla.org/qa/mozmill-tests/rev/f44e7e265da6
http://hg.mozilla.org/qa/mozmill-tests/rev/f22c11cf0194

I had to push a follow-up to default which fixes the annoying failure when downloading the test. Thanks to Mavericks in telling me that and Mossop for the helpful information. It has been fixed with http://hg.mozilla.org/qa/mozmill-tests/rev/5291289926d7
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Whiteboard: [MozMill1.3Testday] → [MozMill1.3Testday][mozmill-test-failure]
Mass 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.
Component: Add-ons Manager → Mozmill Tests
Product: Toolkit → Mozilla QA
QA Contact: add-ons.manager → mozmill-tests
You need to log in before you can comment on or make changes to this bug.