Upgrade test_settings_do_not_track to check the Gecko pref rather than Gaia setting.

RESOLVED DUPLICATE of bug 932753

Status

RESOLVED DUPLICATE of bug 932753
5 years ago
5 years ago

People

(Reporter: zcampbell, Assigned: bsilverberg)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [gaia-ui-test][mentor=zac][mentor=timdream][lang=py])

(Reporter)

Description

5 years ago
timdream has asked that we upgrade the DNT tests to check gecko prefs rather than gaia prefs.

This task is to update all of the assertions to read the Gecko prefs.

The syntax for checking the Gecko pref is something like this:
self.marionette.execute_script("SpecialPowers.getBoolPref('privacy.donottrackheader.value');", special_powers=True)

The prefs to check are listed here:
http://dxr.mozilla.org/mozilla-central/source/b2g/chrome/content/settings.js#l434
(Assignee)

Comment 2

5 years ago
I chatted with ohawk on irc about this today. He is going to give it a try. At first I thought we should add methods to the data layer to retrieve gecko prefs, but that's going to be fairly complicated due to different calls needed for boolPref and intPref (for example). So we discussed just adding a specific method to the data layer for the two prefs we need, for now, which can be refactored later.

I suggested adding get_pref_privacy_donottrackheader_enabled and get_pref_privacy_donottrackheader_value to [1], which would be similar in code to [2], but using the code suggested above in the bug. I also suggested adding a unit test for those, which would be similar to [3].

Once that is complete and the tests are passing I suggest creating a pull request for review. If it looks ok, then I suggest updating the test in question to use those new data layer methods.


[1] https://github.com/mozilla-b2g/gaia/blob/master/tests/python/gaia-ui-tests/gaiatest/gaia_test.py#L133 
[2] https://github.com/mozilla-b2g/gaia/blob/master/tests/python/gaia-ui-tests/gaiatest/gaia_test.py#L232
[3] https://github.com/mozilla-b2g/gaia/blob/master/tests/python/gaia-ui-tests/gaiatest/tests/unit/test_bluetooth.py
Is this a fallout from bug 914407 by any chance?
(Reporter)

Comment 4

5 years ago
(In reply to Jason Smith [:jsmith] from comment #3)
> Is this a fallout from bug 914407 by any chance?

Not exactly, but the history is that Tim recommended we make this enhancement to the test while he was working on that bug.
(In reply to Bob Silverberg [:bsilverberg] from comment #2)
> I chatted with ohawk on irc about this today. He is going to give it a try.
> At first I thought we should add methods to the data layer to retrieve gecko
> prefs, but that's going to be fairly complicated due to different calls
> needed for boolPref and intPref (for example). So we discussed just adding a
> specific method to the data layer for the two prefs we need, for now, which
> can be refactored later.
> 
> I suggested adding get_pref_privacy_donottrackheader_enabled and
> get_pref_privacy_donottrackheader_value to [1], which would be similar in
> code to [2], but using the code suggested above in the bug. I also suggested
> adding a unit test for those, which would be similar to [3].

Are we going to be calling these from many places? I don't think we should be adding specific get/set pref methods into the test class as it's not scalable. I would suggest either simply duplicating the single line wherever we need to get the value of the pref, or maybe requesting a Marionette enhancement that wraps the call in a simpler form.
(In reply to Bob Silverberg [:bsilverberg] from comment #2)
> At first I thought we should add methods to the data layer to retrieve gecko
> prefs, but that's going to be fairly complicated due to different calls
> needed for boolPref and intPref (for example).

Sorry, I missed (or misunderstood) this suggestion. I think this is probably the approach we want to use. I've spoken briefly with jgriffin and AutomatedTester and this wouldn't be something added to Marionette, but it would make sense to have something like this in the consumer.
(Assignee)

Comment 7

5 years ago
Yeah, davehunt, I think that's the best way to go, but I thought that was a bit complicated for a new contributor. Although I know it will need to be rewritten/refactored, I thought my current suggestion was good to get him started. What do you think? Is that a reasonable approach?
It sounds like we should raise a bug for adding the convenience methods for getting/setting gecko preferences. I think this could still be a contributor task, but depending on their skill level may need additional mentoring.

That bug doesn't necessarily have to block this one, but for now I would just use execute_script wherever you need to set/get gecko preferences, and add a TODO to use the convenience methods once they're available.
(Reporter)

Comment 9

5 years ago
Dave's suggestion sounds good to me.
(Assignee)

Comment 10

5 years ago
I agree. @ohawk, please proceed by simply placing the marionette.execute_script calls directly in the test for now (don't worry about doing anything to the app objects or adding any unit tests). We will refactor later after helper methods are added for getting Gecko prefs.
(Assignee)

Updated

5 years ago
Blocks: 932753
(Assignee)

Comment 11

5 years ago
ohawk, do you think you'll be getting to this soon? If not we may ask someone else to work on it. Thanks!
Flags: needinfo?(hawk.ornate)

Comment 12

5 years ago
(In reply to Bob Silverberg [:bsilverberg] from comment #11)
> ohawk, do you think you'll be getting to this soon? If not we may ask
> someone else to work on it. Thanks!

I need more time, better if someone else can fix and continue with this bug.

thanks
Flags: needinfo?(hawk.ornate)
(Assignee)

Updated

5 years ago
Assignee: nobody → bob.silverberg
(Assignee)

Comment 13

5 years ago
(In reply to Zac C (:zac) from comment #0)
> timdream has asked that we upgrade the DNT tests to check gecko prefs rather
> than gaia prefs.
> 
> This task is to update all of the assertions to read the Gecko prefs.
> 
> The syntax for checking the Gecko pref is something like this:
> self.marionette.execute_script("SpecialPowers.getBoolPref('privacy.
> donottrackheader.value');", special_powers=True)
> 

Zac: where did you get that code? It isn;t working for me - it keeps returning None. I've looked at the docs and am a bit stuck. Was there someone who suggested that code?

> The prefs to check are listed here:
> http://dxr.mozilla.org/mozilla-central/source/b2g/chrome/content/settings.
> js#l434
Flags: needinfo?(zcampbell)
(Assignee)

Comment 14

5 years ago
Never mind me. Forgot to add 'return' at the beginning of the JS statement. :(  Working now!
Flags: needinfo?(zcampbell)
(Assignee)

Updated

5 years ago
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 932753
You need to log in before you can comment on or make changes to this bug.