789266 - Make Talos trivial to run from your m-c checkout

Status: RESOLVED DUPLICATE of bug 895451

(Testing :: Talos, defect)

Description

[Justin Lebar (not reading bugmail)]

It seems that although there wasn't consensus on bug 787200, there was consensus that we should make it easier to run Talos locally.

The goal is to make it as easy to run Talos (the full suite, or a particular test) as it is to run a mochitest:

  TEST_PATH=foo/bar/baz make mochitest-plain

Comment 1

[Joel Maher ( :jmaher ) (UTC -8)]

I like this idea and it was suggested to use mach in bug 787200.  

There are a few quirks here (which can all be worked around):
* we need to run talos from a virtualenv and we require pywin32 which is very specific to the os version and python version
* we need to close the existing firefox browser
* the resulting numbers should not be compared to those produced by tbpl or what is on graph server.

Would making this as a first step be acceptable:
hg clone <talos>
python INSTALL.py
<on windows install pywin32>
objdir> TALOS_PATH=~/talos make ts_paint

We have some workarounds we can do for some of this stuff to make it 100% seemless, but we can make this happen faster if we make some assumptions up front.

Lastly make or mach?

Comment 2

[Jeff Hammel]

(In reply to Joel Maher (:jmaher) from comment #1)
> I like this idea and it was suggested to use mach in bug 787200.  

AIUI, mach is allowed to touch the web.  This is a big help, IMHO.
 
> There are a few quirks here (which can all be worked around):
> * we need to run talos from a virtualenv and we require pywin32 which is
> very specific to the os version and python version

So if we install from `easy_install` or `python setup.py {develop|install}` the existing talos setup.py should handle that.

> * we need to close the existing firefox browser

Currently Talos doesn't do this.  It will however refuse to launch if Fx is running. Are you suggesting that we hard-close a browser?  I imagine this will cause other complaints.

Relatedly, if Firefox is running we're (probably) not going to get very realistic numbers.[There are also bugs about process management since we're not yet on mozprocess and instead look for e.g. Firefox in `ps` and other bad design decisions that can be fixed with mozprocess.  But lets pretend those are fixed.  They are fixable.] However, if you're doing anything else intensive -- like, say, building Firefox -- the numbers will also be rubbish.  While I'm not sure there is an easy technological solution here, we should certainly try to communicate that running Talos will completely tie up a machine if you want meaningful numbers.

> * the resulting numbers should not be compared to those produced by tbpl or
> what is on graph server.
> 
> Would making this as a first step be acceptable:
> hg clone <talos>
> python INSTALL.py
> <on windows install pywin32>
(Note python INSTALL.py should already take care of this)

> objdir> TALOS_PATH=~/talos make ts_paint
I'm not sure I grep what this means.  Shouldn't this be e.g. `mach ts_paint`?

IMHO it is better to use the revision of talos from http://mxr.mozilla.org/mozilla-central/source/testing/talos/talos.json . I don't think we should use the .zip for two reasons:

1. We don't want to and it will go away: bug 764588
2. The URL there isn't accessible anyway.

*IFF* we use a parseable format for the zip URL, we can get the version out and ensure the version of talos we use is what buildbot is using for the tree. But +1 on `hg clone`

I would prefer we install talos in the same virtualenv that objdir already uses.

> We have some workarounds we can do for some of this stuff to make it 100%
> seemless, but we can make this happen faster if we make some assumptions up
> front.
> 
> Lastly make or mach?

mach, IMHO.

Also, another big concern is buildbot configs.  Currently, these don't live in the tree.  I assume the make targets should correspond to the TBPL names, e.g. http://k0s.org:8080/tbpl . (And if not, what do they correspond to and how do we figure out Talos command lines?) If we moved the buildbot config data necessary into testing/talos, we could more sensibly get these numbers.  Otherwise, I'm not sure if there is a sensible way to ensure what you're running == what buildbot is running.

A final question: do we care about mobile?  Maybe that can come later?

Comment 3

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

(In reply to Justin Lebar [:jlebar] from comment #0)
>   TEST_PATH=foo/bar/baz make mochitest-plain

I do this a lot, and it's pretty horrible, imho. I expect this to end up being something like

mach mochitest-plain foo/bar/baz

or

mach mochitest-plain --path=foo/bar/baz

...so talos had better use something like

mach talos (some identifier)

(In reply to Jeff Hammel [:jhammel] from comment #2)
> > * we need to close the existing firefox browser
> 
> Currently Talos doesn't do this.  It will however refuse to launch if Fx is
> running. Are you suggesting that we hard-close a browser?  I imagine this
> will cause other complaints.

Agreed.

> IMHO it is better to use the revision of talos from
> http://mxr.mozilla.org/mozilla-central/source/testing/talos/talos.json . I
> don't think we should use the .zip for two reasons:
> 
> 1. We don't want to and it will go away: bug 764588
> 2. The URL there isn't accessible anyway.
> 
> *IFF* we use a parseable format for the zip URL, we can get the version out
> and ensure the version of talos we use is what buildbot is using for the
> tree. But +1 on `hg clone`

Yeah; IMO we should just put the changeset id somewhere in m-c and make sure that all our infra uses that.

> Also, another big concern is buildbot configs.  Currently, these don't live
> in the tree.  I assume the make targets should correspond to the TBPL names,
> e.g. http://k0s.org:8080/tbpl . (And if not, what do they correspond to and
> how do we figure out Talos command lines?) If we moved the buildbot config
> data necessary into testing/talos, we could more sensibly get these numbers.
> Otherwise, I'm not sure if there is a sensible way to ensure what you're
> running == what buildbot is running.

I, for one, would love it if there was a list of tests in the talos repo, and mach, buildbot, and tbpl all got their data from that single place of truth.

> A final question: do we care about mobile?  Maybe that can come later?

You know I don't :)

Comment 4

[Jeff Hammel]

(In reply to :Ms2ger (Gone until 20120914) from comment #3)
> (In reply to Justin Lebar [:jlebar] from comment #0)
> >   TEST_PATH=foo/bar/baz make mochitest-plain
> 
> I do this a lot, and it's pretty horrible, imho. I expect this to end up
> being something like
> 
> mach mochitest-plain foo/bar/baz
> 
> or
> 
> mach mochitest-plain --path=foo/bar/baz
> 
> ...so talos had better use something like
> 
> mach talos (some identifier)

Currently, talos has a predefined set of tests at http://hg.mozilla.org/build/talos/file/1c5976f92643/talos/test.py that are displayed with `talos --print-tests`.  There is no good way to run a subset of pageloader tests though I filed bug 789488 about this.  so as it stands, this would be something like:

mach talos tsvg

or

mach talos tsvg:ts

> (In reply to Jeff Hammel [:jhammel] from comment #2)
> > > * we need to close the existing firefox browser
> > 
> > Currently Talos doesn't do this.  It will however refuse to launch if Fx is
> > running. Are you suggesting that we hard-close a browser?  I imagine this
> > will cause other complaints.
> 
> Agreed.
> 
> > IMHO it is better to use the revision of talos from
> > http://mxr.mozilla.org/mozilla-central/source/testing/talos/talos.json . I
> > don't think we should use the .zip for two reasons:
> > 
> > 1. We don't want to and it will go away: bug 764588
> > 2. The URL there isn't accessible anyway.
> > 
> > *IFF* we use a parseable format for the zip URL, we can get the version out
> > and ensure the version of talos we use is what buildbot is using for the
> > tree. But +1 on `hg clone`
> 
> Yeah; IMO we should just put the changeset id somewhere in m-c and make sure
> that all our infra uses that.

I'm fine adding the changeset ID to talos.json.  I'm also fine (though less so, as it seems like the sort of thing that would break).

> > Also, another big concern is buildbot configs.  Currently, these don't live
> > in the tree.  I assume the make targets should correspond to the TBPL names,
> > e.g. http://k0s.org:8080/tbpl . (And if not, what do they correspond to and
> > how do we figure out Talos command lines?) If we moved the buildbot config
> > data necessary into testing/talos, we could more sensibly get these numbers.
> > Otherwise, I'm not sure if there is a sensible way to ensure what you're
> > running == what buildbot is running.
> 
> I, for one, would love it if there was a list of tests in the talos repo,
> and mach, buildbot, and tbpl all got their data from that single place of
> truth.

I would also love this.  I know Aki had some plans to put some of the buildbot suite information into the talos.json.  As for the rest of it, we can certainly make things better, but there's a lot of spaghetti in the pot.  See http://k0s.org/mozilla/blog/20120724135349 for where all of this comes from.  There's a bunch we can do to make this better and its a lot of little things versus one giant magic thing.

Comment 5

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: very hack patch to run talos from mach

As I am new to mach I am probably overlooking some very basic things in this patch.  The good news is this patch works great for:
../mach talos tsvg

I would really like to remove some assumptions and make sure this works for the people that are running from inside of m-c.

Comment 6

[Joel Maher ( :jmaher ) (UTC -8)]

for reference, I am getting mach from:
http://gregoryszorc.com/blog/2012/08/15/build-firefox-faster-with-build-splendid/

Comment 7

[Gregory Szorc [:gps]]

Comment on attachment 659314 [details] [diff] [review]
very hack patch to run talos from mach

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

This looks pretty good! The Mercurial cloning is a bit hacky. We probably want config integration to choose the destination directory, etc.

You are probably basing this off of my standalone build-splendid patch. In that patch, I copied mozbuild to mozbuild-bs to avoid merge conflicts. If this lands, you'll want to base things off of python/mozbuild/ not python/mozbuild-bs/.

::: python/mozbuild-bs/mozbuild/testing/talos.py
@@ +3,5 @@
> +# You can obtain one at http://mozilla.org/MPL/2.0/.
> +
> +# This modules contains code for interacting with xpcshell tests.
> +
> +import os.path

import os

The Python people tell me this is how you are supposed to do it. I'm taking them at their word.

@@ +13,5 @@
> +
> +
> +class TalosRunner(Base):
> +    """Run talos tests."""
> +    tests = ['dromaeojs', 'kraken', 'sunspider', 'ts', 'tpaint', 'v8', 'dromaeo_css', 'dromaeo_lib', 'a11y', 'tsvg', 

Nit: trailing whitespace

@@ +17,5 @@
> +    tests = ['dromaeojs', 'kraken', 'sunspider', 'ts', 'tpaint', 'v8', 'dromaeo_css', 'dromaeo_lib', 'a11y', 'tsvg', 
> +             'tsvg_opacity', 'tdhtml']
> +
> +    def run_suite(self):
> +        print "You must run a single test"

You shouldn't print from library modules. raise instead?

@@ +30,5 @@
> +            raise Exception('Test name %s is not in the known list of tests: %s' % (test_name, self.tests))
> +
> +        firefox_app = os.path.join(os.path.abspath('.'), 'dist', 'bin', 'firefox')
> +        if not os.path.exists(firefox_app):
> +            raise Exception('Unable to find the Firefox binary: %s' % (firefox_app))

We probably want to hook up mozrunner here. jhammel could tell you more.

@@ +35,5 @@
> +
> +        self.setup()
> +        self.log_manager.enable_unstructured()
> +        # TODO: This doesn't support the proper application name
> +        os.system('talos/bin/python talos/talos/run_tests.py -v -e %s -a %s --develop --results_url=file://%s' % (firefox_app, test_name, os.path.join(os.path.abspath('.'), 'results.out')))

self._run_command() provides a handy API for invoking processes, complete with output capturing, etc. See base.py.

Comment 8

[Jeff Hammel]

Comment on attachment 659314 [details] [diff] [review]
very hack patch to run talos from mach

+        talos.add_argument('test_name', default='ts', nargs='?',

I'm inclined not to have a default.  Also, ideally we'd like a way to print the possible tests (a la `talos --print-tests)

+    tests = ['dromaeojs', 'kraken', 'sunspider', 'ts', 'tpaint', 'v8', 'dromaeo_css', 'dromaeo_lib', 'a11y', 'tsvg', 
+             'tsvg_opacity', 'tdhtml']

This already is contained in talos.  Why are we repeating this?

+        """Runs an individual xpcshell test."""
...

+        firefox_app = os.path.join(os.path.abspath('.'), 'dist', 'bin', 'firefox')

Is this firefox.exe on windows?  (Also, it'd be really nice to have a generate method/property to get this path)

+        # TODO: This doesn't support the proper application name
Not sure what this means?

+        os.system('talos/bin/python talos/talos/run_tests.py -v -e %s -a %s --develop --results_url=file://%s' % (firefox_app, test_name, os.path.join(os.path.abspath('.'), 'results.out')))

I *guess* this is okay?  I'd be more inclined to use talos/bin/talos (on linux) and talos\Scripts\talos.exe on windows.  I don't know how mach handles virtualenv stuff (or not) but this would be nice to have a front-end for too (as mozharness has and most any sort of automation that deals with virtualenvs on multiple platforms).  :gps?

I'm also much less inclined to use --results_url vs --datazilla-url.  Though we could have both.

Also, why os.system?  If we don't have our own front-end for spawning processes, I'm more inclined to subprocess.call.

+            os.system('python talos/INSTALL.py')

For m-c I'm much more inclined to have this use the objdir's virtualenv directory.  Though I wouldn't want to block landing on this.

I'm also inclined to figure out the version of talos in m-c.  But I wouldn't want to block landing on that either.

Nice work, though, except these nits

Comment 9

[Jeff Hammel]

> import os

> The Python people tell me this is how you are supposed to do it. I'm taking them at their word.

Quite :) import os.path has been deprecated since at least 2.3 maybe before

Comment 10

[Jeff Hammel]

> We probably want to hook up mozrunner here. jhammel could tell you more.

Wait, can I? ;) Talos doesn't use mozrunner (though we really really want it to, just not much manpower in that direction right now).  Though I'm not sure what the context of this comment is...

Comment 11

[Gregory Szorc [:gps]]

(In reply to Jeff Hammel [:jhammel] from comment #10)
> > We probably want to hook up mozrunner here. jhammel could tell you more.
> 
> Wait, can I? ;) Talos doesn't use mozrunner (though we really really want it
> to, just not much manpower in that direction right now).  Though I'm not
> sure what the context of this comment is...

I thought mozrunner was just a generic library to find and execute binaries. We could certainly use the finding part here, right? Or, do I not understand how mozrunner works (I haven't looked at the API).

Jeff: I assumed since mozrunner is part of mozbase that you know what's going on here :)

Comment 12

[Jeff Hammel]

mozrunner no longer has logic to find e.g. firefox and hasn't for some time.  It is only used to run firefox :)

Although I do recommend a method (somewhere, I'm not yet up to speed on mach/etc) that gives the platform-appropriate location of firefox{.exe}

Comment 13

[Gregory Szorc [:gps]]

(In reply to Jeff Hammel [:jhammel] from comment #12)
> mozrunner no longer has logic to find e.g. firefox and hasn't for some time.
> It is only used to run firefox :)
> 
> Although I do recommend a method (somewhere, I'm not yet up to speed on
> mach/etc) that gives the platform-appropriate location of firefox{.exe}

At some point mozbuild will likely grow an API to manage the binaries. I'm thinking it's just an object attached to Base with properties that return the full paths to the binaries. e.g.

  self.binaries.firefox
  self.binaries.js
  self.binaries.xpcshell

If you want to create a simple dict on Base now, I think that would be sufficient. If you don't, don't worry about it.

Comment 14

[Joel Maher ( :jmaher ) (UTC -8)]

good idea;  I saw you referred to 'make' targets for mochitest.  I was trying to avoid adding a new make target and then using that in the mach command.

Comment 15

[Gregory Szorc [:gps]]

(In reply to Joel Maher (:jmaher) from comment #14)
> good idea;  I saw you referred to 'make' targets for mochitest.  I was
> trying to avoid adding a new make target and then using that in the mach
> command.

I only used the make targets there out of laziness. The long term goal is to replace these make targets with mach commands because make isn't really useful there. Therefore, please don't create make targets to support a mach feature.

Comment 16

[Nathan Froyd [:froydnj]]

Comment on attachment 659314 [details] [diff] [review]
very hack patch to run talos from mach

gps already gave feedback in comment 7.  Don't cross the streams! ;)

Comment 17

[Joel Maher ( :jmaher ) (UTC -8)]

this landed a while ago, after a few bugzilla searches I am unable to find the bug where it did.