700415 - run peptest.py on try

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Aki Sasaki (not active)]

.

Comment 1

[Aki Sasaki (not active)]

From prod_config.py:

    "test_url": "url_to_packaged_tests",
    # path or url to a zip or folder containing the mozbase packages
    "mozbase_path": "url_to_mozbase_zip",
    # path or url to a zip or folder containing peptest
    "peptest_path": "url_to_peptest_zip",

    # peptest options
    "appname": "url_to_application",    # i.e the firefox build on ftp.m.o
    # defaults to firefox, can also be thunderbird, fennec, etc.
    "app": "firefox",
    # if test_url is specified, this should be the relative
    # path to the manifest inside the extracted test directory
    # otherwise, should be path to a test manifest on the local file system
    "test_manifest": "path_to_test_manifest",

I think mozbase and peptest zipfiles are not difficult to create, but they'll grow stale over time.

The test_url and appname can be set via buildbot.

What test_manifest should I be using?

Comment 2

[Aki Sasaki (not active)]

Attachment: first stab at buildbot-configs

There really isn't any space for extra unittest suite configs in our existing configs/generateTalosBranchObjects loop.

I'm going to see if I can get this to work for now before trying to slip a dict in there.

Comment 3

[Aki Sasaki (not active)]

Attachment: buildbotcustom wip

I've got MozharnessFactory which looks like it should work.
The generateTalosBranchObjects portion is a bit hairier and still needs fleshing out, not to mention the trychooser portion.

Comment 4

[Andrew Halberstadt [:ahal]]

(In reply to Aki Sasaki [:aki] from comment #1)
> I think mozbase and peptest zipfiles are not difficult to create, but
> they'll grow stale over time.
I forgot to update the example config, they can also be local zipfiles or directories (so we could just clone the repo). I can add an action for checking out the repo and doing a pull before running if you want.

> The test_url and appname can be set via buildbot.
Yep, these are also command line options which should overwrite the config file if specified.

> What test_manifest should I be using?
So we'll have to see what the test suite will look like in m-c. If you use the mock tests.zip I hosted at https://github.com/downloads/ahal/peptest/tests.zip, the manifest is located at tests/firefox/all_tests.ini

I'll try to get the tests checked into m-c and packaged into the real tests.zip asap.

Comment 5

[Andrew Halberstadt [:ahal]]

Alternatively I could also package the peptest harness in m-c, but then we'd have to decide whether to do the same for mozbase (if we check in one why not the other?). Personally I'd rather not do this.

Comment 6

[Aki Sasaki (not active)]

(In reply to Andrew Halberstadt [:ahal] from comment #4)
> (In reply to Aki Sasaki [:aki] from comment #1)
> > I think mozbase and peptest zipfiles are not difficult to create, but
> > they'll grow stale over time.
> I forgot to update the example config, they can also be local zipfiles or
> directories (so we could just clone the repo). I can add an action for
> checking out the repo and doing a pull before running if you want.

Production test slaves don't have git installed and can't reach github, so unless there's an hg.m.o mirror, we're probably using zipfiles.

> > What test_manifest should I be using?
> So we'll have to see what the test suite will look like in m-c. If you use
> the mock tests.zip I hosted at
> https://github.com/downloads/ahal/peptest/tests.zip, the manifest is located
> at tests/firefox/all_tests.ini
> 
> I'll try to get the tests checked into m-c and packaged into the real
> tests.zip asap.

ok.

Comment 7

[Aki Sasaki (not active)]

(In reply to Andrew Halberstadt [:ahal] from comment #5)
> Alternatively I could also package the peptest harness in m-c, but then we'd
> have to decide whether to do the same for mozbase (if we check in one why
> not the other?). Personally I'd rather not do this.

I'm fine either way here.

Comment 8

[Andrew Halberstadt [:ahal]]

(In reply to Aki Sasaki [:aki] from comment #7)
> I'm fine either way here.

If we did just use the zipball, how would production update to the latest revision? And how often? 

The reason I hesitate to commit the harness to m-c is because
1) I'll be updating the code on a probably daily basis (I also don't have commit access)
2) We'd also need to check in mozbase somehow

But if the zipball is going to be getting stale anyway, maybe it wouldn't be a bad idea to just do this and update the m-c repo every once in awhile.

Comment 9

[Aki Sasaki (not active)]

(In reply to Andrew Halberstadt [:ahal] from comment #8)
> (In reply to Aki Sasaki [:aki] from comment #7)
> > I'm fine either way here.
> 
> If we did just use the zipball, how would production update to the latest
> revision? And how often? 

Probably manually, every few weeks.

> The reason I hesitate to commit the harness to m-c is because
> 1) I'll be updating the code on a probably daily basis (I also don't have
> commit access)
> 2) We'd also need to check in mozbase somehow
> 
> But if the zipball is going to be getting stale anyway, maybe it wouldn't be
> a bad idea to just do this and update the m-c repo every once in awhile.

Of all the possibilities, creating an hg.m.o mirror of mozbase and peptest seems like the least painful to me.

If you want to check it into m-c and make sure it gets into the test zip, that's certainly a good way as well, though there's a significant barrier to making changes.

Comment 10

[Jeff Hammel]

(In reply to Andrew Halberstadt [:ahal] from comment #8)
> (In reply to Aki Sasaki [:aki] from comment #7)
> > I'm fine either way here.
> 
> If we did just use the zipball, how would production update to the latest
> revision? And how often? 
> 
> The reason I hesitate to commit the harness to m-c is because
> 1) I'll be updating the code on a probably daily basis (I also don't have
> commit access)
> 2) We'd also need to check in mozbase somehow
> 
> But if the zipball is going to be getting stale anyway, maybe it wouldn't be
> a bad idea to just do this and update the m-c repo every once in awhile.

I don't think avoiding putting in m-c for these reasons is necessarily the best idea.  For one, if the tests live in m-c and there is any API change to the harness, you are not guaranteed that you are running a test harness that is compatible with the tests you have in the tree.  

jetpack, AFAIK, are the only tests that pulled from a non-m-c location.  When these just pulled trunk, not only were tests broken all the time and intermittently depending on the timing of jetpack commits, it also broke bisection. The way this was fixed is there is now a file in m-c that pegs to a revision of jetpack: http://mxr.mozilla.org/mozilla-central/source/testing/jetpack/jetpack-location.txt . Now updating is done by changing this file.  Note that while, from one perspective, this is less to change in mozilla-central, you still have to change mozilla-central to follow this strategy.  IMHO, this is a small price to pay for having tests and harnesses that are concurrent with a given revision of m-c.

I think we are going to want to mirror mozbase to m-c at some point as we get more harnesses in m-c that use it.  While I'd like to start strategizing on this, and perhaps act on it soon if we can reach some easy consensus, it probably shouldn't block this work.  If the jetpack strategy is an expedient way forward, that is probably sufficient. If a script is a good mirroring strategy, that is sufficient too and I'd be more than happy to help write one (I actually have a few prototyped). But I think we should be careful before giving up concurrent tests + harnesses with m-c revision.

Comment 11

[Andrew Halberstadt [:ahal]]

So bug 700656 is closed and some example peptests should be in the tests.zip.

aki: The path to the manifest you'll want to use is 'peptest/tests/firefox/firefox-all.ini'. Let me know if anything in the configuration is unclear.

Comment 12

[Aki Sasaki (not active)]

(In reply to Andrew Halberstadt [:ahal] from comment #11)
> So bug 700656 is closed and some example peptests should be in the tests.zip.
> 
> aki: The path to the manifest you'll want to use is
> 'peptest/tests/firefox/firefox-all.ini'. Let me know if anything in the
> configuration is unclear.

Awesome, this is only blocked on my time.
I still want to get this live by EOQ, and unless I get thrown another curveball that looks doable.  I'm on PTO next week and still have native Android work to do, but it should (hopefully) not be a full time thing now.

Comment 13

[Aki Sasaki (not active)]

Attachment: unbitrotted configs

Passes test-masters.sh and setup-master.py -t (with buildbotcustom patch).

Comment 14

[Aki Sasaki (not active)]

Attachment: unbitrotted buildbotcustom

Comment 15

[Aki Sasaki (not active)]

Fired this up.
I'm not sure I like how they're named "Rev3 MacOSX Leopard 10.5.8 try mozharness test peptest" etc., and I probably need to differentiate opt from debug.

Comment 16

[Aki Sasaki (not active)]

Attachment: now part of opt_unittests

Comment 17

[Aki Sasaki (not active)]

Attachment: this works, but (custom)

I have to use buildbot factory logic to get at build.source.changes afaict.
H8.

Comment 18

[Aki Sasaki (not active)]

Attachment: (custom) use scriptfactory

Along with mozharness hacks to get things working, things are looking up.
I need to hack peptest.py to read buildprops.json for the browser/testzip urls.

Comment 19

[Aki Sasaki (not active)]

Attachment: configs

Comment 20

[Aki Sasaki (not active)]

Attachment: (custom) a lot cleaner

This, along with the corresponding configs patch and --enable-try once I'm able to consume that option, is a lot cleaner and I think gives me enough to get the rest of the logic in Mozharness.

Planning on writing a BuildbotMixin to deal with a) getting info out of buildprops.json and b) dealing with trychooser email stuff.  It'll probably grow later.

Thankful that after this first mozharness test suite, it'll be a lot easier to add mozharness test suites in the future.

Comment 21

[Aki Sasaki (not active)]

Attachment: (configs) a lot cleaner

I need to remove the user repo line before asking for r?.
WithProperties() works just fine in the extra_args, should we want to have per-branch config files, for instance; at first blush it looks like we want the same config everywhere except Try, which we can deal with in the buildbotcustom patch.  Or by editing extra_args for try specifically in buildbot-configs.

Comment 22

[Aki Sasaki (not active)]

Attachment: (mozharness) wip

TODO: BuildbotMixin, test.

Comment 23

[Aki Sasaki (not active)]

Attachment: (mozharness) first stab at BuildbotMixin

ImportError: No module named json

It's enough to make me want to only support .py config files by default in mozharness, and then we can install simplejson inside mozharness rather than having it be a prerequisite to running mozharness at all.

Not sure if this is true for all our talos boxes, but talos-r4-snow-010 doesn't have virtualenv or simplejson installed.

Comment 24

[Andrew Halberstadt [:ahal]]

Mozbase and the Peptest harness have both landed in mozilla-central:
https://hg.mozilla.org/mozilla-central/rev/b10b930500f1
https://hg.mozilla.org/mozilla-central/rev/604476c59495

So the mozharness configs should specify getting them from the packaged tests.

Comment 25

[Aki Sasaki (not active)]

(In reply to Aki Sasaki [:aki] (back nov 28) from comment #23)
> Created attachment 576893 [details] [diff] [review] [diff] [details] [review]
> (mozharness) first stab at BuildbotMixin
> 
> ImportError: No module named json

https://bugzilla.mozilla.org/show_bug.cgi?id=706244#c1 shows which pythons have json or simplejson support on our test farm.

https://bugzilla.mozilla.org/show_bug.cgi?id=706244#c3 shows where virtualenv is on our test farm.

Back in business, once I steal some time away from Android native.

Comment 26

[Aki Sasaki (not active)]

Hitting a self.extract() error:

22:44:23     INFO - #####
22:44:23     INFO - ##### Running run-peptest step.
22:44:23     INFO - #####
22:44:23     INFO - Downloading http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-linux/1323011477//firefox-11.0a1.en-US.linux-i686.tests.zip
22:44:33     INFO - mkdir: /home/cltbld/talos-slave/test/build/tests
22:44:33     INFO - Extracting /home/cltbld/talos-slave/test/firefox-11.0a1.en-US.linux-i686.tests.zip to /home/cltbld/talos-slave/test/build/tests
22:44:41     INFO - rmtree: firefox-11.0a1.en-US.linux-i686.tests.zip
22:44:41     INFO - Extract returning []
Traceback (most recent call last):
  File "scripts/scripts/peptest.py", line 360, in <module>
    peptest.run()
  File "/home/cltbld/talos-slave/test/scripts/mozharness/base/script.py", line 606, in run
    self._possibly_run_method("preflight_%s" % method_name)
  File "/home/cltbld/talos-slave/test/scripts/mozharness/base/script.py", line 578, in _possibly_run_method
    return getattr(self, method_name)()
  File "scripts/scripts/peptest.py", line 233, in preflight_run_peptest
    self.test_path = os.path.join(files[0],
IndexError: list index out of range


I'm thinking about replacing extract() here with run_command("unzip ...") for now.

Comment 27

[Aki Sasaki (not active)]

Attachment: (mozharness) latest, with broken extract()

I need to port the fixed action logic from the talosrunner branch's mozharness.base.config, or change the default_actions list in the script.

Comment 28

[Aki Sasaki (not active)]

Attachment: (configs) so far

Comment 29

[Aki Sasaki (not active)]

Attachment: (custom) so far

Comment 30

[Andrew Halberstadt [:ahal]]

Attachment: fixes broken extract

This patch updates the extract method to work with python 2.5 and also fixes the error that you ran across. It's up to you whether you want to take this patch or just shell out and call unzip.

Note that the fix from bug 708309 will still need to land before the peptest script is compatible with python 2.5

Comment 31

[Andrew Halberstadt [:ahal]]

Another thing I noticed while writing above patch is that I don't think there is a way to tell the peptest script to use the mozbase and peptest harness located inside the packaged tests. This shouldn't be hard to fix, let me know if you want me to make a new patch for this.

Comment 32

[Aki Sasaki (not active)]

01:08:32     INFO -    File "/home/cltbld/talos-slave/test/build/venv/lib/python2.6/site-packages/distribute-0.6.14-py2.6.egg/pkg_resources.py", line 1954, in load01:08:32     INFO -      entry = __import__(self.module_name, globals(),globals(), ['__name__'])01:08:32     INFO -    File "/home/cltbld/talos-slave/test/build/venv/lib/python2.6/site-packages/peptest-0.0-py2.6.egg/peptest/runpeptests.py", line 42, in <module>01:08:32     INFO -      from pepprocess import PepProcess
01:08:32     INFO -    File "/home/cltbld/talos-slave/test/build/venv/lib/python2.6/site-packages/peptest-0.0-py2.6.egg/peptest/pepprocess.py", line 39, in <module>01:08:32     INFO -      import mozlog
01:08:32     INFO -    File "/home/cltbld/talos-slave/test/build/venv/lib/python2.6/site-packages/mozlog-1.0-py2.6.egg/mozlog/__init__.py", line 36, in <module>
01:08:32     INFO -      from logger import *01:08:32     INFO -    File "/home/cltbld/talos-slave/test/build/venv/lib/python2.6/site-packages/mozlog-1.0-py2.6.egg/mozlog/logger.py", line 40, in <module>01:08:32     INFO -      _LoggerClass = getLoggerClass()
01:08:32    ERROR -  NameError: name 'getLoggerClass' is not defined

Should mozlog

  from logging import getLoggerClass

?

Comment 33

[Andrew Halberstadt [:ahal]]

I don't think so: https://github.com/mozilla/mozbase/blob/master/mozlog/mozlog/logger.py#L37

There is also no mention of getLoggerClass being new in any particular version of python: http://docs.python.org/library/logging.html#logging.getLoggerClass

Comment 34

[Andrew Halberstadt [:ahal]]

I tested with python2.6 just to be sure and it wfm...

Comment 35

[Aki Sasaki (not active)]

So mozlog logger needs |from logging import getLoggerClass, addLevelName, setLoggerClass| to run.

And then we get:

13:29:33     INFO -  PEP TEST-START | test_contextMenu.js
13:29:33    ERROR -  PEP ERROR | test_contextMenu.js | TimeoutError: controller.waitForPageLoad(): Timeout waiting for page loaded.
13:29:33     INFO -  PEP TEST-START | test_openBlankTab.js
13:29:33    ERROR -  PEP ERROR | test_openBlankTab.js | Error: could not find element ID: home
13:29:33     INFO -  PEP TEST-START | test_openBookmarksMenu.js
13:29:33    ERROR -  PEP ERROR | test_openBookmarksMenu.js | ReferenceError: pep is not defined
13:29:33     INFO -  PEP TEST-START | test_searchGoogle.js
13:29:33    ERROR -  PEP ERROR | test_searchGoogle.js | Error: could not find element ID: lst-ib
13:29:33     INFO -  PEP TEST-START | test_openWindow.js
13:29:33    ERROR -  PEP ERROR | test_openWindow.js | ReferenceError: pep is not defined
13:29:33     INFO -  PEP TEST-START | test_resizeWindow.js
13:29:33    ERROR -  PEP ERROR | test_resizeWindow.js | ReferenceError: pep is not defined
13:29:33     INFO - Return code: 0

Comment 36

[Aki Sasaki (not active)]

Per ahal, I updated the zips on build.m.o:

-rw-r--r-- 1 asasaki users 168084 Dec 13 14:33 mozilla-mozbase-74f5c2a.zip
-rw-r--r-- 1 asasaki users 138173 Dec 13 14:29 mozilla-peptest-56ee00b.zip

Still getting errors, but they look like test failures rather than harness failures?

14:54:08     INFO - Running command: ['/home/cltbld/talos-slave/test/build/venv/bin/python', '/home/cltbld/talos-slave/test/build/venv/bin/peptest', '--app', 'firefox', '--binary', '/home/cltbld/talos-slave/test/build/application/firefox/firefox', '--test-path', '/home/cltbld/talos-slave/test/build/tests/peptest/tests/firefox/firefox_all.ini', '--timeout', '60', '--tracer-threshold', '50', '--tracer-interval', '10', '--log-level', 'INFO']
14:54:51     INFO -  MOZ_EVENT_TRACE start 1323816849690
14:54:51     INFO -  MOZ_EVENT_TRACE stop 1323816849690
14:54:51     INFO -  PEP TEST-START | test_contextMenu.js
14:54:51    ERROR -  PEP ERROR      | test_contextMenu.js | TimeoutError: controller.waitForPageLoad(): Timeout waiting for page loaded.
14:54:51     INFO -  PEP TEST-START | test_openBlankTab.js
14:54:51    ERROR -  PEP ERROR      | test_openBlankTab.js | Error: could not find element ID: home
14:54:51     INFO -  PEP TEST-START | test_openBookmarksMenu.js
14:54:51  WARNING -  PEP WARNING    | test_openBookmarksMenu.js | show_all_bookmarks | unresponsive time: 54 ms
14:54:51  WARNING -  PEP WARNING    | test_openBookmarksMenu.js | show_all_bookmarks | unresponsive time: 195 ms
14:54:51    ERROR -  PEP TEST-UNEXPECTED-FAIL | test_openBookmarksMenu.js | fail threshold: 40.0 < metric: 40.941
14:54:51     INFO -  PEP TEST-END   | test_openBookmarksMenu.js | finished in: 635 ms
14:54:51     INFO -  PEP TEST-START | test_searchGoogle.js
14:54:51    ERROR -  PEP ERROR      | test_searchGoogle.js | Error: could not find element ID: lst-ib
14:54:51     INFO -  PEP TEST-START | test_openWindow.js
14:54:51     INFO -  PEP TEST-PASS  | test_openWindow.js | fail threshold: 0.0 >= metric: 0
14:54:51     INFO -  PEP TEST-END   | test_openWindow.js | finished in: 214 ms
14:54:51     INFO -  PEP TEST-START | test_resizeWindow.js
14:54:51     INFO -  PEP TEST-PASS  | test_resizeWindow.js | fail threshold: 0.0 >= metric: 0
14:54:51     INFO -  PEP TEST-END   | test_resizeWindow.js | finished in: 412 ms
14:54:51     INFO -  PEP INFO       | Test Suite Finished
14:54:51    ERROR - Return code: 1
14:54:51     INFO - /home/cltbld/talos-slave/test/build/venv/bin/python exited with return code 1: test failures

Comment 37

[Andrew Halberstadt [:ahal]]

Yes those are definitely test failures. One of them is a known failure and the other two are probably a result of network/timing issues. I wrote the tests pretty quickly and haven't had a chance to fix them up yet, but that should be tangential to this bug.

Are you going to need the changes you made to mozlog committed into m-c?

Comment 38

[Aki Sasaki (not active)]

(In reply to Andrew Halberstadt [:ahal] from comment #37)
> Yes those are definitely test failures. One of them is a known failure and
> the other two are probably a result of network/timing issues. I wrote the
> tests pretty quickly and haven't had a chance to fix them up yet, but that
> should be tangential to this bug.
> 
> Are you going to need the changes you made to mozlog committed into m-c?

Right now I have a patched mozbase zipfile, so that's working for the moment.
However, for production we should probably

a) yes, please, land that change in m-c
b) get peptest.py installing the mozbase + peptest bits from that instead of these external zipfiles that will grow stale.

Comment 39

[Andrew Halberstadt [:ahal]]

Ok..

a) is the entire change just 
"from logging import getLoggerClass, addLevelName, setLoggerClass"
?

b) I added some patches on bug 710779 to give the peptest.py script the capability of running the m-c versions of peptest/mozbase. You'll need to apply both of them (and of course if you do use m-c versions you'll run into a) again until that is landed)

Comment 40

[Aki Sasaki (not active)]

(In reply to Andrew Halberstadt [:ahal] from comment #39)
> Ok..
> 
> a) is the entire change just 
> "from logging import getLoggerClass, addLevelName, setLoggerClass"

Yup!

> b) I added some patches on bug 710779 to give the peptest.py script the
> capability of running the m-c versions of peptest/mozbase. You'll need to
> apply both of them (and of course if you do use m-c versions you'll run into
> a) again until that is landed)

Ok.
I've got a bunch of peptest changes to get things working for me; I'll layer that on top and test it, with the knowledge that I'm probably going to hit (a) again once I do so.  Thanks!

Comment 41

[Aki Sasaki (not active)]

Attachment: (configs) same, but unbitrotted

Comment 42

[Aki Sasaki (not active)]

Attachment: (custom) working log parsing (turns orange if we hit test failures)

Comment 43

[Aki Sasaki (not active)]

Attachment: (mozharness) combined working patch

This combines ahal's fixes and mine.
This works for fed64 in staging; I need to either create other test.zip files or wait until the mozlog fix lands in m-c to test the other platforms.
I'm also thinking about splitting out the install-app action, so we can --run-peptest as many times as we want in a row without reinstalling anything.

Comment 44

[Aki Sasaki (not active)]

Comment on attachment 581843 [details] [diff] [review]
(mozharness) combined working patch

Hey Andrew,

I made a ton of changes here to

a) make this work in buildbot,
b) make this a bit more mozharness-esque, and
c) some personal preference cleanup stuff.

If I'm breaking something, or if I'm changing things that you are confused about or are unhappy with, let me know.  I'm not quite done here, but we're close to having this go live; I'm hoping things are mostly kosher here for you.


>+    "buildbot_json_path": "buildprops.json",

This, and the mozharness.buildbot stuff, is for integration into our buildbot infrastructure.  I'm trying to keep this lightweight as possible.

>+    "virtualenv_modules": ["simplejson"],

I'm adding this just in case we're running python2.5, which we are in some places.

>+    "mozInstall_url": "http://people.mozilla.org/~asasaki/mozInstall-0.3.tar.gz",

You had |pip install mozinstall| which was unhappy-making in staging-with-no-internet-access.

>+    "exes": {
>+        'python': '/tools/buildbot/bin/python',
>+        'virtualenv': '/tools/misc-python/virtualenv.py',
>+    },

I'm specifying the paths to various binaries here without changing PATH.

I'll need to update the windows_config soon.

>+    def query_virtualenv_path(self):

This is so you don't have to do your os.path.join(abs_work_dir, "venv") in the config file; you can specify abs_virtualenv_dir in query_abs_dirs() as a script-level default.

>-                self.python_paths[binary] = binary
>-        return self.which(self.python_paths[binary])
>+                self.python_paths[binary] = self.query_exe(binary)
>+        return self.python_paths[binary]

Previous behavior:

  running command [None, "--option1", "--option2"]

Changed behavior:

  running command ["missing_exe", "--option1", "--option2"]

The difference is the former will be difficult to track down.
The latter will tell you that "missing_exe" is not found, which gives you something specific to debug.

>+        self.run_command([python, virtualenv, "--no-site-packages",

I added a 'python' in front of most or all python scripts, since some of these are not executable (e.g. virtualenv on the test farm, sadly).

>             all_actions=['clobber',
>+                         'create-virtualenv',
>+                         'read-buildbot-config',
>+                         'get-latest-tinderbox',
>                          'create-deps',
>-                         'get-latest-tinderbox',
>                          'run-peptest'],
>-            default_actions=['run-peptest'],
>+            default_actions=['clobber',
>+                             'create-virtualenv',
>+                             'read-buildbot-config',
>+                             'create-deps',
>+                             'run-peptest'],

Here I'm trying to make things a little more discrete and predictable.  I'm considering adding an 'install-app' action between 'create-deps' and 'run-peptest', so we can run each bit individually or skip them as wanted.  Right now if you want to re-run peptest 10 times, you recreate the virtualenv and reinstall the app 10 times.  (I know I told you to try this; my bad.)

I moved the helper apps up just so you can easily read the logic flow of the actions in one chunk of code... just a personal preference.

I'm also thinking of moving away from a multistate "self.appname is either a url of the installer, path to the installer, or the installed path"; I may split that into installer_url and app_path.  The main reason for this is to be able to install the app in --install-app and to just run the app in --run-peptest without having to figure out the new self.appname by reinstalling.

>-        self.rmtree(mozbase)

I'm leaving more cruft behind by not deleting certain directories or certain download files.  It'll all get cleaned up during a clobber, and sometimes these things are useful for debugging.  If I'm making too much of a mess, though, let me know.

Comment 45

[Aki Sasaki (not active)]

Also, http://hg.mozilla.org/users/asasaki_mozilla.com/mozharness/ if the diff is too large or hard to read.

Comment 46

[Aki Sasaki (not active)]

00:25:34     INFO - Running command: ['c:/mozilla-build/python25/python', 'c:/mozilla-build/buildbotve/virtualenv.py', '--no-site-packages', '--distribute', 'c:\\talos-slave\\test\\build\\venv']
00:25:37     INFO -  New python executable in c:\talos-slave\test\build\venv\Scripts\python.exe
00:25:37     INFO -  ERROR: The executable c:\talos-slave\test\build\venv\Scripts\python.exe is not functioning
00:25:37     INFO -  ERROR: It thinks sys.prefix is 'c:\\talos-slave\\test' (should be 'c:\\talos-slave\\test\\build\\venv')
00:25:37     INFO -  ERROR: virtualenv is not compatible with this system or executable
00:25:37     INFO -  Note: some Windows users have reported this error when they installed Python for "Only this user".  The problem may be resolvable if you install Python "For all users".  (See https://bugs.launchpad.net/virtualenv/+bug/352844)
00:25:37    ERROR - Return code: 100

:( :( :( :(
Maybe I shoulda tried windows first.

Comment 47

[Andrew Halberstadt [:ahal]]

(In reply to Aki Sasaki [:aki] from comment #46)
> :( :( :( :(
> Maybe I shoulda tried windows first.

Looking at https://bugs.launchpad.net/virtualenv/+bug/352844/comments/3, it seems that all that is missing is a pythonXX.dll. Maybe we could package these dll's with mozharness, and when we create a new virtualenv we can copy and paste them into the virtualenv/Scripts directory.

An alternative approach is to make peptest.py not depend on virtualenv (add dependencies to sys.path instead), though this doesn't really help mozharness at all.

I haven't had a chance to look at your patch yet, though will do that soon.

Comment 48

[Andrew Halberstadt [:ahal]]

Comment on attachment 581843 [details] [diff] [review]
(mozharness) combined working patch

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

Looks good. I'm fine with these changes, just a couple of comments below.

::: configs/peptest/prod_config.py
@@ +9,5 @@
>      "log_name": "pep",
> +    "buildbot_json_path": "buildprops.json",
> +    "virtualenv_modules": ["simplejson"],
> +    "simplejson_url": "http://people.mozilla.org/~asasaki/simplejson-2.2.1.tar.gz",
> +    "mozInstall_url": "http://people.mozilla.org/~asasaki/mozInstall-0.3.tar.gz",

I guess doing this (and the mozinstall one) properly depends on bug 701506?

::: mozharness/base/script.py
@@ +336,5 @@
>          if set_self_env:
>              self.env = env
>          return env
>  
> +    def query_exe(self, exe_name, exe_dict='exes'):

This seems very similar to the 'which()' method already in this file. Maybe they should be merged? (I don't particularly care what the function is called).

::: scripts/peptest.py
@@ +98,5 @@
> +            default_actions=['clobber',
> +                             'create-virtualenv',
> +                             'read-buildbot-config',
> +                             'create-deps',
> +                             'run-peptest'],

Just a little bit confused about the logic behind the defaults. Earlier you said that the build slaves wouldn't be clobbering and creating a new virtualenv and installing dependencies for each job. I agree making it predictable is nice, but won't this just slow job times needlessly? I'm also confused how/if an additional install-app action would solve this.

Comment 49

[Aki Sasaki (not active)]

(In reply to Andrew Halberstadt [:ahal] from comment #48)
> ::: configs/peptest/prod_config.py
> @@ +9,5 @@
> >      "log_name": "pep",
> > +    "buildbot_json_path": "buildprops.json",
> > +    "virtualenv_modules": ["simplejson"],
> > +    "simplejson_url": "http://people.mozilla.org/~asasaki/simplejson-2.2.1.tar.gz",
> > +    "mozInstall_url": "http://people.mozilla.org/~asasaki/mozInstall-0.3.tar.gz",
> 
> I guess doing this (and the mozinstall one) properly depends on bug 701506?

I'm not going to let it stop us from rolling it out (I'll put the tarballs on build.m.o rather than people), but yes, to do it properly we'll need an internal pypi server.

> ::: mozharness/base/script.py
> @@ +336,5 @@
> >          if set_self_env:
> >              self.env = env
> >          return env
> >  
> > +    def query_exe(self, exe_name, exe_dict='exes'):
> 
> This seems very similar to the 'which()' method already in this file. Maybe
> they should be merged? (I don't particularly care what the function is
> called).

Possibly?
which() looks in your PATH and returns None if it's not found.
query_exe() only looks in your config and returns the bare exe name if it's not found, defaulting to what's in the PATH.
I can see both being useful in different situations; I don't mind keeping them as two separate and simple methods.

> ::: scripts/peptest.py
> @@ +98,5 @@
> > +            default_actions=['clobber',
> > +                             'create-virtualenv',
> > +                             'read-buildbot-config',
> > +                             'create-deps',
> > +                             'run-peptest'],
> 
> Just a little bit confused about the logic behind the defaults. Earlier you
> said that the build slaves wouldn't be clobbering and creating a new
> virtualenv and installing dependencies for each job. I agree making it
> predictable is nice, but won't this just slow job times needlessly? I'm also
> confused how/if an additional install-app action would solve this.

On the build slaves, we have a build- and branch- specific work dir in buildbot land.  On the test slaves, we have one directory that we run all tests from.  Also, we're installing branch-specific libraries here; I'd be worried if we can install mozilla-beta peptest after installing a mozilla-central peptest the previous run (this would be a downgrade unless we land changes on all branches simultaneously).

So reusing a venv would a) assume some level of separation of jobs, and b) assume we're installing stable modules that don't change for months.

As for slowing job times, peptest was finishing under 2 minutes consistently, so I'm not too worried about that right now.

The defaults should probably be more user-friendly than buildbot-friendly, so I should remove read-buildbot-config later.  However, we're not currently able to override the default actions in the config file in build/mozharness; I have a set of patches in the github talosrunner branch that fixes this.  I was avoiding making this patch even larger, however.

So: a) splitting up actions lets users skip or add actions easily, and is less about number of actions and more about being able to specify what you want and don't want to do, and b) my concept about how virtualenv should work in mozharness in production and users' desktops is still fluid and probably not 100% matching what is optimal yet.

Comment 50

[Aki Sasaki (not active)]

(In reply to Andrew Halberstadt [:ahal] from comment #47)
> (In reply to Aki Sasaki [:aki] from comment #46)
> > :( :( :( :(
> > Maybe I shoulda tried windows first.
> 
> Looking at https://bugs.launchpad.net/virtualenv/+bug/352844/comments/3, it
> seems that all that is missing is a pythonXX.dll. Maybe we could package
> these dll's with mozharness, and when we create a new virtualenv we can copy
> and paste them into the virtualenv/Scripts directory.

Awesome! This worked:

1) mkdir -p venv\Scripts
2) copy \mozilla-build\python25\python25.dll venv\Scripts
3) wget http://people.mozilla.org/~asasaki/distribute-0.6.24.tar.gz
4) wget http://people.mozilla.org/~asasaki/pip-1.0.2.tar.gz
5) \mozilla-build\python25\python c:/talos-slave/virtualenv.py --no-site-packages --never-download --distribute c:/talos-slave/test/build/venv

So I need to add the mkdir/copy/wget steps to create_virtualenv if certain config settings are set.  I also need to add a c.get('virtualenv_options', ['--no-site-packages', '--distribute']) there.  I think I should be able to get this working in mozharness :)

Comment 51

[Aki Sasaki (not active)]

On leopard:
17:43:12     INFO - Downloading http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-macosx64/1323973575//firefox-11.0a1.en-US.mac.dmg
17:43:18     INFO - Getting output from command: ['/Users/cltbld/talos-slave/test/build/venv/bin/mozinstall', '--source', '/Users/cltbld/talos-slave/test/firefox-11.0a1.en-US.mac.dmg', '--destination', '/Users/cltbld/talos-slave/test/build/application']
17:43:37     INFO - Output received:
17:43:37     INFO -  None

(mozinstall doesn't work from buildbot; works from the commandline.)

On w7:

14:09:25     INFO -    File "C:\mozilla-build\python25\lib\tempfile.py", line 33, in <module>
14:09:25     INFO -      from random import Random as _Random
14:09:25     INFO -    File "C:\mozilla-build\python25\lib\random.py", line 838, in <module>
14:09:25     INFO -      _inst = Random()
14:09:25     INFO -    File "C:\mozilla-build\python25\lib\random.py", line 94, in __init__
14:09:25     INFO -      self.seed(x)
14:09:25     INFO -    File "C:\mozilla-build\python25\lib\random.py", line 108, in seed
14:09:25     INFO -      a = long(_hexlify(_urandom(16)), 16)
14:09:25     INFO -  WindowsError: [Error 22] Invalid Signature

(Random error when creating the mozprofile.)

Things aren't looking so hot right now.

Comment 52

[Aki Sasaki (not active)]

Leopard: mozinstall works if I try it twice, and then I hit a dyld error:

18:15:08     INFO - Downloading http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-macosx64/1323973575//firefox-11.0a1.en-US.mac.dmg
18:15:13     INFO - Running command: ['/Users/cltbld/talos-slave/test/build/venv/bin/mozinstall', '--source', '/Users/cltbld/talos-slave/test/firefox-11.0a1.en-US.mac.dmg', '--destination', '/Users/cltbld/talos-slave/test/build/application']
18:15:32     INFO -  None
18:15:32     INFO - Return code: 0
18:15:32     INFO - Getting output from command: ['/Users/cltbld/talos-slave/test/build/venv/bin/mozinstall', '--source', '/Users/cltbld/talos-slave/test/firefox-11.0a1.en-US.mac.dmg', '--destination', '/Users/cltbld/talos-slave/test/build/application']
18:15:45     INFO - Output received:
18:15:45     INFO -  /Users/cltbld/talos-slave/test/build/application/Nightly.app/Contents/MacOS/firefox
18:15:45     INFO - rmtree: tmpfile_stderr
18:15:45     INFO - rmtree: tmpfile_stdout
18:15:45     INFO - Running command: ['/Users/cltbld/talos-slave/test/build/venv/bin/peptest', '--app', 'firefox', '--binary', '/Users/cltbld/talos-slave/test/build/application/Nightly.app/Contents/MacOS/firefox', '--test-path', '/Users/cltbld/talos-slave/test/build/tests/peptest/tests/firefox/firefox_all.ini', '--timeout', '60', '--tracer-threshold', '50', '--tracer-interval', '10', '--log-level', 'INFO']
18:15:47     INFO -  dyld: unknown required load command 0x80000022

Comment 53

[Aki Sasaki (not active)]

w7 and xp: I've stopped hitting the WindowsError and just end up hanging and timing out after 1200 seconds with no output during the peptest run.

Right now it looks like if we roll out, it'll be linux only =\

Comment 54

[Andrew Halberstadt [:ahal]]

It sounds like you had a rough day yesterday :/

Let me know if there's anything I can do, I wouldn't mind helping debug if I can get build vpn access. Otherwise there's not much I can do without reproducing.

Comment 55

[Aki Sasaki (not active)]

I'm able to reproduce the osx errors on my laptop.
On Windows I hit other errors before hitting the hanging issue when running manually (either on my xp vm or the win7 talos box).

Comment 56

[Aki Sasaki (not active)]

The blocking bugs were fixed while I was on PTO.
I re-ran peptest on staging on snow leopard, lion, xp, and fed64 and while each of them hit unexpected failures inside of peptest, they each finished (orange, not red or purple).  Once it's on Try, developers or the a-team can push patches to Try to get this green. 

This is ready to go for Try, once I clean up the patches and get review.  Thanks everyone!

Comment 57

[Andrew Halberstadt [:ahal]]

Those tests were mostly meant as examples.  Most of them don't test anything that is _actually_ useful in terms of catching responsiveness anyway.  I'll fix the ones that are broken and remove the examples from the manifest.

I know mcote has a handful of nice actually useful tests that should be checked in and added to the manifest.

Comment 58

[Mark Côté [:mcote]]

Well my tests are actually verifying that there *are* problems with responsiveness, for example, when loading a large image and switching to another tab and back again.  As far as I can tell none of these have been fixed yet, so if the goal here is to get green results, adding these tests would be counterproductive. :)  I *did* attach some of these tests to bugs, however, so they (or possibly slightly modified versions of them) should be added if/when those bugs are fixed.

ahal's basic tests (at least those that pass) should be kept in there, just because something would be really broken if they started failing.  A basic smoke test, if you will.

I think going forward these should be real regression tests, meaning that when someone believes they have fixed a problem with responsiveness, they should verify it by adding a peptest, thereby also ensuring that the problem remains fixed in future versions.  I don't think there's much value in adding lots of simplistic tests, aside from the ones ahal already added (which also serve as good, if basic, examples for future test creation).

Comment 59

[Aki Sasaki (not active)]

I think:

a) having a green baseline would be good, so we do catch regressions and don't just ignore peptest as perma-orange;
b) having tests that are known current failures that we want to fix would be good, but we should have a way to not turn orange on those before they're fixed (separate manifests? marked as a known failure?); and
c) both of the above are outside the scope of this bug.  Expanding on my comment 56, developers and the a-team can edit the manifest and test scripts and iterate as needed once peptest is on try.

Comment 60

[Andrew Halberstadt [:ahal]]

There's already a way to mark a test as a known failure.  In the manifest you can specify a failThreshold = xxx for a particular test.

For example if Mark wrote a test called test1.js that usually has a metric of around 100, the manifest might look like:

[test1.js]
failThreshold = 150

This way it will only turn orange if it is even _more_ unresponsive than normal.  The downside is that it becomes easy to overlook when a test case actually is fixed for good.

Comment 61

[Aki Sasaki (not active)]

Attachment: (tools) update trychooser

Lukas:  I'd love to have this *off* by default, but the "all" option kind of goes against that.  Is this good, or is there a way to leave peptest off by default?

Comment 62

[Aki Sasaki (not active)]

Attachment: (configs) peptest on try

Comment 63

[Aki Sasaki (not active)]

Attachment: (custom) peptest on try

Comment 64

[Aki Sasaki (not active)]

Attachment: (mozharness) peptest on try

This is a fairly large (58K, 1447 line) patch.

History is in http://hg.mozilla.org/users/asasaki_mozilla.com/mozharness

I've tested across fedora64, snow leopard, lion, win7, and xp, and previously tested with leopard and fedora.

I'm happy to write up a long spiel about what I've done, or go over the code with you, or whatever path makes this review easier.

I'm also open to adding other reviewers if certain portions need more coverage.

Comment 65

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 587945 [details] [diff] [review]
(tools) update trychooser

This will work for including it as part of 'all' or as a custom selected test.  If we want to be able to keep certain test suites out of 'all' then we might want to look into having a config variable in the try configs that lets us list names of tests we don't want to fall under that umbrella, pass them in here:
http://hg.mozilla.org/build/buildbotcustom/file/tip/misc.py#l851
Then filter them out in try_parser here: http://hg.mozilla.org/build/buildbotcustom/file/0a3b81093407/try_parser.py#l167

Sorry I don't have time at the moment to write and test this idea but bug 691177 will keep this on my radar.

Comment 66

[John Ford [:jhford] CET/CEST Berlin Time]

Comment on attachment 587965 [details] [diff] [review]
(custom) peptest on try

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

r+.  I'd like to see the hg variable renamed, but its your choice.

::: process/factory.py
@@ +7993,5 @@
>  class ScriptFactory(BuildFactory):
>      def __init__(self, scriptRepo, scriptName, cwd=None, interpreter=None,
>              extra_data=None, extra_args=None,
> +            script_timeout=1200, script_maxtime=None, log_eval_func=None,
> +            hg='hg'):

using the var hg might become confusing, especially when looking at the argv lists.  what about calling it 'hg_bin'?

Comment 67

[John Ford [:jhford] CET/CEST Berlin Time]

Comment on attachment 587964 [details] [diff] [review]
(configs) peptest on try

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

::: mozilla-tests/config.py
@@ +995,5 @@
> +for branch in ('try',  ):
> +    for pf in PLATFORMS:
> +        if 'android' in pf:
> +            continue
> +        hg = 'hg'

like in the buildbotcustom patch, I think this var should be named 'hg_bin'.  It makes working with this section of code less confusing.

Comment 68

[Jeff Hammel]

Comment on attachment 587966 [details] [diff] [review]
(mozharness) peptest on try

+        # https://bugs.launchpad.net/virtualenv/+bug/352844/comments/3
+        # https://bugzilla.mozilla.org/show_bug.cgi?id=700415#c50

Is this ticketed at https://github.com/pypa/virtualenv?  If not, it probably should be since I don't think the bitbucket issues are triaged or looked at

             if zipfile.is_zipfile(path):
                 bundle = zipfile.ZipFile(path)
                 namelist = bundle.namelist()
+                if hasattr(bundle, 'extractall'):
+                    bundle.extractall(path=extdir)
+                # zipfile.extractall doesn't exist in Python 2.5
+                else:
+                    for name in namelist:
+                        filename = os.path.realpath(os.path.join(extdir, name))
+                        if name.endswith("/"):
+                            os.makedirs(filename)
+                        else:
+                            path = os.path.dirname(filename)
+                            if not os.path.isdir(path):
+                                os.makedirs(path)
+                            dest = open(filename, "wb")
+                            dest.write(bundle.read(name))

This should probably go to a utility function

+        if config:
+            self.config = config
+        else:
+            self.config = {}

FWIW, I prefer self.config = config or {}

+# load modules from parent dir
+sys.path.insert(1, os.path.dirname(sys.path[0]))
Not a huge fan of this pattern

+#        env = self.query_env(partial_env={"PYTHONUNBUFFERED": "1"})
Please remove

Looks good!

Comment 69

[Andrew Halberstadt [:ahal]]

Comment on attachment 587966 [details] [diff] [review]
(mozharness) peptest on try

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

I didn't pour over every line of code in detail, but from what I've seen from the diffs it looks good to me. r=ahal

::: scripts/peptest.py
@@ +39,5 @@
> +import os
> +import sys
> +
> +# load modules from parent dir
> +sys.path.insert(1, os.path.dirname(sys.path[0]))

Is this a shortcut so that we don't have to have mozharness installed in the package index? If so you might want to use "os.path.dirname(os.path.realpath(__file__))" instead so it doesn't depend on where you run the script from.

@@ +103,5 @@
>              require_config_file=require_config_file,
>              config={'dependencies': ['mozlog',
>                                       'mozinfo',
>                                       'mozhttpd',
> +                                     'mozinstall',

Peptest doesn't depend on mozinstall (though this peptest.py script does use it). I can't really remember if these are dependencies for peptest itself, or the peptest.py script, but I guess either way there isn't really a problem

Comment 70

[Aki Sasaki (not active)]

(In reply to Jeff Hammel [:jhammel] from comment #68)
> Comment on attachment 587966 [details] [diff] [review]
> (mozharness) peptest on try
> 
> +        # https://bugs.launchpad.net/virtualenv/+bug/352844/comments/3
> +        # https://bugzilla.mozilla.org/show_bug.cgi?id=700415#c50
> 
> Is this ticketed at https://github.com/pypa/virtualenv?  If not, it probably
> should be since I don't think the bitbucket issues are triaged or looked at

https://bugs.launchpad.net/virtualenv/+bug/352844/comments/1 suggests that we need to reinstall python on the system.


>              if zipfile.is_zipfile(path):
>                  bundle = zipfile.ZipFile(path)
>                  namelist = bundle.namelist()
> +                if hasattr(bundle, 'extractall'):
> +                    bundle.extractall(path=extdir)
> +                # zipfile.extractall doesn't exist in Python 2.5
> +                else:
> +                    for name in namelist:
> +                        filename = os.path.realpath(os.path.join(extdir,
> name))
> +                        if name.endswith("/"):
> +                            os.makedirs(filename)
> +                        else:
> +                            path = os.path.dirname(filename)
> +                            if not os.path.isdir(path):
> +                                os.makedirs(path)
> +                            dest = open(filename, "wb")
> +                            dest.write(bundle.read(name))
> 
> This should probably go to a utility function

We're not actually using extract() right now since we kept running into issues where it was holding up testing due to borkage.  I'm open to refactoring it, but if unzip continues to be a more reliable option than extract(), we should remove extract() altogether.
 
> +        if config:
> +            self.config = config
> +        else:
> +            self.config = {}
> 
> FWIW, I prefer self.config = config or {}

Good to know; I may try that.
I tend to prefer more verbose and clear code even if it adds lines, but that seems clear enough.

> +# load modules from parent dir
> +sys.path.insert(1, os.path.dirname(sys.path[0]))
> Not a huge fan of this pattern

Yeah, I want to keep the ability to launch mozharness without setting up a virtualenv though. bug 611791 tracks getting rid of this at some point, but I'm not sure when that will happen.

> +#        env = self.query_env(partial_env={"PYTHONUNBUFFERED": "1"})
> Please remove

Done.

(In reply to Andrew Halberstadt [:ahal] from comment #69)
> ::: scripts/peptest.py
> @@ +39,5 @@
> > +import os
> > +import sys
> > +
> > +# load modules from parent dir
> > +sys.path.insert(1, os.path.dirname(sys.path[0]))
> 
> Is this a shortcut so that we don't have to have mozharness installed in the
> package index? If so you might want to use
> "os.path.dirname(os.path.realpath(__file__))" instead so it doesn't depend
> on where you run the script from.

Yes.
I think I tested the above line and I can run the script from anywhere.
I added your comment to bug 611791 comment 10, since I pictured making your change without testing and then failing on one of the more obtuse platforms.
 
> @@ +103,5 @@
> >              require_config_file=require_config_file,
> >              config={'dependencies': ['mozlog',
> >                                       'mozinfo',
> >                                       'mozhttpd',
> > +                                     'mozinstall',
> 
> Peptest doesn't depend on mozinstall (though this peptest.py script does use
> it). I can't really remember if these are dependencies for peptest itself,
> or the peptest.py script, but I guess either way there isn't really a problem

It's very fast to install mozinstall, and previously there was a lot of code dedicated to conditionally installing mozinstall after creating the virtualenv.  I decided to simplify and just install it no matter what.

Comment 71

[Aki Sasaki (not active)]

Comment on attachment 587966 [details] [diff] [review]
(mozharness) peptest on try

http://hg.mozilla.org/build/mozharness/rev/dadcba16f19e

Comment 72

[Aki Sasaki (not active)]

Comment on attachment 587945 [details] [diff] [review]
(tools) update trychooser

http://hg.mozilla.org/build/tools/rev/989e0139dd8b

Comment 73

[Aki Sasaki (not active)]

Comment on attachment 587964 [details] [diff] [review]
(configs) peptest on try

http://hg.mozilla.org/build/buildbot-configs/rev/bf5aac7791a2
with hg -> hg_bin

Comment 74

[Aki Sasaki (not active)]

Comment on attachment 587965 [details] [diff] [review]
(custom) peptest on try

http://hg.mozilla.org/build/buildbotcustom/rev/9762618ce570

Comment 75

[Aki Sasaki (not active)]

Live.