Closed Bug 628030 Opened 13 years ago Closed 11 years ago

Keep track of Preferences dialog state in global variable

Categories

(Mozilla QA Graveyard :: Mozmill Tests, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: u279076, Assigned: balazs)

References

()

Details

(Whiteboard: [mentor=whimboo][lang=js][good first bug])

Attachments

(1 file, 4 obsolete files)

RE: https://developer.mozilla.org/en/XUL/prefwindow#p-lastSelected

When testPaneRetention.js was originally created, we did not have persisted object in Mozmill.  Given our recent problems with window handling (bug 626674) we should move to remembering state of the preferences dialog in the persisted object.

Currently, we check against a string-literal to verify pane retention:
var prefPaneCheckCallback = function(controller)
{
  let prefDialog = new prefs.preferencesDialog(controller);
  controller.assertJS("subject.paneId == 'panePrivacy'",
                      {paneId: prefDialog.paneId});
  prefDialog.close();
} 

We should move to using the properties available to us, but we should retain these values in persisted storage:
prefDialog.currentPane.id === prefDialog.lastSelected
Once we have these in persisted storage we can:

1. Open dialog
2. Switch pane
3. Save to persisted
4. Close dialog
5. Open dialog
6. Verify current values against persisted storage values
This should be moved to Shared Modules or some other component -- I don't think Mozmill Tests is appropriate.
No, this is special to some tests but does not influence anything we want to do generally. Storing data in the persisted object should only be done if necessary.
Given that, I don't think we need this in a separate bug. Each test which needs it should deal with it on their development bug.

On the other hand, if we have existing tests we want refactored, this should be moved into the refactor project.

Do you agree, Henrik?
Do whatever works best for you. But yeah, sounds good.
Vlad, can you check if any Prefs tests will need to be refactored for this?
Summary: Keep track of Prefs dialog state in persisted object → Keep track of Preferences dialog state in persisted object
Whiteboard: [mentor=whimboo][lang=js][good first bug]
Hi, I would be interested in working on that bug. Could it be assigned to me?
Sure. You are welcome Theod! Doing it now.
Assignee: nobody → theod.moz
Status: NEW → ASSIGNED
Theod, are you still working on this bug?
Flags: needinfo?(theod.moz)
Given that we have not getting feedback from theod since 2 months on this bug, I'm going to put this bug back into the general bucket.
Assignee: theod.moz → nobody
Status: ASSIGNED → NEW
Flags: needinfo?(theod.moz)
Comment on attachment 808152 [details] [diff] [review]
store last selected paneId in persisted v1.0

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

Given that the test itself is contained in a single file, there is no need anymore to necessarily use the persisted object. Just declare a new variable in setupModule, which will hold the current pane id. It will be accessible also from callback methods. Otherwise it looks good.
Attachment #808152 - Flags: review?(hskupin) → review-
Moved lastSelectedPaneId variable from persisted storage to setupModule.

Functional testrun:
http://mozmill-crowd.blargon7.com/#/functional/report/837c1e0f361ac93453ee697219863b99
Attachment #808152 - Attachment is obsolete: true
Attachment #810186 - Flags: review?(hskupin)
Comment on attachment 810186 [details] [diff] [review]
Store last selected paneId in setupModule variable

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

Just a minor thing to update in this patch. Then we can get it landed. With the fix you get my r+. Thanks!

::: tests/functional/testPreferences/testPaneRetention.js
@@ +40,5 @@
>  
>    prefDialog.paneId = 'paneAdvanced';
>    prefDialog.paneId = 'panePrivacy';
> +
> +  // Store actual paneId in the persisted object

nit: We don't use the persisted object here anymore. So please update the comment to reflect that.
Attachment #810186 - Flags: review?(hskupin) → review+
Corrected the comment
Attachment #810186 - Attachment is obsolete: true
Attachment #815531 - Flags: review?(hskupin)
The previous patch clearly applies on:
  default
  aurora
  mozilla-beta
  mozilla-release
  mozilla-esr24

I will create a backported patch for mozilla-esr17.
Backported patch for (mozilla-esr17)
Attachment #815553 - Flags: review?(hskupin)
Comment on attachment 815531 [details] [diff] [review]
Store last selected paneId in setupModule variable v2

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

::: tests/functional/testPreferences/testPaneRetention.js
@@ +10,5 @@
>  var utils = require("../../../lib/utils");
>  
>  function setupModule(aModule) {
>    aModule.controller = mozmill.getBrowserController();
> +  aModule.lastSelectedPaneId = 'paneMain';

Sorry, that I missed this the last time. But we cannot initialize the variable with 'paneMain'. Reason is that this test is run whenever already other tests have been performed before, so it is not clear in which state the preferences pane currently is. Lets initialize is with undefined.
Attachment #815531 - Flags: review?(hskupin) → review-
Comment on attachment 815553 [details] [diff] [review]
Store last selected paneId in setupModule variable (mozilla-esr17) v1

This bug is not about a test failure, which would require us to backport a patch. It should only land on default.
Attachment #815553 - Attachment is obsolete: true
Attachment #815553 - Flags: review?(hskupin)
initialize lastSelectedPaneId as undefined
Attachment #815531 - Attachment is obsolete: true
Attachment #816270 - Flags: review?(hskupin)
Comment on attachment 816270 [details] [diff] [review]
Store last selected paneId in setupModule variable v3

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

That looks fine now. Thanks Balazs!
Attachment #816270 - Flags: review?(hskupin) → review+
Landed as: http://hg.mozilla.org/qa/mozmill-tests/rev/ccb616a6ef8c
Assignee: nobody → juhaszbal
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Summary: Keep track of Preferences dialog state in persisted object → Keep track of Preferences dialog state in global variable
Product: Mozilla QA → Mozilla QA Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: