[mozmill] Add module to have easier access to preferences

RESOLVED FIXED

Status

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

People

(Reporter: whimboo, Assigned: whimboo)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

9 years ago
We should have the availability to read and write preferences. This feature should be put into the utils module which can be included by all tests which need to check or modify preferences.

http://hg.mozilla.org/qa/mozmill-tests/file/2cde62365a0c/shared-modules/test_utilsAPI.js

I'll try to get something written over the weekend.
(Assignee)

Updated

9 years ago
Blocks: 479500
(Assignee)

Updated

9 years ago
Blocks: 479314
(Assignee)

Comment 1

9 years ago
Created attachment 363654 [details] [diff] [review]
Helper module for preferences

This module gives access to the preferences service and branch. Further two more helper methods were added which simplify the getter/setter access.
Attachment #363654 - Flags: review?(ctalbert)

Updated

9 years ago
No longer blocks: 479314
(Assignee)

Updated

9 years ago
Blocks: 479314
(Assignee)

Updated

9 years ago
Summary: Add functions to read/write preferences to the utils module → [mozmill] Add module to have easier access to preferences
(Assignee)

Comment 2

9 years ago
Created attachment 363827 [details] [diff] [review]
patch v1.1

I messed-up with the license in mostly all tests. Let's get this in correctly.
Attachment #363654 - Attachment is obsolete: true
Attachment #363827 - Flags: review?(ctalbert)
Attachment #363654 - Flags: review?(ctalbert)
(Assignee)

Comment 3

9 years ago
Created attachment 363829 [details] [diff] [review]
patch v1.1.1

Now with a complete qrefresh...
Attachment #363827 - Attachment is obsolete: true
Attachment #363829 - Flags: review?(ctalbert)
Attachment #363827 - Flags: review?(ctalbert)

Comment 4

9 years ago
Comment on attachment 363829 [details] [diff] [review]
patch v1.1.1

>diff --git a/shared-modules/test_prefsAPI.js b/shared-modules/test_prefsAPI.js

>+function Preferences() {
>+  this.branch  = Components.classes["@mozilla.org/preferences-service;1"].
>+                 getService(Components.interfaces.nsIPrefBranch);
>+  this.service = Components.classes["@mozilla.org/preferences-service;1"].
>+                 getService(Components.interfaces.nsIPrefService);

this.service is never used in this class.  Eliminate it.  r=ctalbert with that.  Otherwise, the patch looks great.  Thanks!
Attachment #363829 - Flags: review?(ctalbert) → review+
(Assignee)

Comment 5

9 years ago
(In reply to comment #4)
> >+  this.service = Components.classes["@mozilla.org/preferences-service;1"].
> >+                 getService(Components.interfaces.nsIPrefService);
> 
> this.service is never used in this class.  Eliminate it.  r=ctalbert with that.
>  Otherwise, the patch looks great.  Thanks!

Oh, the class itself doesn't use it. But any test can suffer from it. Let's take an example:

var teardownModule = function(module) {
  module.service.resetPrefs();
}

In that way you can completely reset all changed preferences. I really don't wanna kill it. Clint, please reconsider.

Comment 6

9 years ago
(In reply to comment #5)
> (In reply to comment #4)
> > >+  this.service = Components.classes["@mozilla.org/preferences-service;1"].
> > >+                 getService(Components.interfaces.nsIPrefService);
> > 
> > this.service is never used in this class.  Eliminate it.  r=ctalbert with that.
> >  Otherwise, the patch looks great.  Thanks!
> 
> Oh, the class itself doesn't use it. But any test can suffer from it. Let's
> take an example:
> 
> var teardownModule = function(module) {
>   module.service.resetPrefs();
> }
> 
> In that way you can completely reset all changed preferences. I really don't
> wanna kill it. Clint, please reconsider.
Ok, that's fine, but let's put this as a getter on the class then.  Something like getPrefService which calls into the service and gets the preference service and caches the object at that time as this.service.  Just having it hang out for the entire duration of the test seems wasteful if the test doesn't use it.

Comment 7

9 years ago
Whimboo, I also just noticed that the call to setComplexPref is wrong.  it takes one more value: http://mxr.mozilla.org/mozilla-central/source/modules/libpref/public/nsIPref.idl#160

Comment 8

9 years ago
Sorry to keep "one more thing"ing you.  But in the set pref code, you need break statements or else your switch statement will fall through to the next case.  You don't need these in the getPref function because you early return.
(Assignee)

Comment 9

9 years ago
Created attachment 364489 [details] [diff] [review]
patch v2

Ok, seems like the way I have used wasn't the best way. As what we have seen using any functions from nsIPrefService isn't really healthy. That's why I have removed the instantiation completely. Further it will be not possible to get/set a complex pref. If users wanna do that they have to use the methods of nsIPrefBranch directly. Even a prototype isn't really necessary. We only have one instance open all the time while a test is running. So no code has to be shared. All of this makes the class really simple to use.

Clint, I hope you will like the new patch. I personally do it and have learned a lot while working on it.
Attachment #363829 - Attachment is obsolete: true
Attachment #364489 - Flags: review?
(Assignee)

Updated

9 years ago
Attachment #363829 - Flags: review+ → review-
(Assignee)

Updated

9 years ago
Attachment #364489 - Flags: review? → review?(ctalbert)

Comment 10

9 years ago
Comment on attachment 364489 [details] [diff] [review]
patch v2

r=ctalbert.  I really like the new approach. Nice work.
Attachment #364489 - Flags: review?(ctalbert) → review+
(Assignee)

Comment 11

9 years ago
checked in: http://hg.mozilla.org/qa/mozmill-tests/rev/fd0cd7b00bdd

It can be used in the following way:


var setupModule = function(module) {
  module.controller = mozmill.getBrowserController();
  module.prefs = collector.getModule('prefsAPI').preferences;
}

var testPrefs = function() {
  // simplified access
  prefs.setPref('this.is.my.own.user.pref', true);

  // usage of a function of the nsIPrefBranch
  prefs.branch.clearUserPref('this.is.my.own.user.pref');
}
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
(Assignee)

Comment 12

8 years ago
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: Mozmill → Mozmill Tests
Product: Testing → Mozilla QA
QA Contact: mozmill → mozmill-tests
Version: Trunk → unspecified
You need to log in before you can comment on or make changes to this bug.