Bug 932750 - Add helper methods to GaiaData to get and set Gecko prefs #13644
Conversation
| @@ -183,6 +183,12 @@ def set_setting(self, name, value): | |||
| result = self.marionette.execute_async_script('return GaiaDataLayer.setSetting("%s", %s)' % (name, value), special_powers=True) | |||
| assert result, "Unable to change setting with name '%s' to '%s'" % (name, value) | |||
|
|
|||
| def get_pref(self, datatype, name): | |||
There was a problem hiding this comment.
Rather than passing the type, can we use type to infer it? http://docs.python.org/2/library/functions.html#type
There was a problem hiding this comment.
There are a couple of issues with this:
- We are passing the datatype as expected for the JS function, so boolean is actually
booland interger/numeric is actuallyint. I did this to avoid having to have the translation logic in theget_prefmethod. - We are passing exactly what we want passed to the JS method as the value, so if I want to pass true, I pass
'true'as opposed to the PythonTrue, so I'm not sure if type inference would even work in these situations.
I might be able to get it to work, but it would mean a lot more code in get_pref, so I prefer (pun inetnded) to keep things simple and just have the consumer pass the expected datatype.
What do you think?
There was a problem hiding this comment.
Sorry, my comment was on the wrong method anyway, it only makes sense to do this on set_pref... I see your points and in that case would prefer we map the JavaScript methods, and have get_int_pref, set_int_pref, etc. Passing the types as string arguments just doesn't seem safe to me.
There was a problem hiding this comment.
I was trying to avoid that at first, but now I see it makes perfect sense. If you're going to have to pass the datatype anyway, why not just call the correct method? I will fix it, and add char too.
|
@bobsilverberg can you add some dock about prefs and what's the diff between setting and prefs |
|
I updated the unit tests to use a unique pref which we can set and then get, and also added some docs to the new methods. |
|
I've updated the PR to include separate methods for |
|
tests pass both on device and desktop |
|
I have updated this yet again. Sorry folks. |
|
I feel like having separate methods for the data types might be misleading... How about simple get / set methods and the user should specify the data type? |
|
disregard my previous comment (Bad ideea) |
Bug 932750 - Add helper methods to GaiaData to get and set Gecko prefs
Also fixed a couple of PEP8 issues.