Closed Bug 905400 Opened 7 years ago Closed 5 years ago

Inherit from mozprofile's general Profile class and add all testing preferences

Categories

(Testing Graveyard :: Mozmill, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: whimboo, Unassigned, Mentored)

References

()

Details

Attachments

(1 file)

As of now we put more and more testing related preferences to the mozprofile FirefoxProfile class. For an example see bug 897891.

We should stop this and instead have our own profile class inherited from the base Profile class with all the necessary preferences directly in Mozmill. We should not base on the FirefoxProfile of mozbase.

This work should be part of the work for Mozmill 2.1.
Whiteboard: [mozmill-2.1?] → [mozmill-2.1?][mentor=whimboo][lang=py]
Blocks: 905404
I am willing to solve the bug.
That's great to hear shikher! I will assign this bug to you now. If you have further questions please don't hesitate to ask here or join us on IRC in #automation. We will give you all the necessary information to get started.
Assignee: nobody → shikher111
Status: NEW → ASSIGNED
Hi Henrik,

Can u tell me a step by step procedure on how can i use my python/java knowledge to solve this bug as this will be my first bug. I downloaded the repository on github, now i just need your guidance on which file i should understand and edit, etc.

Thanks,
Shikher
You have to clone two repositories:
https://github.com/mozilla/mozmill
https://github.com/mozilla/mozbase

The base classes for profiles you will find in mozbase under mozprofile. There is a profile module. Just get familiar with the profile and FirefoxProfile classes. For the changes in  Mozmill you will have to do edits in the __init__.py module of mozmill.

Surprisingly it seems easier as I thought given that we can already set preferences in Mozmill directly via the profile_args argument of mozrunner:

https://github.com/mozilla/mozmill/blob/master/mozmill/mozmill/__init__.py#L116

But before we start with that I would like to have feedback from Jeff if that is the correct way or if we should really start implementing a subclass of Profile instead.
Flags: needinfo?(jhammel)
IS it profile.py in mozprofile? What do you mean by add all testing preferences, just so i know what exactly i am doing? I will wait till you confirm from Jeff, In the meantime, i will start reading the class.
Re comment 4 , either way seems fine to me.
Flags: needinfo?(jhammel)
Since, Jeff has finally given his verdict. It would be great if i could have instructions on what to do now.

Thanks Jeff
Hi Shikher, it looks like no-one followed up here. Would you still be interested to work on this bug?
Whiteboard: [mozmill-2.1?][mentor=whimboo][lang=py] → [mozmill-2.2?][mentor=whimboo][lang=py]
When we continue on this bug we should take the following list of preferences into account:
http://mxr.mozilla.org/mozilla-central/source/testing/profiles/prefs_general.js#57
Looks like Shikher is not working on this bug.
Assignee: shikher111 → nobody
Status: ASSIGNED → NEW
We quickly found a new assignee! So Fredrik would like to work on it. Great!
Assignee: nobody → fredrik.broman
Status: NEW → ASSIGNED
How can I test this? I've tried mutt (maybe not the best way to test this?) both with and without the patch I'm working on but I can't see any difference in the output.
What I would do here is to have all the testing preferences globally and exported in the mozmill module. Then we can import them in a mutt test, and push the list via the persisted property to the extension side. There we can check each of the preferences if they have been set.

Please keep in mind that you will also have to remove all those testing preferences from mozprofile's FirefoxProfile. This would just be for testing Mozmill. Once this bug is fixed those settings will have to be removed via bug 905404.

I hope that helps you Fredrik. I'm looking forward to your patch!
Whiteboard: [mozmill-2.2?][mentor=whimboo][lang=py] → [mozmill-2.2?][mentor=whimboo][lang=py][good first bug]
Hi Fredrik, I would like to ask if you can give us some updates regarding your assignment on this bug. Do you need further help or was my last explanation clear enough. Please let us know. Thanks!
Flags: needinfo?(fredrik.broman)
Henrik,

I've been working on it and hopefully I'll have a first attempt for a patch ready soon. Maybe even tonight!
Flags: needinfo?(fredrik.broman)
I'm looking forward to comments on this one!
Attachment #8407057 - Flags: review?(hskupin)
Comment on attachment 8407057 [details] [diff] [review]
0001-Bug-905400-Inherit-from-mozprofile-s-general-Profile.patch

Review of attachment 8407057 [details] [diff] [review]:
-----------------------------------------------------------------

Wow, lots of preferences here! It should have been a massive work to get all this added. So I hope you are not sad about my proposal below. I digged through all of the preferences, and you can find my comments inline.

::: mozmill/mozmill/__init__.py
@@ +98,4 @@
>          results = m.finish()
>      """
>  
> +    class_preferences = {}

I think it would be better when we add the prefs_general.js file as resource into the Mozmill package, and read-in all the values. The mozprofile class has a read_prefs() method, which can directly load such a preferences file: http://mozbase.readthedocs.org/en/latest/mozprofile.html#mozprofile.prefs.Preferences.read_prefs

That way it would be much easier to maintain all those preferences, which are indeed a huge list.

Also you missed some of the prefs from the current list of default preferences as listed here:

http://mxr.mozilla.org/mozilla-central/source/testing/mozbase/mozprofile/mozprofile/profile.py#315

But maybe we should first figure out which of those will remain in mozprofile.

@@ +155,5 @@
> +    # Disable intalling any distribution add-ons
> +    class_preferences['extensions.installDistroAddons'] = False
> +    # XPI extensions are required for test harnesses to load
> +    class_preferences['extensions.defaultProviders.enabled'] = True
> +    class_preferences['geo.wifi.uri'] = 'http://%(server)s/tests/dom/tests/mochitest/geolocation/network_geolocation.sjs'

Those entries with '%(server)s' for local test pages we might want to strip off. That has to be defined by tests themselves, or by a framework around those tests.

@@ +159,5 @@
> +    class_preferences['geo.wifi.uri'] = 'http://%(server)s/tests/dom/tests/mochitest/geolocation/network_geolocation.sjs'
> +    class_preferences['geo.wifi.timeToWaitBeforeSending'] = 200
> +    class_preferences['geo.wifi.scan'] = False
> +    class_preferences['geo.wifi.logging.enabled'] = True
> +    class_preferences['camino.warn_when_closing'] = False # Camino-only: harmless to others

We do not test Camino. So kill it.

@@ +161,5 @@
> +    class_preferences['geo.wifi.scan'] = False
> +    class_preferences['geo.wifi.logging.enabled'] = True
> +    class_preferences['camino.warn_when_closing'] = False # Camino-only: harmless to others
> +    # Make url-classifier updates so rare that they won't affect tests
> +    class_preferences['urlclassifier.updateinterval'] = 172800

That's an interesting change which could indeed help our tests for Firefox.

@@ +199,5 @@
> +    # Don't allow the Data Reporting service to prompt for policy acceptance.
> +    class_preferences['datareporting.policy.dataSubmissionPolicyBypassAcceptance'] = True
> +    # Point Firefox Health Report at a local server. We don't care if it actually
> +    # works. It just can't hit the default production endpoint.
> +    class_preferences['datareporting.healthreport.documentServerURI'] = 'http://%(server)s/healthreport/'

We might want to leave this to prevent sending reports.

@@ +235,5 @@
> +    # prefs for firefox metro.
> +    # Disable first-tun tab
> +    class_preferences['browser.firstrun.count'] = 0
> +    # Tell the PBackground infrastructure to run a test at startup.
> +    class_preferences['pbackground.testing'] = True

We don't need this.

@@ +240,5 @@
> +    # Enable webapps testing mode: which bypasses native installation.
> +    class_preferences['browser.webapps.testing'] = True
> +    # Disable android snippets
> +    class_preferences['browser.snippets.enabled'] = False
> +    class_preferences['browser.snippets.syncPromo.enabled'] = False

Mozmill doesn't work on Android. So we can kill that.

@@ +242,5 @@
> +    # Disable android snippets
> +    class_preferences['browser.snippets.enabled'] = False
> +    class_preferences['browser.snippets.syncPromo.enabled'] = False
> +    # Do not turn HTTP cache v2 for our infra tests (some tests are failing)
> +    class_preferences['browser.cache.use_new_backend_temp'] = False

This can also be removed. We want to be as close as possible to the users situation.

@@ +269,3 @@
>          if isinstance(preferences, dict):
> +            if bool(profile_args['preferences']):
> +                class_prefs.update(profile_args['preferences'])

You don't need class_prefs here. Instead simply update the preferences dict.

@@ +271,5 @@
> +                class_prefs.update(profile_args['preferences'])
> +
> +            profile_args['preferences'] = class_prefs
> +            profile_args['preferences']['extensions.jsbridge.port'] = jsbridge_port
> +            profile_args['preferences']['focusmanager.testmode'] = True

This line should actually be part of the default profiles. Not sure why it has been added here.

@@ +277,5 @@
>          elif isinstance(preferences, list):
>              preferences.append(('extensions.jsbridge.port', jsbridge_port))
>              preferences.append(('focusmanager.testmode', True))
> +            class_prefs.update(dict(profile_args['preferences']))
> +            profile_args['preferences'] = class_prefs

See above. Please directly update preferences.

@@ +926,5 @@
>          profile_args.setdefault('addons', []).extend(ADDONS)
>  
> +        profile_args['preferences'] = copy.deepcopy(MozMill.class_preferences)
> +        profile_args['preferences']['extensions.jsbridge.port'] = self.jsbridge_port
> +        profile_args['preferences']['focusmanager.testmode'] = True

Same as above.
Attachment #8407057 - Flags: review?(hskupin) → review-
Jonathan, quick question here. It would be good to know for us which frameworks already make use of mozprofile, and require the default preferences as set there:

http://mxr.mozilla.org/mozilla-central/source/testing/mozbase/mozprofile/mozprofile/profile.py#315

How many of those could be removed? As far as I can remember Jeff wanted to have all of them removed over on bug 905404.
Flags: needinfo?(jgriffin)
Most of our in-tree automation uses mozprofile now, so any changes could be potentially disruptive.  That said, since they're in-tree, you could test out removals on try.

I don't know what out-of-tree automation uses mozprofile; some of our on-device automation for B2G, and maybe a few other things.  Adding Joel in case he knows.
Flags: needinfo?(jgriffin)
mozmill
maybe autophone
talos does not, but it uses mozprocess.
Some questions:

Reading the prefs_general.js was no problem but how do I know the path to it? Is it always in the same place on the testing system or should I pass it in as an argument or something?

Also, some of the things I should remove according to comment #17 (stuff for camino, android etc) is read from the prefs_general.js. Should I write som code to filter this stuff out?

Finally: if preferences are passed in to the class (via the create classmethod) should they be the only preferences used and the general_prefs.js stuff completely ignored?
Mentor: hskupin
Whiteboard: [mozmill-2.2?][mentor=whimboo][lang=py][good first bug] → [mozmill-2.2?][lang=py][good first bug]
I see thefredrik has some questions in Comment 21, could you help him get answers?
Flags: needinfo?(hskupin)
I can remember that I talked with Fredrik on IRC and I thought we sorted those questions out. Fredrik please let me know if I'm wrong, what specifically you need answered.
Flags: needinfo?(hskupin)
I still need answers for all of my questions in #21.
You can count on the relative (from the runtests.py files) path 'profile_data/prefs_general.py' existing. I think the mochitest harness should know to look there automatically and you only need to pass in the argument if you are using a different directory somewhere else.

If additional preferences are passed in, I think they should 'update' the prefs in prefs_general. I.e if there are two prefs the same, the one in prefs_general should be overwritten, but otherwise both should get used.
(In reply to Fredrik Broman [:bromaniac] from comment #21)
> Reading the prefs_general.js was no problem but how do I know the path to
> it? Is it always in the same place on the testing system or should I pass it
> in as an argument or something?

So I thought once more about this specific bug. Maybe we should not make it too complex, and simply get the Mozmill related preferences out of the profile.py module? And add those simply in mozmill's init.py, so without any file handling foo. That could be done later on as a follow-up bug.

http://mxr.mozilla.org/mozilla-central/source/testing/mozbase/mozprofile/mozprofile/profile.py#311

Best way here would be to decide if a given preference is necessary for a test framework only. If a user who runs mozrunner, does not need it, we should move it over to the test framework. So the list of preferences becomes small enough. One thing to test through would be how other test frameworks beside Mozmill would behave. Maybe compare entries in the prefs_general.py with those here. For those which are missing in that other file, we should add them there. 

> Also, some of the things I should remove according to comment #17 (stuff for
> camino, android etc) is read from the prefs_general.js. Should I write som
> code to filter this stuff out?

I think this question obsoletes itself with my above comment. We should do this as a second (later) step.

> Finally: if preferences are passed in to the class (via the create
> classmethod) should they be the only preferences used and the
> general_prefs.js stuff completely ignored?

Yeah, ignore general_prefs.js for now. What we would need are the base preferences as set by mozprofile, and the ones from Mozmill itself.

Fredric, does that solve your questions?
Henrik, I think so. I will give this bug another try. If I have further questions I'll get back to you.
Frederick, are you still interested in this problem?  If so, has the problem been well defined enough to act on?
Flags: needinfo?(fredrik.broman)
No I haven't had any time to look into this. It's better if someone else takes it.
Flags: needinfo?(fredrik.broman)
Ok, thanks for the update Fredrik.
Assignee: fredrik.broman → nobody
Status: ASSIGNED → NEW
Hi.I would like to take this as my first bug
Hi Tessy, welcome :) Please have a look at the documentation and the comments in the bug to understand what is requested from you. If you have further questions, we're around.
You can follow these instructions to get mozmill locally running and make the changes:
https://github.com/mozilla/mozmill
Assignee: nobody → tessyjoseph1992
Status: NEW → ASSIGNED
I have clone the repo.need some mentoring.shall be working on it after 28th.is that fine.
How to chexk it on my system.have gone through the documentation.
How to chexk it on my system.have gone through the documentation.
Hi Tessy. Are your last two comments questions or just status updates? Anything you need help with?
Hi  Henrik Skupin (:whimboo) :Is to possible to run the software locally on my system?I need help
Tessy, I have just seen that Andreea didn't give a URL to our documentation. So please check https://wiki.mozilla.org/Auto-tools/Projects/Mozmill/RepoSetup how to setup Mozmill locally.
Tessy, do you have an update for us? If you think that you will not be able to handle this bug, please let me know. Thinking more about it's really not a good first one.
Whiteboard: [mozmill-2.2?][lang=py][good first bug] → [mozmill-2.2?][lang=py]
Mozmill will reach its end of life soon. We are currently working on getting
all the tests for Firefox ported to Marionette. For status updates please see
bug 1080766.

There is now bug 1123683 for getting all the necessary testing preferences added to Marionette.
Assignee: tessyjoseph1992 → nobody
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → WONTFIX
Whiteboard: [mozmill-2.2?][lang=py]
Product: Testing → Testing Graveyard
No longer blocks: 905404
You need to log in before you can comment on or make changes to this bug.