Closed Bug 1132867 Opened 9 years ago Closed 9 years ago

Allow using mozregression with a particular pref

Categories

(Testing :: mozregression, defect)

Product:
Testing
Component:
mozregression
Platform:
x86
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: jrmuizel, Assigned: imjalpreet, Mentored)

Details

(Whiteboard: [good next bug][lang=python])

Attachments

(1 file)

Bug-1132867-Proposed_Patch
49 bytes, text/x-github-pull-request
parkouss
: review+
Details | Review 
It would be very handy to have mozregression be able to set a pref everytime it starts firefox. This would help when a regression is hidden behind a preference change.
mozrunner supports that so it may only need a forwarding command line option.
So it is handled by mozprofile exactly (which is used by mozrunner). To add that behaviour, we have to add a keyword argument when we instanciate the Profile: preferences (http://mozbase.readthedocs.org/en/latest/mozprofile.html?highlight=mozprofile#mozprofile.profile.Profile)

In mozreg code, the call to mozprofile is made here: https://github.com/mozilla/mozregression/blob/46f9c305477c24fcc97b15339c805b7164100e59/mozregression/launchers.py#L113

I propose that we mimic the --pref command line option of mozrunner/mozprofile to pass the user defined prefs.

@Will
What do you think ? it could be a mentored bug.
Flags: needinfo?(wlachance)
(In reply to Julien Pagès from comment #2)
> ...
> 
> @Will
> What do you think ? it could be a mentored bug.

This all makes sense to me.
Flags: needinfo?(wlachance)
Assignee: nobody → jalpreetnanda
Mentor: j.parkouss
Whiteboard: [good next bug][lang=python]
Can you please tell what needs to be done in this bug?
(In reply to Jalpreet Singh Nanda [:imjalpreet] from comment #4)
> Can you please tell what needs to be done in this bug?

We need to be able to change the preferences for the tested firefox from the command line. Preferences are what you can see in the about:config page of the browser. As an example, we should be able to run:

mozregression -g 2014-11-01 -b 2014-11-02 --pref "browser.warnOnQuit:true"

And that must define the browser.warnOnQuit preference to True when firefox is started.

To do that, we have to:

1. add the new command line parameter (look in the main.py file)
2. pass a preferences argument to the Profile constructor (see comment 3) and particularly https://github.com/mozilla/mozregression/blob/46f9c305477c24fcc97b15339c805b7164100e59/mozregression/launchers.py#L113

Once this is done (or in the same time), I suppose that we can add a --preferences that will do the same thing, but read a list of preferences from a file:

--preferences myprefs.json
--preferences myprefs.ini

I think that a good approach is to construct a mozprofile.prefs.Preferences object and pass it around until we construct the Profile.

Have a look at the Profile api for preferences: http://mozbase.readthedocs.org/en/latest/mozprofile.html#module-mozprofile.prefs
Talking about the main.py file, I have to add a new argument, so what all fields I have to write in parser.add_argument for adding a new commmandline parameter. Where can I get to know about the parser.add_argument?
Can you give a highlight on how can I add --pref argument? I mean to say that I understood how to add but I didn't get what all I have write in parser.add_argument(...). Can you explain it?
well, I suppose that something like:

parser.add_argument('--pref', nargs='*',
                    help="...")

should be sufficient. It will define an option that can be specifed from 0 to n times with the associated help.

try in a script the following code (say argtest.py):

import argparse

parser = argparse.ArgumentParser()
parser.add_argument('--pref', nargs='*', dest='prefs',
                    help="...")

options = parser.parse_args()
print options.prefs


then execute this script to see what it does:

python argtest.py -h
python argtest.py --pref 'something:1'
python argtest.py --pref 'something:1' 'somethingelse:2'
python argtest.py --pref 'something:1' --pref 'somethingelse:2'

As I said, look at the official doc (https://docs.python.org/2/library/argparse.html) - there are good examples in it.
Yeah, Thanks it's working!
Sorry, for the delay.

(In reply to Julien Pagès from comment #5)
> (In reply to Jalpreet Singh Nanda [:imjalpreet] from comment #4)
> > Can you please tell what needs to be done in this bug?
> 
> We need to be able to change the preferences for the tested firefox from the
> command line. Preferences are what you can see in the about:config page of
> the browser. As an example, we should be able to run:
> 
> mozregression -g 2014-11-01 -b 2014-11-02 --pref "browser.warnOnQuit:true"
> 
> And that must define the browser.warnOnQuit preference to True when firefox
> is started.
> 
> To do that, we have to:
> 
> 1. add the new command line parameter (look in the main.py file)
> 2. pass a preferences argument to the Profile constructor (see comment 3)
> and particularly
> https://github.com/mozilla/mozregression/blob/
> 46f9c305477c24fcc97b15339c805b7164100e59/mozregression/launchers.py#L113

  What exactly to do after passing the preferences argument?

> Once this is done (or in the same time), I suppose that we can add a
> --preferences that will do the same thing, but read a list of preferences
> from a file:
> 
> --preferences myprefs.json
> --preferences myprefs.ini
> 
> I think that a good approach is to construct a mozprofile.prefs.Preferences
> object and pass it around until we construct the Profile.

 

> Have a look at the Profile api for preferences:
> http://mozbase.readthedocs.org/en/latest/mozprofile.html#module-mozprofile.
> prefs
Sorry, for the delay.

(In reply to Julien Pagès from comment #5)
> (In reply to Jalpreet Singh Nanda [:imjalpreet] from comment #4)
> > Can you please tell what needs to be done in this bug?
> 
> We need to be able to change the preferences for the tested firefox from the
> command line. Preferences are what you can see in the about:config page of
> the browser. As an example, we should be able to run:
> 
> mozregression -g 2014-11-01 -b 2014-11-02 --pref "browser.warnOnQuit:true"
> 
> And that must define the browser.warnOnQuit preference to True when firefox
> is started.
> 
> To do that, we have to:
> 
> 1. add the new command line parameter (look in the main.py file)
> 2. pass a preferences argument to the Profile constructor (see comment 3)
> and particularly
> https://github.com/mozilla/mozregression/blob/
> 46f9c305477c24fcc97b15339c805b7164100e59/mozregression/launchers.py#L113

  What exactly to do after passing the preferences argument?

> Once this is done (or in the same time), I suppose that we can add a
> --preferences that will do the same thing, but read a list of preferences
> from a file:
> 
> --preferences myprefs.json
> --preferences myprefs.ini
> 
> I think that a good approach is to construct a mozprofile.prefs.Preferences
> object and pass it around until we construct the Profile.
  
  Where do I construct this object?

> Have a look at the Profile api for preferences:
> http://mozbase.readthedocs.org/en/latest/mozprofile.html#module-mozprofile.
> prefs
(In reply to Jalpreet Singh Nanda [:imjalpreet] from comment #12)
> Sorry, for the delay.

No problem :)

>   What exactly to do after passing the preferences argument?

Well nothing, we just need to pass it along to the Profile constructor. Something like:

self.profile_class(preferences=preferences)

>   Where do I construct this object?

I suspect that the main.cli function is a good place for that. You can create a preferences object
then directly feed it with what we have in the command line. Something like that (this is an example for the --preferences argument):

prefs = Preferences()
if options.preferences:
    prefs.add_file(options.preferences)
So, in the launchers.py file do I need to check the same way as it has been done for the profile argument?
Well I suppose that if a preferences object is empty it won't change the behaviour. In other words, I think that calling:

self.profile_class(preferences=Preferences())

is equivalent to:

self.profile_class()

In both cases, we have a profile initialized with no specific prefs.

So I think it is safe and cleaner to always pass a preferences object.
I think after I add the preferences argument, we will have to make some changes in the test_launcher.py too, right?
Hmm probably. Ran the tests with: "python setup.py test" to know if something is failing, and adapt the tests accordingly.

Also if you could provide some tests for what you are adding, that would be wonderful! That can be done in a followup though.
Now, after I add the preferences argument 2 tests are failing, so should I do any change in the tests?
Yes, tests need always to be up to date in order to be useful. This is probably a minor needed change to allow the tests running with the new arguments.

I suggest that we speak on mozilla irc: this will be easier and we won't spam here too much.

The easiest way is to use mibbit (https://wiki.mozilla.org/IRC#Mibbit) then choose a pseudo, and ping me here.

Once you are connected, type "/msg parkouss hello" in the mozilla tab to start a private discussion with me, I' connected.
Hey, I have tested both of them(--pref and --preferences). Can you tell me what is to be done now?

        Attachment #8570240 -
        Flags: review?(j.parkouss)

    
    

    
  
Comment on attachment 8570240 [details] [review]
Bug-1132867-Proposed_Patch

Seems good to me ! I just noted a few things on the PR. Could you fix them please ? I can merge this in after that, it seems safe.

Next thing to do will be to add a unit test.
I do care about unit test - that helps ensure followup patches does not break things and give us confidence, if'you're interested I can help you to write one for the new function you added (in a new commit). :)

        Attachment #8570240 -
        Flags: review?(j.parkouss) → review+

    
    

    
  
Yeah, I would like to add a unit test. Can you guide me how to start with it?

    
    

    
  
Thanks Jalpreet!

I merged this manually. (I moved the new options to group them when doing mozregression --help, I forgot to ask about that :))

I closed the pull request also, you can create a new one for the test (or reopen it if you prefer).

So, for the unit test, the idea is to test the behaviour of the new "preference" function. Our unit tests are organized in the tests/unit folder, and for this case you will need to create a class in the test_main.py file.

the name of the class starts with "Test" by convention and must be explicit, maybe "TestPreference" here. The class must inherit from the unittest.TestCase class.

A good starting point is to look at https://docs.python.org/2/library/unittest.html

Then each method of the class starting with "test" will be called automatically when you will run "python setup.py test" (this is indicated in the doc above).

The idea is to write a function that will call the new "prefence" function and test its result.

"preference" function returns a list of tuple. For example:

preference(None, ["browser.formfill.debug:false"]) == [('browser.formfill.debug', False)]

You need to call your preferences with at least one file, and one pref arg, and test that the result is what you expected. Instead of using '==' like I did, you must use the appropriate method (it will print a good error message in case of error). So in your test method, it will looks like:

prefs = main.preference(None, ["browser.formfill.debug:false"])
self.assertEqual(prefs, [('browser.formfill.debug', False)])

You can look at the other tests as an example, and the python unitest documentation.

Don't hesitate to ask if needed!

    
    

    
  
Sorry, I didn't notice this. I will look at this and get back to you.

    
    

    
  
Tests have been merged, we can close this bug now.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: