Closed
Bug 479571
Opened 15 years ago
Closed 15 years ago
[mozmill] Add module to have easier access to preferences
Categories
(Mozilla QA Graveyard :: Mozmill Tests, defect)
Mozilla QA Graveyard
Mozmill Tests
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: whimboo, Assigned: whimboo)
References
Details
Attachments
(1 file, 3 obsolete files)
3.87 KB,
patch
|
cmtalbert
:
review+
|
Details | Diff | Splinter Review |
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 | ||
Comment 1•15 years ago
|
||
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)
Assignee | ||
Updated•15 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•15 years ago
|
||
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•15 years ago
|
||
Now with a complete qrefresh...
Attachment #363827 -
Attachment is obsolete: true
Attachment #363829 -
Flags: review?(ctalbert)
Attachment #363827 -
Flags: review?(ctalbert)
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•15 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.
(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.
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
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•15 years ago
|
||
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•15 years ago
|
Attachment #363829 -
Flags: review+ → review-
Assignee | ||
Updated•15 years ago
|
Attachment #364489 -
Flags: review? → review?(ctalbert)
Comment 10•15 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•15 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
Closed: 15 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 12•14 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
Updated•5 years ago
|
Product: Mozilla QA → Mozilla QA Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•