Closed Bug 692091 Opened 13 years ago Closed 13 years ago

Set up mozharness code for peptest

Categories

(Release Engineering :: Applications: MozharnessCore, enhancement, P5)

x86_64
All
enhancement

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: catlee, Assigned: ahal)

References

Details

(Whiteboard: [tests][mozbase][mozharness])

Attachments

(1 file, 2 obsolete files)

The auto-tools team is coming up with new test suites designed to test user responsiveness. The work is being documented here: https://wiki.mozilla.org/Auto-tools/Projects/peptest

They would like to have these tests run under the regular test automation.

To run the harness, we need to run:

python runpeptests.py --binary <path_to_binary> --profile-path <path_to_profile> --test-path <path_to_test_manifest> --log-file <path_to_logfile>

The process will exit with 0 for success, 1 for warnings, 2 for failure.

It needs to have mozrunner, mozprofile, mozprocess, mozlog and manifestdestiny available in a virtualenv. I believe that the runpeptests.py script itself and  the rest of the peptest will eventually end up in the tests.zip package.

The trickiest thing here will be setting up the virtualenv. I don't think we have any other tests that require their own virtualenv at this point.
(In reply to Chris AtLee [:catlee] from comment #0)
> The trickiest thing here will be setting up the virtualenv. I don't think we
> have any other tests that require their own virtualenv at this point.

For what it's worth those instructions were more general purpose and not really targeted at releng. I can package the harnesses' dependencies along with the harness for the purpose of buildbot. Although I think this ties in to some work that Jeff is doing with virtualenv and m-c so I'll cc him.
Mozharness has a way to install a virtualenv and use it now, if we decide to go that route. Alternately, puppeting the dependencies or untarring-and-PYTHONPATHing could also work.
Whiteboard: [tests] → [tests][mozbase]
So the master bug for virtualenv in m-c is bug 661908. The telegraphed
intent is that virtualenv is available on the system, though this
could be changed with bundling virtualenv in m-c, as it already is:

http://mxr.mozilla.org/mozilla-central/source/testing/mozmill/virtualenv/

While no active tests use virtualenv, both the mozmill and firebug
tests make use of it (they just aren't run on checkin; it is uncertain
whether they still work or not).  However, see bug 676078 and bug
685903 . Once bug 676078 is landed, virtualenv will be in the fairly
sane `other_licenses` directory.  So if we wanted to make use of it
we could, at least in the interim.  Or we could tackle bug 661908 and
have virtualenv be a prereq.

Likewise, as a lesson from bug 676078 and firebug "stealing" mozmill
packages, I would prefer to mirror internal dependencies (mozprocess,
mozprofile, mozlog [if this is used/existent]) to a generic directory
of m-c, such as `build/python-packages` or
`testing/python-packages`. We will have other testing frameworks that
will use these and automation.py.in and other utilities already in m-c
are scheduled to be rewritten as shims that utlitize these packages.
manifestparser.py and mozinfo.py are already in mozilla-central
(http://mxr.mozilla.org/mozilla-central/source/build/) but not in
packaged form.  We can introduce them in packaged form, or somehow
work with what is already there.

I am all for getting this to play with mozharness nicely too.
Presumedly peptest as it stands will remain a free-standing package,
and there will be a mozharness wrapper, though maybe you had a more
specific direction to take, :aki?

If there is general consensus that this is mostly the correct
approach, I would be happy to take this on (though, tbh, I'm not sure
if I feel like fighting the fight in bug 661908).
Some amendments to catlee's post.

1) jhammel pointed out that mozrunner depends on mozinfo, so that package is also required.
2) The --profile-path and --log-file parameters aren't necessary (in fact specifying a log-file means no output will go to stdout).
3) I'm also working on handling crashes which should be done soon, so a --symbol-path path/url should also be specified.
(In reply to Aki Sasaki [:aki] from comment #2)
> Mozharness has a way to install a virtualenv and use it now, if we decide to
> go that route. Alternately, puppeting the dependencies or
> untarring-and-PYTHONPATHing could also work.

Using mozharness sounds like the best route. Can it use a local package repository instead of hitting pypi?
(In reply to Chris AtLee [:catlee] from comment #5)
> (In reply to Aki Sasaki [:aki] from comment #2)
> > Mozharness has a way to install a virtualenv and use it now, if we decide to
> > go that route. Alternately, puppeting the dependencies or
> > untarring-and-PYTHONPATHing could also work.
> 
> Using mozharness sounds like the best route. Can it use a local package
> repository instead of hitting pypi?

Yes.
By default we'll point to pypi (so people can run the mozharness scripts in the community); in our production config files we'll change those defaults to internal urls.

I do need to:

a) only run --create-virtualenv the first time, and/or
b) make sure --create-virtualenv is idempotent
Whiteboard: [tests][mozbase] → [tests][mozbase][mozharness]
Aki, let me know what I need to do to get the harness ready for mozharness.

The harness should now return the proper error codes and handle test crashes.
See Also: → 674606
Assigning to Aki so he can provide the required mozharness info to Andrew.
Assignee: nobody → aki
Priority: -- → P5
Attached file Very early basic mozharness script (obsolete) —
This is my very basic mozharness script that probably has many things wrong with it. 

Things TODO:
- Log handling (not sure if anything is required here)
- Cleanup code
- It currently downloads the zipfiles of the repos that it uses (changeable via command line arg). This should be changed to use a configuration file.
- Add all the required peptest args
- Support downloading test repo or zipfile or similar (see https://wiki.mozilla.org/Auto-tools/Projects/peptest#Test_Manifest for what the --test-path parameter is expecting)

Basically I'm not really familiar with the build slave environments nor how buildbot will invoke this script so I don't want to proceed too much further without some kind of feedback.
Attachment #569808 - Flags: feedback?
Comment on attachment 569808 [details]
Very early basic mozharness script

>import urllib2
>import urlparse
>import tarfile
>import zipfile
>import shutil
>import os
> 

Nit: trailing whitespace!
I see this in a number of places in your script -- just a pet peeve of mine.

>        [["--binary"],

We may want to rename this, but I'm not going to harp too much on this at this point.

>        [["--test-path"],
>        {"action":"store",
>         "dest": "testPath",
>         "default":None,
>         "help": "Path to test manifest to run",
>        }],

Nit: During the initial mozharness review cycle(s), I switched to PEP 8 (http://www.python.org/dev/peps/pep-0008/) which has a variable name convention of lower_case_with_underscores.

>    def __init__(self, require_config_file=False):
>        self.python = None

I don't think you refer to self.python at all ?

>        super(PepTest, self).__init__(
>            config_options=self.config_options,
>            all_actions=['create-virtualenv',
>                         'install-deps',
>                         'install-peptest',
>                         'run-peptest'],

We probably need actions for:

1) clobbering your directory to start clean; we need to think about running this multiple times in a row reliably.
   To do this, you may want to start working in dirs['abs_work_dir'] (where dirs = self.query_abs_dirs()).
2) downloading and extracting firefox from someplace on ftp.m.o

>            default_actions=['create-virtualenv',
>                             'install-deps',
>                             'install-peptest',
>                             'run-peptest'],

I haven't yet decided what the default actions should be: assume that the virtualenv is already created, or no?  This should be fine, assuming that running create-virtualenv, install-deps, and install-peptest are all idempotent.

I suppose we could call create-virtualenv, install-deps, install-peptest from preflight_run_peptest() if we fail a try/except pass to check for all dependencies, but that's definitely extra credit.  Just mentioning since I thought of it.

>            config={'dependencies': ['mozlog',
>                                     'mozinfo',
>                                     'mozhttpd',
>                                     'mozprofile',
>                                     'mozprocess',
>                                     'mozrunner',
>                                     'manifestdestiny']},)

This actually doesn't work:

    PYTHONPATH=`pwd` python scripts/peptest.py --venv-path `pwd`/venv --create-virtualenv

gives a bare virtualenv.

Currently the config variable you're looking for is 'virtualenv_modules'.
Looks like s,dependencies,virtualenv_modules, fixes it, though you can set MODULENAME_url in your config to set a specific version.
(We'll be overriding these in the production configs to point to internal urls.)

Testing each individual action, as I am, above, is good practice.
If you get tired of specifying those things in your command line, creating a test config file like https://github.com/escapewindow/mozharness/blob/talosrunner/configs/users/aki/tablet1.py and running

    scriptname --config-file users/aki/tablet1.py [other options]

will let you set things like the virtualenv_path without having to respecify every test run.

>        cwd = os.getcwd()
>        os.chdir(files[0])

Please use self.chdir() to log this!

>        # install dependencies
>        for module in self.config['dependencies']:
>            os.chdir(module)
>            self.run_command(python + " setup.py install")
>            os.chdir("..")
>        os.chdir(cwd)

These places too.
The run_command() could use error detection; at the very least an

    self.run_command(python + " setup.py install", error_list=PythonErrorList)

would catch any python errors that are specified in mozharness.base.errors.
You can also specify cwd in the run_command() call to avoid all the chdir's.

>        shutil.rmtree(files[0])

self.rmtree() please :)

Same comments for install_peptest().

>    def run_peptest(self):
>        # TODO Peptest has many more cmdargs that should be passed in
>        cmdargs = ['--test-path', self.config['testPath'],
>                   '--binary', self.config['binary'],]
>        
>        peptest = self.query_python_path('peptest')
>        self.run_command(peptest + " " + " ".join(cmdargs))

I imagine peptest will throw warnings/errors and exit codes etc.; you'll want to add an error_list for the former and check the status for the latter.

>    def _download(self, url, savepath=''):
>        """
>        Save the file located at 'url' into 'savepath'
>        If savepath is a directory or None, use the last part of the url path.
>        Returns the path of the saved file.
>        """
>        data = urllib2.urlopen(url)
>        if savepath == '' or os.path.isdir(savepath):
>            parsed = urlparse.urlsplit(url)
>            filename = parsed.path[parsed.path.rfind('/')+1:]
>            savepath = os.path.join(savepath, filename)
>        savedir = os.path.dirname(savepath)
>        if savedir != '' and not os.path.exists(savedir):
>            os.makedirs(savedir)
>        outfile = open(savepath, 'wb')
>        outfile.write(data.read())
>        outfile.close()
>        return os.path.realpath(savepath)

is this self.download_file() ?

>    def _extract(self, path, savedir=None, delete=False):
>        """
>        Takes in a tar or zip file and extracts it to savedir
>        If savedir is not specified, extracts to path
>        If delete is set to True, deletes the bundle at path
>        Returns the list of top level files that were extracted
>        """
>        if zipfile.is_zipfile(path):
>            bundle = zipfile.ZipFile(path)
>            namelist = bundle.namelist()
>        elif tarfile.is_tarfile(path):
>            bundle = tarfile.open(path)
>            namelist = bundle.getnames()
>        else:
>            return
>        if savedir is None:
>            savedir = os.path.dirname(path)
>        elif not os.path.exists(savedir):
>            os.makedirs(savedir)
>        bundle.extractall(path=savedir)
>        bundle.close()
>        if delete:
>            os.remove(path)
>        return [os.path.join(savedir, name) for name in namelist 
>                    if len(name.rstrip(os.sep).split(os.sep)) == 1]

I like this, but we lose logging.
s,os.makedirs,self.mkdir_p,
s,os.remove,self.rmtree,

I'd personally almost lean towards a run_command("unzip ...") since it gives logging, but I'd like a generic extract function in mozharness.base.* .  This could be the start of one.

If it were to be generic, I'd like a verbose flag, or default to being verbose. I'd also like error checking to make sure the extraction was successful.  Extracting other file types could be future enhancements.

Overall though, this is a great start for someone who's unfamiliar with mozharness; thanks for the progress so far!
Attachment #569808 - Flags: feedback? → feedback+
Thanks for the feedback! I'll fix up the things you mentioned, couple comments inline:

(In reply to Aki Sasaki [:aki] from comment #10)
> Comment on attachment 569808 [details]
> >            config={'dependencies': ['mozlog',
> >                                     'mozinfo',
> >                                     'mozhttpd',
> >                                     'mozprofile',
> >                                     'mozprocess',
> >                                     'mozrunner',
> >                                     'manifestdestiny']},)
> 
> This actually doesn't work:
> 
>     PYTHONPATH=`pwd` python scripts/peptest.py --venv-path `pwd`/venv
> --create-virtualenv
> 
> gives a bare virtualenv.
> 
> Currently the config variable you're looking for is 'virtualenv_modules'.
> Looks like s,dependencies,virtualenv_modules, fixes it, though you can set
> MODULENAME_url in your config to set a specific version.
> (We'll be overriding these in the production configs to point to internal
> urls.)
The reason I used dependencies instead of virtualenv_modules was because VirtualenvMixin installs them with pip (filed bug 697462). Right now the pip story for a lot of those packages is kind of sad. Some of them aren't on there at all, some have multiple versions, and some of those versions are incompatible with the others. I guess my thinking was that create-virtualenv would just make an empty virtualenv and then the other two actions would populate it.. though as your comment above states, maybe these should be combined into one action (pre_flight_run_peptest).

> >    def _extract(self, path, savedir=None, delete=False):
> >        """
> >        Takes in a tar or zip file and extracts it to savedir
> >        If savedir is not specified, extracts to path
> >        If delete is set to True, deletes the bundle at path
> >        Returns the list of top level files that were extracted
> >        """
> >        if zipfile.is_zipfile(path):
> >            bundle = zipfile.ZipFile(path)
> >            namelist = bundle.namelist()
> >        elif tarfile.is_tarfile(path):
> >            bundle = tarfile.open(path)
> >            namelist = bundle.getnames()
> >        else:
> >            return
> >        if savedir is None:
> >            savedir = os.path.dirname(path)
> >        elif not os.path.exists(savedir):
> >            os.makedirs(savedir)
> >        bundle.extractall(path=savedir)
> >        bundle.close()
> >        if delete:
> >            os.remove(path)
> >        return [os.path.join(savedir, name) for name in namelist 
> >                    if len(name.rstrip(os.sep).split(os.sep)) == 1]
> 
> I like this, but we lose logging.
> s,os.makedirs,self.mkdir_p,
> s,os.remove,self.rmtree,
> 
> I'd personally almost lean towards a run_command("unzip ...") since it gives
> logging, but I'd like a generic extract function in mozharness.base.* . 
> This could be the start of one.
> 
> If it were to be generic, I'd like a verbose flag, or default to being
> verbose. I'd also like error checking to make sure the extraction was
> successful.  Extracting other file types could be future enhancements.
The download and extract functions were things that I found myself needing over and over again in my various projects, so I decided to just write some generic methods. Feel free to use them as base mozharness functions if you want. Historically I've left the error handling up to the code that calls these functions, but I can add some in if you think it would be helpful.
Attached patch Peptest script v1.0 (obsolete) — Splinter Review
Apologies for the wait, I got a bit sidetracked. I think this patch addresses most of the things you brought up in the last feedback. It includes peptest.py as well as a test config file I was using (not proposing this gets committed, just adding it to the patch for example purposes).

I still won't request review due to the error in the first note and because there are still some minor tweaks I want to add. But I'd still appreciate some more feedback.

Notes:

* For some reason if I point the binary to my local nightly on my machine everything works fine. But if I point it to a tinderbox build (i.e run 'python peptest.py --cfg config_file --get-latest-tinderbox --run-peptest') run_command() hangs waiting for output. I'll investigate further (may be a peptest bug).

* I added error checking and logging to the _extract method.

* The _which() and _query_package() methods could also be added to mozharness base. I feel like _which should probably be called by query_python_path() internally (that is the only use case I'm using it for).

* I didn't change 'dependencies' to 'virtualenv_modules' yet for more control. Instead I put the create_virtualenv action inside of the pre_flight_run_peptest action. I may change this later.

* It would be nice if mozharness depended on mozbase so that the mozbase utilities would be available by default to all mozharness scripts.

* I'm still not sure how buildbot will be calling this script. Right now I'm assuming it will call run-peptest and pass in an appname. Which means I'm not entirely sure what the purpose of the get-latest-tinderbox action is, but it's there anyway.

* Is there some way to tell mozharnesses which output lines are to be interpreted as DEBUG statements?
Assignee: aki → ahalberstadt
Attachment #569808 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #571071 - Flags: feedback?(aki)
(In reply to Andrew Halberstadt [:ahal] from comment #12)
> Apologies for the wait,

No worries.

> I still won't request review due to the error in the first note and because
> there are still some minor tweaks I want to add. But I'd still appreciate
> some more feedback.

Sure.

> * For some reason if I point the binary to my local nightly on my machine
> everything works fine. But if I point it to a tinderbox build (i.e run
> 'python peptest.py --cfg config_file --get-latest-tinderbox --run-peptest')
> run_command() hangs waiting for output. I'll investigate further (may be a
> peptest bug).

I get further if I s,getlatesttinderbox,GetLatestTinderbox,
It seems to be spelled that way on pypi.
http://pypi.python.org/pypi/GetLatestTinderbox/0.2.10

I'm not entirely sure GetLatestTinderbox is what we're looking for in buildbot land; generally we know the path to the build we want to test there, so a self.download_file(c.get("browser_url")) might do the trick.  But it might be helpful for someone who's testing on their local desktop.

> * I added error checking and logging to the _extract method.

Great!

> * The _which() and _query_package() methods could also be added to
> mozharness base. I feel like _which should probably be called by
> query_python_path() internally (that is the only use case I'm using it for).

Hm, sure. I tend to fall through to just calling the bare exe name, and if it's missing from PATH the output will tell me.  But that can be dangerous in get_output_from_command, sure.

> * I didn't change 'dependencies' to 'virtualenv_modules' yet for more
> control. Instead I put the create_virtualenv action inside of the
> pre_flight_run_peptest action. I may change this later.

Ok.  A note on this:

preflight_ACTIONNAME is a special method.  It's always run before the action:
http://hg.mozilla.org/build/mozharness/file/b4f5b4964827/mozharness/base/script.py#l491

This is to allow for error- and prerequisite- checking before actually doing anything.

However, if you wanted to run peptest on the same machine 50x, it might get annoying or slow to have the virtualenv created every single time.  So having a separate action to create the virtualenv is good.  If you want to either error out, or attempt to create the virtualenv in preflight_run_peptest()  if it's missing or incomplete, that's great (since someone might call peptest.py --run-peptest without having created the virtualenv first).  But pre-flight-run-peptest should probably get renamed back to create-deps or something :)

I'm doing this here:
https://github.com/escapewindow/mozharness/blob/e7b559a89f3c3ca6810c1159908f74572bd1e105/scripts/device_talosrunner.py#L355
You could do something similar, if you wish:

def preflight_run_peptest(self):
    if 'create-deps' not in self.actions:
        # verify the virtualenv and all dependencies are in place

> * It would be nice if mozharness depended on mozbase so that the mozbase
> utilities would be available by default to all mozharness scripts.

Possibly.
Right now I'm trying to

a) make sure that we can clone mozharness and run the scripts without doing anything else;
b) make sure that mozharness.base.* has zero mozilla-specific hardcodes.

If I abandon (a), and split mozharness.base.* out into, say, a package called ScriptHarness, then Mozharness could potentially depend on both ScriptHarness and MozBase.

> * I'm still not sure how buildbot will be calling this script. Right now I'm
> assuming it will call run-peptest and pass in an appname. Which means I'm
> not entirely sure what the purpose of the get-latest-tinderbox action is,
> but it's there anyway.

It will most likely pass in an ftp.m.o URL, which the script will need to download and extract.  So as mentioned above, get-latest-tinderbox might not be a critical part of this at all.

> * Is there some way to tell mozharnesses which output lines are to be
> interpreted as DEBUG statements?

If you mean logging.DEBUG, then you can set the level to DEBUG in the error_list for run_command().  The output will be hidden unless you set |--log-level debug|.

I still haven't actually looked in-depth at the script+config yet; that's coming.
Comment on attachment 571071 [details] [diff] [review]
Peptest script v1.0

>+        self.appname = self.config['appname']
>+        self.symbols = self.config['symbols_path']

I don't think these are required at all since we can always access self.config, but are harmless.

>+    def get_latest_tinderbox(self):

If this is slowing you down too much, feel free to skip and I can genericize what I've got in DeviceTalosRunner.download()
https://github.com/escapewindow/mozharness/blob/e7b559a89f3c3ca6810c1159908f74572bd1e105/scripts/device_talosrunner.py#L236

As noted there, however, getting the latest tinderbox build is a user-friendly default, so if this works we could genericize this and make it available to other scripts that want to download a build.

>+    def run_peptest(self):
>+        if "pre-flight-run-peptest" not in self.actions:
>+            # ensure all the dependencies are installed
>+            for module in self.config['dependencies'] + ['peptest']:
>+                if len(self._query_package(module)) == 0:
>+                    self.action_message("Dependencies missing, " +
>+                                        "running pre-flight-run-peptest step")
>+                    self.pre_flight_run_peptest()
>+                    break

You've got the right structure, the logic in helper methods; as mentioned above I think you need to rename to a create-deps action, a create_deps() method that calls  the helper methods, and a preflight_run_peptest() that makes sure everything's installed before running would be extra credit user-friendly.

Sorry if my initial comment about preflight_* was confusing.  Feel free to ping if my re-explanation is still confusing.

>+            cmd.extend(self._create_arg('--log-level',
>+                       self.config['log_level'].upper()))
>+
>+        self.run_command(cmd,
>+                         error_list=error_list)

Since running peptest is the point of this script, it might be nice to look for status and set a summary message.

level = INFO
status = self.run_command(cmd, error_list=error_list)
if status:
    level = ERROR
self.add_summary("%s exited with status %d." % (cmd, status), level=level)

Since this is measuring something, exit status probably isn't the best thing to summarize, even though it's probably easiest.  If there's a useful number, or some sort of "browser X is (peppy|not peppy|molasses slow)!" metric that might go here.

If there's particular line(s) in the output that gives a good summary, it would probably be a good thing to add a summary=True special keyword to run_command's log parsing.  I should file a bug about that.

Summary messages get output in the log like everything else, at the log_level specified (default INFO), but also get output at the very end of the script.  If we end up emailing or sending status messages elsewise the summary would probably be part of that as well.

>+    def _extract(self, path, extdir=None, delete=False,
>+                 error_level=ERROR, exit_code=-1):

I like this quite a bit; it seems at home in mozharness.base.script.
(If you want to do that; cool; otherwise, I'll most likely move it if/when
I need to use this method elsewhere.)

>+    def _query_package(self, package_name):
>+        """
>+        Returns a list of all installed packages
>+        that contain package_name in their name
>+        """
>+        pip = self._which(self.query_python_path("pip"))
>+        if not pip:
>+            return []
>+        packages = self.get_output_from_command(pip + " freeze",
>+                                                silent=True).split()
>+        return [package for package in packages
>+                if package.lower().find(package_name.lower()) != -1]

Cool.
Yeah, I don't know what to do every time when there's an error, either.
One thing might be

    def _query_package(self, package_name, error_level=WARNING):
        ...
        if not pip:
            self.log("Can't find pip in PATH!", level=error_level)
            return []

and we can set error_level to FATAL if we want it to die, or w/e.
This works as is, though, and can be enhanced later when we know what types of issues we hit with it.

>+    def _which(self, program):
>+        """
>+        OS independent implementation of Unix's which command
>+        Takes in a program name
>+        Returns path to executable or None
>+        """

Cool, we can definitely use this in mozharness.base.script.
I added query_exe() to the talosrunner branch recently:
https://github.com/escapewindow/mozharness/blob/a6df483b440273b1ced13d060408fa564af7a7e7/mozharness/base/script.py#L255
which allows you to specify paths for executables outside your PATH.
These should play nicely with each other.

I haven't tried running this -- are you all good other than the latest tinderbox stuff?  Overall this is definitely coming together; let me know if you need anything to get it finished.
Attachment #571071 - Flags: feedback?(aki) → feedback+
(In reply to Aki Sasaki [:aki] from comment #13)
> I get further if I s,getlatesttinderbox,GetLatestTinderbox,
> It seems to be spelled that way on pypi.
> http://pypi.python.org/pypi/GetLatestTinderbox/0.2.10

Hm, you're saying that you can't even get the script to install GetLatestTinderbox? Afaict pypi is case insensitive, but I'll change it if it doesn't work for you. The problem I was describing is that the browser session hangs after the peptests have completed. I meant that it wasn't working with the actual tinderbox build (sorry for confusing choice of words). I did some investigation and this happens outside of mozharness land as well, so lets not discuss here.

> I'm not entirely sure GetLatestTinderbox is what we're looking for in
> buildbot land; generally we know the path to the build we want to test
> there, so a self.download_file(c.get("browser_url")) might do the trick. 
> But it might be helpful for someone who's testing on their local desktop.

I'll add a check for if isURL(self.config['appname']) and download/install if it is. I guess get-latest isn't horribly useful, but maybe someone running it outside of buildbot might find it convenient.

> If I abandon (a), and split mozharness.base.* out into, say, a package
> called ScriptHarness, then Mozharness could potentially depend on both
> ScriptHarness and MozBase.

By depend on I mean in the python package world. I.e, add dependencies to the mozharness setup.py so that when I make my mozharness virtualenv, all the mozbase packages can be imported into the mozharness scripts.
 
(In reply to Aki Sasaki [:aki] from comment #14)
> >+        self.appname = self.config['appname']
> >+        self.symbols = self.config['symbols_path']
> 
> I don't think these are required at all since we can always access
> self.config, but are harmless.

These potentially get modified by get-latest-tinderbox, and are needed since self.config is read-only.

> >+    def get_latest_tinderbox(self):
> 
> If this is slowing you down too much, feel free to skip and I can genericize
> what I've got in DeviceTalosRunner.download()
> 
> As noted there, however, getting the latest tinderbox build is a
> user-friendly default, so if this works we could genericize this and make it
> available to other scripts that want to download a build.

I think this is more or less done (it works fine for me at least). I'm all for genericizing it. Otherwise I'll just leave it here in case someone runs this script outside of buildbot.

> Cool, we can definitely use this in mozharness.base.script.

Should I leave integrating the _extract, _query_package and _which methods up to you? Otherwise I can file some bugs and upload some patches when I have the time.

> I haven't tried running this -- are you all good other than the latest
> tinderbox stuff?  Overall this is definitely coming together; let me know if
> you need anything to get it finished.

Yep, everything seems to work (tinderbox stuff included). If I can summarize your comments, other than some cleanup, re-factoring and status the main (only?) critical thing I need to do to get this working is to download and install a build url that will be passed in by buildbot.

I do have one question though. Peptest needs a --test-path parameter that specifies a manifest of tests to run. This means that the tests need to be accessible somewhere on the file system. Do I need to write an action that checks out a test repository or something? Or will buildbot have the packaged tests already downloaded and extracted?
FWIW, there is also http://brasstacks.mozilla.com/latestbuilds/ as a getlatest-tinderbox alternative.
(In reply to Andrew Halberstadt [:ahal] from comment #15)
> Hm, you're saying that you can't even get the script to install
> GetLatestTinderbox?

That's correct, not without modifying it.

> I'll add a check for if isURL(self.config['appname']) and download/install
> if it is. I guess get-latest isn't horribly useful, but maybe someone
> running it outside of buildbot might find it convenient.

That works; if we hit an instance where we need appname to only refer to the name of the application, it's easy enough to add another config item later.
 
> By depend on I mean in the python package world. I.e, add dependencies to
> the mozharness setup.py so that when I make my mozharness virtualenv, all
> the mozbase packages can be imported into the mozharness scripts.

Yes. However, if someone runs the script without using setup.py, they may not have mozbase in their virtualenv unless it's specified in virtualenv_modules.

> (In reply to Aki Sasaki [:aki] from comment #14)
> > >+        self.appname = self.config['appname']
> > >+        self.symbols = self.config['symbols_path']
> > 
> > I don't think these are required at all since we can always access
> > self.config, but are harmless.
> 
> These potentially get modified by get-latest-tinderbox, and are needed since
> self.config is read-only.

Ah, ok.
 
> > >+    def get_latest_tinderbox(self):
> > 
> > If this is slowing you down too much, feel free to skip and I can genericize
> > what I've got in DeviceTalosRunner.download()
> > 
> > As noted there, however, getting the latest tinderbox build is a
> > user-friendly default, so if this works we could genericize this and make it
> > available to other scripts that want to download a build.
> 
> I think this is more or less done (it works fine for me at least). I'm all
> for genericizing it. Otherwise I'll just leave it here in case someone runs
> this script outside of buildbot.

I had issues running it, but I didn't delve too deep into those issues.
I'm ok with leaving it since production won't run it.

> > Cool, we can definitely use this in mozharness.base.script.
> 
> Should I leave integrating the _extract, _query_package and _which methods
> up to you? Otherwise I can file some bugs and upload some patches when I
> have the time.

Either way works for me.
If you have interest+time, please do; I'm happy to otherwise.

> Yep, everything seems to work (tinderbox stuff included). If I can summarize
> your comments, other than some cleanup, re-factoring and status the main
> (only?) critical thing I need to do to get this working is to download and
> install a build url that will be passed in by buildbot.

Yes.  Config files that work for a generic user and for production would be good, but only the latter is required for production use.  However, there's your next question:

> I do have one question though. Peptest needs a --test-path parameter that
> specifies a manifest of tests to run. This means that the tests need to be
> accessible somewhere on the file system. Do I need to write an action that
> checks out a test repository or something? Or will buildbot have the
> packaged tests already downloaded and extracted?

If you look in, say, http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-mozilla-central/ , you'll see there are installers as well as test.zip's for each platform.

So the download+extract will probably need to grab/extract both the browser and tests.  Production will need to specify this either in the config file or via commandline.

Could you add this functionality and make sure this script works for you when you're using a downloaded browser + tests?
Notes:

* There are three config files. prod_config.py and user_config.py are templates with comments and are very similar to each other. test_config.py *should* work as is without any modification (assuming you're using linux). If you aren't using linux, just point the appname config to a build for your platform.

* For testing purposes I uploaded a zipfile of the tests to the peptest github page. This way we can pretend like this file is the packaged tests until we actually get the tests into the real tests.zip

* This adds the which() method to OSMixin and the query_package() method to VirtualEnvMixin. Further more, I made query_python_path() call which() internally. This adds a dependency to OSMixin and might break other scripts, so I can remove it if you want.

* I tested under windows and linux (didn't have a mac environment, though the mac specific code such as dmg extracting in mozinstall has been tested under osx)
Attachment #571071 - Attachment is obsolete: true
Attachment #572078 - Flags: review?(aki)
Another note: The get-latest-tinderbox step likely won't work in Windows since GetLatestTinderbox depends on lxml which pip tries to compile. The compilation requires some libraries and will fail if you don't have them (libxml + another one).
Comment on attachment 572078 [details] [diff] [review]
Patch 1.0 - Mozharness peptest script

(In reply to Andrew Halberstadt [:ahal] from comment #18)
> * This adds the which() method to OSMixin and the query_package() method to
> VirtualEnvMixin. Further more, I made query_python_path() call which()
> internally. This adds a dependency to OSMixin and might break other scripts,
> so I can remove it if you want.

This also makes OSMixin dependent on ShellMixin.
However, I think keeping them separate is difficult and somewhat arbitrary; I don't think there's anything that inherits one that doesn't inherit the other.

Most likely these two will merge into one big Mixin at some point.

> * I tested under windows and linux (didn't have a mac environment, though
> the mac specific code such as dmg extracting in mozinstall has been tested
> under osx)

Awesome.

We don't yet know that this will work perfectly in production, but given local testing we have a decent amount of confidence we can get it there with minimal fuss.  The code is well written and fits well into the mozharness framework.  If we wanted, we could easily expand this script to handle all desktop unit test types.

A few points:

* there's no need to re-specify base_work_dir and work_dir, which are set to the defaults. Redefining them doesn't hurt, however.

* I prefer to have the script callable directly; I'll add a #!/usr/bin/env python and chmod a+x; sorry for not pointing this out earlier.

* Now that I think about it, adding the default virtualenv_path and adding a 'which' into query_python_path() will possibly introduce confusing virtualenv- or non-virtualenv python behavior.  I may end up tweaking this or adding a 'require_virtualenv' or other such config variable.  I'm going to try not to overthink it and just leave it until we hit an issue, though.

Overall it's an r+.
Would you like me to land it?
Attachment #572078 - Flags: review?(aki) → review+
Summary: Set up buildbot code for peptest → Set up mozharness code for peptest
Comment on attachment 572078 [details] [diff] [review]
Patch 1.0 - Mozharness peptest script

http://hg.mozilla.org/build/mozharness/rev/023fafc291a5

Landed.  It's easy enough to patch later if we want further changes, and I have some ideas about how to get mozbase supported for mozharness scripts in general.
Attachment #572078 - Flags: checked-in+
Aki, awesome! Let me know if you need me to fix, update or add anything to this script. I'll file new bugs if I find I need additional functionality.

Re: re-specifying work_dir, I only did it so that I could make the virtualenv_path relative to the work dir. I mainly did this because I wasn't sure if clobber should only delete the abs_work_dir or also the virtualenv_path. I solved the problem by making sure virtualenv_path was in abs_work_dir.

If you want to get a discussion started on mozharness + mozbase  (maybe next mozharness meeting?) I'd be happy to contribute and/or help if anything needed doing on the mozbase side.

Also thanks for reviewing, committing and the general help. I had to write some buildbot code last year and mozharness was so so much nicer and easier to develop on and (especially) test with.
Aki, thanks again for your help.  What's the next set of steps here in order to get peptest running on TryServer?  I'd like to deploy it first to try server so that we can give developers a chance to submit tests for the harness and experiment with how it works before we activate it for Mozilla-Central or Mozilla-Inbound.

Next set of steps from an ateam side:
* ensure package-tests package up peptest
* figure out how to create make targets for peptest (how do we get mozharness installed on developers machines when they do this?)
* Evangelize the "new make target" world with mozharness - i.e. make target will now hit the network if needed.

Next set of steps from the Releng side:
* ???
Blocks: 700415
(In reply to Clint Talbert ( :ctalbert ) from comment #23)
> Next set of steps from an ateam side:
> * ensure package-tests package up peptest

Yes.

> * figure out how to create make targets for peptest (how do we get
> mozharness installed on developers machines when they do this?)
> * Evangelize the "new make target" world with mozharness - i.e. make target
> will now hit the network if needed.

I don't have an answer here right now.
I'm open to suggestions?
One solution would be to not call mozharness, but call the peptest script directly.
Another would be to clone and run mozharness.
Another would be to refer to the mozharness script and not have a makefile target (not sure how much pushback this would get).
Another would be to check everything into m-c and deal with updating it as things change.... I'd rather not do this, but developers are often particular about everything being in m-c.

> Next set of steps from the Releng side:

* bug 700415.
I think remaining work is tracked in bug 700415 and bug 700656; resolving.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Product: mozilla.org → Release Engineering
Component: Other → Mozharness
You need to log in before you can comment on or make changes to this bug.