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

Status: RESOLVED DUPLICATE of bug 932753

(Firefox OS Graveyard :: Gaia::UI Tests, defect)

Description

[Zac C (:zac)]

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

Comment 1

[Zac C (:zac)]

https://github.com/mozilla-b2g/gaia/blob/master/tests/python/gaia-ui-tests/gaiatest/tests/functional/settings/test_settings_do_not_track.py

Comment 2

[Bob Silverberg [:bsilverberg]]

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

Comment 3

[Jason Smith [:jsmith]]

Is this a fallout from bug 914407 by any chance?

Comment 4

[Zac C (:zac)]

(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.

Comment 5

[Dave Hunt [:davehunt] [he/him] ⌚BST]

(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.

Comment 6

[Dave Hunt [:davehunt] [he/him] ⌚BST]

(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.

Comment 7

[Bob Silverberg [:bsilverberg]]

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?

Comment 8

[Dave Hunt [:davehunt] [he/him] ⌚BST]

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.

Comment 9

[Zac C (:zac)]

Dave's suggestion sounds good to me.

Comment 10

[Bob Silverberg [:bsilverberg]]

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.

Comment 11

[Bob Silverberg [:bsilverberg]]

ohawk, do you think you'll be getting to this soon? If not we may ask someone else to work on it. Thanks!

Comment 12

[O Hawk]

(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

Comment 13

[Bob Silverberg [:bsilverberg]]

(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

Comment 14

[Bob Silverberg [:bsilverberg]]

Never mind me. Forgot to add 'return' at the beginning of the JS statement. :(  Working now!