Closed Bug 516984 Opened 15 years ago Closed 14 years ago

Run mozmill tests on buildbot

Categories

(Testing Graveyard :: Mozmill, defect, P3)

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: catlee, Unassigned)

References

Details

Attachments

(8 files, 27 obsolete files)

9.22 KB, patch
ted
: review+
Details | Diff | Splinter Review
832 bytes, patch
ted
: review+
Details | Diff | Splinter Review
605 bytes, patch
mozilla
: review+
Details | Diff | Splinter Review
790 bytes, patch
cmtalbert
: review+
Details | Diff | Splinter Review
7.15 KB, patch
whimboo
: review+
Details | Diff | Splinter Review
7.09 KB, patch
k0scist
: review+
Details | Diff | Splinter Review
704 bytes, patch
armenzg
: review+
Details | Diff | Splinter Review
485 bytes, patch
mozilla
: review+
Details | Diff | Splinter Review
We should be running mozmill tests on our regular depend builds.

Clint, any need for them to be run on debug builds as well?
Priority: -- → P3
Somehow missed this bug.  Sorry.  No need for them to be run on debug builds at the moment, no.
Whiteboard: Q4 goal
Latest version of mozmill still has issues with hangs post running.

I think we need to set up testdev with their own automation systems so they can work on debugging these issues.

Al mentioned that there is already, or will be soon, a bug for that.
Assignee: catlee → nobody
Component: Release Engineering → Release Engineering: Future
Whiteboard: Q4 goal
Mass move of bugs from Release Engineering:Future -> Release Engineering. See
http://coop.deadsquid.com/2010/02/kiss-the-future-goodbye/ for more details.
Component: Release Engineering: Future → Release Engineering
Attached patch WIP (obsolete) — Splinter Review
Assignee: nobody → jhammel
functional cleanup of the patch to integrate mozmill and buildbot.  All of the steps actually run, though mozmill does not work successfully yet on the nightly builds (see https://wiki.mozilla.org/Auto-tools/Projects/MozmillBuildbot#Outstanding_Issues )

virtualenv is used to install mozmill.  Currently, the pypi version is used (http://pypi.python.org/pypi/mozmill ; v1.4) at the time of this writing.

The patch includes details that aren't important to the work, so should be cleaned up.  The previous method of installing mozmill is still present, but commented out

An alternative is to not use virtualenv. To be determined.
Attachment #434790 - Attachment is obsolete: true
(In reply to comment #6)
> Created an attachment (id=437934) [details]
> new version of mozmill + buildbot patch, unclean
> 
> functional cleanup of the patch to integrate mozmill and buildbot.  All of the
> steps actually run, though mozmill does not work successfully yet on the
> nightly builds (see
> https://wiki.mozilla.org/Auto-tools/Projects/MozmillBuildbot#Outstanding_Issues
> )
> ...

Issue moved here:

Running mozmill does not work as expected.  This is shown in the buildbot step as well as reproducible from the command line following the build.

 > /home/jhammel/mozilla/src/buildbot/slave/full/mozmill/bin/mozmill --showall -b firefox/firefox -t mozmill-tests/firefox --showall
 (firefox-bin:13024): GLib-WARNING **: g_set_prgname() called multiple times
 (firefox-bin:13024): GLib-WARNING **: g_set_prgname() called multiple times
 Traceback (most recent call last):
  File "/home/jhammel/mozilla/src/buildbot/slave/full/mozmill/bin/mozmill", line 8, in <module>
    load_entry_point('mozmill==1.4', 'console_scripts', 'mozmill')()
  File "/home/jhammel/mozilla/src/buildbot/slave/full/mozmill/lib/python2.6/site-packages/mozmill-1.4-py2.6.egg/mozmill/__init__.py", line 541, in cli
    CLI().run()
  File "/home/jhammel/mozilla/src/buildbot/slave/full/mozmill/lib/python2.6/site-packages/mozmill-1.4-py2.6.egg/mozmill/__init__.py", line 518, in run
    self._run()
  File "/home/jhammel/mozilla/src/buildbot/slave/full/mozmill/lib/python2.6/site-packages/mozmill-1.4-py2.6.egg/mozmill/__init__.py", line 468, in _run
    self.mozmill.start(runner=runner, profile=runner.profile)
  File "/home/jhammel/mozilla/src/buildbot/slave/full/mozmill/lib/python2.6/site-packages/mozmill-1.4-py2.6.egg/mozmill/__init__.py", line 162, in start
    self.create_network()
  File "/home/jhammel/mozilla/src/buildbot/slave/full/mozmill/lib/python2.6/site-packages/mozmill-1.4-py2.6.egg/mozmill/__init__.py", line 142, in create_network
    self.jsbridge_port)
  File "/home/jhammel/mozilla/src/buildbot/slave/full/mozmill/lib/python2.6/site-packages/jsbridge-2.3.4-py2.6.egg/jsbridge/__init__.py", line 72, in wait_and_create_network
    raise Exception("Sorry, cannot connect to jsbridge extension")
 Exception: Sorry, cannot connect to jsbridge extension

The command (<tt>mozmill -b /home/jhammel/bin/firefox -t /home/jhammel/mozilla/src/buildbot/slave/full/build/mozmill-tests/firefox --showall</tt>) works on Firefox 3.6.2 but not on on 3.7.a{4,5}pre.

--

This is due to the maxVersion tags in the extesions/install.rdf file being set to 3.7a1pre in the versions of both mozmill (1.4) and jsbridge (2.3.4) on pypi at this date (thanks, Clint!).  Changing these causes the test to run correctly.  So the patch should work when next week's upcoming release of these packages are posted on pypi, though the patch should be cleaned up first.
i am having issues untarring firefox bug this seems an orthogonal issue;  future patches will be at http://k0s.org/mozilla/hg/buildbotcustom-patches/ (though I'll repost "publishable" patches here)
Attachment #437934 - Attachment is obsolete: true
The untarring bug is a bit confusing.  Running the identical tar command by hand works fine, and the firefox untarred by buildbot is runnable.  Nonetheless, I get the following error via buildbot:

tar -jxvf firefox-3.7a5pre.en-US.linux-i686.tar.bz2
 in dir /home/jhammel/mozilla/src/buildbot/slave/full/build (timeout 1200 secs)
 watching logfiles {}
 argv: ['tar', '-jxvf', 'firefox-3.7a5pre.en-US.linux-i686.tar.bz2']
 environment:
  CLICOLOR=1
  COLORTERM=gnome-terminal
  DBUS_SESSION_BUS_ADDRESS=unix:abstract=/tmp/dbus-pHtvBMzkgw,guid=0f0ede9b8ae68f81472492e44bbe8fe6
  DESKTOP_SESSION=fluxbox
  DISPLAY=:0.0
  EDITOR=emacs -nw
  GDMSESSION=fluxbox
  GDM_KEYBOARD_LAYOUT=us
  GDM_LANG=en_US.UTF-8
  GNOME_KEYRING_PID=2063
  GNOME_KEYRING_SOCKET=/tmp/keyring-ReC1KQ/socket
  GTK_MODULES=canberra-gtk-module
  HOME=/home/jhammel
  LANG=en_US.UTF-8
  LOGNAME=jhammel
  OLDPWD=/home/jhammel/mozilla
  ORBIT_SOCKETDIR=/tmp/orbit-jhammel
  PATH=/home/jhammel/mozilla/src/buildbot/bin:/home/jhammel/bin:/home/jhammel/python:/home/jhammel/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/sbin:/usr/games/bin
  PS1=(buildbot)> 
  PWD=/home/jhammel/mozilla/src/buildbot/slave/full/build
  PYTHONPATH=/home/jhammel/mozilla/src/buildbot/buildbotcustom:/home/jhammel/python:
  SHELL=/bin/bash
  SHLVL=2
  SPEECHD_PORT=7560
  SSH_AGENT_PID=2119
  SSH_AUTH_SOCK=/tmp/ssh-IZegHg2078/agent.2078
  TERM=xterm
  USER=jhammel
  USERNAME=jhammel
  VIRTUAL_ENV=/home/jhammel/mozilla/src/buildbot
  WINDOWID=4194503
  XAUTHORITY=/var/run/gdm/auth-for-jhammel-CrlBQJ/database
  XDG_SESSION_COOKIE=12dac5c6da5380afaa9b27384b9dd707-1270779878.616093-1316947122
  _=/home/jhammel/mozilla/src/buildbot/bin/buildbot
 closing stdin
 using PTY: False
tar: Record size = 8 blocks
firefox/
firefox/mozilla-xremote-client
firefox/platform.ini
firefox/README.txt
firefox/libsoftokn3.chk
firefox/libnssckbi.so
firefox/libxul.so
firefox/crashreporter-override.ini
firefox/libxpcom.so
firefox/libmozsqlite3.so
firefox/crashreporter.ini
firefox/icons/
firefox/icons/updater.png
firefox/icons/mozicon128.png
firefox/icons/document.png
firefox/libplds4.so
firefox/extensions/
firefox/extensions/{972ce4c6-7e08-4474-a285-3208198ce6fd}/
firefox/extensions/{972ce4c6-7e08-4474-a285-3208198ce6fd}/preview.png
firefox/extensions/{972ce4c6-7e08-4474-a285-3208198ce6fd}/icon.png
firefox/extensions/{972ce4c6-7e08-4474-a285-3208198ce6fd}/install.rdf
firefox/modules/
firefox/modules/LightweightThemeManager.jsm
firefox/modules/distribution.js
firefox/modules/Microformats.js
firefox/modules/debug.js
firefox/modules/XPCOMUtils.jsm
firefox/modules/InlineSpellChecker.jsm
firefox/modules/NetworkPrioritizer.jsm
firefox/modules/CrashSubmit.jsm
firefox/modules/LightweightThemeConsumer.jsm
firefox/modules/CSPUtils.jsm
firefox/modules/Services.jsm
firefox/modules/FileUtils.jsm
firefox/modules/utils.js
firefox/modules/DownloadLastDir.jsm
firefox/modules/PluralForm.jsm
firefox/modules/PlacesDBUtils.jsm
firefox/modules/NetUtil.jsm
firefox/modules/openLocationLastURL.jsm
firefox/modules/SpatialNavigation.js
firefox/modules/WindowDraggingUtils.jsm
firefox/modules/CertUtils.jsm
firefox/modules/ctypes.jsm
firefox/modules/DownloadUtils.jsm
firefox/modules/ISO8601DateUtils.jsm
firefox/application.ini
firefox/Throbber-small.gif
firefox/chrome/
firefox/chrome/browser.manifest
firefox/chrome/toolkit.jar
firefox/chrome/pippki.manifest
firefox/chrome/icons/
firefox/chrome/icons/default/
firefox/chrome/icons/default/default16.png
firefox/chrome/icons/default/default32.png
firefox/chrome/icons/default/default48.png
firefox/chrome/en-US.jar
firefox/chrome/toolkit.manifest
firefox/chrome/en-US.manifest
firefox/chrome/browser.jar
firefox/chrome/pippki.jar
firefox/update.locale
firefox/libsmime3.so
firefox/libmozjs.so
firefox/firefox-bin
firefox/dictionaries/
firefox/dictionaries/en-US.dic
firefox/dictionaries/en-US.aff
firefox/res/
firefox/res/language.properties
firefox/res/langGroups.properties
firefox/res/table-add-column-after-active.gif
firefox/res/dtd/
firefox/res/dtd/xhtml11.dtd
firefox/res/dtd/mathml.dtd
firefox/res/table-add-column-before-active.gif
firefox/res/table-remove-row-active.gif
firefox/res/table-remove-row.gif
firefox/res/table-add-row-after.gif
firefox/res/table-remove-column.gif
firefox/res/svg.css
firefox/res/table-add-column-after-hover.gif
firefox/res/fonts/
firefox/res/fonts/mathfont.properties
firefox/res/fonts/mathfontStandardSymbolsL.properties
firefox/res/fonts/mathfontUnicode.properties
firefox/res/fonts/mathfontSTIXNonUnicode.properties
firefox/res/fonts/mathfontSTIXSize1.properties
firefox/res/table-remove-column-active.gif
firefox/res/table-add-column-after.gif
firefox/res/grabber.gif
firefox/res/table-add-row-before-active.gif
firefox/res/table-add-row-after-hover.gif
firefox/res/table-add-column-before.gif
firefox/res/table-add-column-before-hover.gif
firefox/res/designmode.css
firefox/res/table-add-row-before.gif
firefox/res/table-remove-column-hover.gif
firefox/res/contenteditable.css
firefox/res/table-add-row-before-hover.gif
firefox/res/table-remove-row-hover.gif
firefox/res/EditorOverride.css
firefox/res/entityTables/
firefox/res/entityTables/html40Special.properties
firefox/res/entityTables/mathml20.properties
firefox/res/entityTables/htmlEntityVersions.properties
firefox/res/entityTables/html40Symbols.properties
firefox/res/entityTables/transliterate.properties
firefox/res/entityTables/html40Latin1.properties
firefox/res/html/
firefox/res/html/folder.png
firefox/res/table-add-row-after-active.gif
firefox/dependentlibs.list
firefox/defaults/
firefox/defaults/pref/
firefox/defaults/pref/reporter.js
firefox/defaults/pref/firefox.js
firefox/defaults/pref/channel-prefs.js
firefox/defaults/pref/firefox-l10n.js
firefox/defaults/pref/firefox-branding.js
firefox/defaults/profile/
firefox/defaults/profile/bookmarks.html
firefox/defaults/profile/chrome/
firefox/defaults/profile/chrome/userContent-example.css
firefox/defaults/profile/chrome/userChrome-example.css
firefox/defaults/profile/mimeTypes.rdf
firefox/defaults/profile/prefs.js
firefox/defaults/profile/localstore.rdf
firefox/defaults/autoconfig/
firefox/defaults/autoconfig/prefcalls.js
firefox/defaults/autoconfig/platform.js
firefox/libnssdbm3.chk
firefox/blocklist.xml
firefox/libnssutil3.so
firefox/libnssdbm3.so
firefox/updater
firefox/run-mozilla.sh
firefox/components/
firefox/components/nsPlacesTransactionsService.js
firefox/components/nsSafebrowsingApplication.js
firefox/components/nsAddonRepository.js
firefox/components/nsLoginManager.js
firefox/components/nsTaggingService.js
firefox/components/nsBrowserContentHandler.js
firefox/components/nsUrlClassifierListManager.js
firefox/components/nsDefaultCLH.js
firefox/components/nsLivemarkService.js
firefox/components/nsProxyAutoConfig.js
firefox/components/nsSearchSuggestions.js
firefox/components/fuelApplication.js
firefox/components/nsExtensionManager.js
firefox/components/FeedProcessor.js
firefox/components/nsLoginInfo.js
firefox/components/nsBadCertHandler.js
firefox/components/browser.xpt
firefox/components/storage-mozStorage.js
firefox/components/NetworkGeolocationProvider.js
firefox/components/libdbusservice.so
firefox/components/nsDownloadManagerUI.js
firefox/components/nsPrivateBrowsingService.js
firefox/components/crypto-SDR.js
firefox/components/nsPlacesAutoComplete.js
firefox/components/nsContentDispatchChooser.js
firefox/components/nsUrlClassifierLib.js
firefox/components/nsBrowserGlue.js
firefox/components/nsINIProcessor.js
firefox/components/nsTryToClose.js
firefox/components/PlacesProtocolHandler.js
firefox/components/FeedWriter.js
firefox/components/nsUpdateServiceStub.js
firefox/components/nsHelperAppDlg.js
firefox/components/pluginGlue.js
firefox/components/nsMicrosummaryService.js
firefox/components/libmozgnome.so
firefox/components/nsSetDefaultBrowser.js
firefox/components/WebContentConverter.js
firefox/components/nsPlacesDBFlush.js
firefox/components/nsWebHandlerApp.js
firefox/components/nsContentPrefService.js
firefox/components/nsPlacesExpiration.js
firefox/components/nsUpdateTimerManager.js
firefox/components/libbrowsercomps.so
firefox/components/nsSessionStartup.js
firefox/components/jsconsole-clhandler.js
firefox/components/nsFormAutoComplete.js
firefox/components/contentSecurityPolicy.js
firefox/components/nsFilePicker.js
firefox/components/nsURLFormatter.js
firefox/components/GPSDGeolocationProvider.js
firefox/components/txEXSLTRegExFunctions.js
firefox/components/nsHandlerService.js
firefox/components/nsBlocklistService.js
firefox/components/nsLoginManagerPrompter.js
firefox/components/nsUpdateService.js
firefox/components/nsSearchService.js
firefox/components/components.list
firefox/components/storage-Legacy.js
firefox/components/FeedConverter.js
firefox/components/nsSidebar.js
firefox/components/nsSessionStore.js
firefox/components/libnkgnomevfs.so
firefox/libplc4.so
firefox/plugin-container
firefox/updater.ini
firefox/libnspr4.so
firefox/plugins/
firefox/plugins/libnullplugin.so
firefox/browserconfig.properties
firefox/removed-files
firefox/searchplugins/
firefox/searchplugins/creativecommons.xml
firefox/searchplugins/wikipedia.xml
firefox/searchplugins/amazondotcom.xml
firefox/searchplugins/yahoo.xml
firefox/searchplugins/google.xml
firefox/searchplugins/eBay.xml
firefox/searchplugins/answers.xml
firefox/libsoftokn3.so
firefox/libmozalloc.so
firefox/firefox
firefox/libfreebl3.chk
firefox/libfreebl3.so
firefox/libssl3.so
firefox/greprefs.js
firefox/libnss3.so
firefox/crashreporter

bzip2: I/O or other error, bailing out.  Possible reason follows.
bzip2: Broken pipe
	Input file = (stdin), output file = (stdout)
tar: Child returned status 1
tar: Exiting with failure status due to previous errors
program finished with exit code 2
elapsedTime=2.013660

Will investigate
(In reply to comment #9)
> The untarring bug is a bit confusing.  Running the identical tar command by
> hand works fine, and the firefox untarred by buildbot is runnable. 
> Nonetheless, I get the following error via buildbot:
> 
> tar -jxvf firefox-3.7a5pre.en-US.linux-i686.tar.bz2
> <snip/>
> 
> Will investigate

This is currently working, with no change in code.  Unsure if this problem is serious.
Does it mean we can dupe bug 457265 against this bug now?
(In reply to comment #11)
> Does it mean we can dupe bug 457265 against this bug now?

Yep, I think so.
To make the factory more extensible, I'd suggest refactoring everything firefox-specific into separate methods, which can be overridden by a CCMozmillFactory for comm-central code. This way we don't need to copy all of the method code.

Examples: finding the firefox executable, setting the test dirs (this could be a class variable maybe), ...
Blocks: 458352
As a note, Thunderbird already has something set up for running mozmill tests in automation, see bug 500142, etc. - parts from they have been doing there might be helpful for the work being done here (and in the end, it would be nice if all our products run the same infrastructure for this).
(In reply to comment #14)
> To make the factory more extensible, I'd suggest refactoring everything
> firefox-specific into separate methods, which can be overridden by a
> CCMozmillFactory for comm-central code. This way we don't need to copy all of
> the method code.

That's one thing to think of - ideally, I think this should be fully integrated in the PackagedUnittestFactory though, so we can just run it by adding a 'mozmill' item to the generic test suites list.
Comment on attachment 438468 [details] [diff] [review]
clean version of mozmill+buildbot integration using virtualenv (semi-tested)

>+        # Checkout mozmill-tests
>+        self.addStep(ShellCommand(
>+         command=['hg', 'clone', 'http://hg.mozilla.org/qa/mozmill-tests'],
>+         haltOnFailure=True,
>+         name="clone_mozmill-tests",
>+        ))

If you only do this step you will get the default branch which contain tests against the most recent version of Firefox. Regarding the Firefox branch we are testing on you will have to switch the branch of the test repository:

https://developer.mozilla.org/en/Mozmill_Tests#Handling_Branches

We will branch for Firefox 3.7 soon.

>+        # Run those tests!
>+        # XXX should be separated into two steps, as the two need
>+        # to be run differently
>+        tests_dirs = [
>+                'mozmill-tests/firefox',
>+                'mozmill-tests/firefox/restartTests',
>+                ]
>+        for tests_dir in tests_dirs:
>+            self.addStep(unittest_steps.MozmillStep(
>+             name="run_mozmill",
>+             tests_dir=tests_dir,
>+             env=self.env,
>+             timeout=5*60,
>+            ))

Do you differentiate between the 'mozmill' and 'mozmill-restart' applications?

>+class MozmillStep(ShellCommandReportTimeout):
[...]
>+            WithProperties('%(mozmillpath)s'),
>+            '--showall',

Do we really need '--showall' or would '--show-errors' be enough?
Depends on: 557336
(In reply to comment #14)
> To make the factory more extensible, I'd suggest refactoring everything
> firefox-specific into separate methods, which can be overridden by a
> CCMozmillFactory for comm-central code. This way we don't need to copy all of
> the method code.
> 
> Examples: finding the firefox executable, setting the test dirs (this could be
> a class variable maybe), ...

This is already planned, bug 557336 ; I have now noted this in the dependencies for this bug
(In reply to comment #15)
> As a note, Thunderbird already has something set up for running mozmill tests
> in automation, see bug 500142, etc. - parts from they have been doing there
> might be helpful for the work being done here (and in the end, it would be nice
> if all our products run the same infrastructure for this).

Noted in the See Also section;  I agree, I think there is in general from unification of infrastructure (so long as it avoids overspecialization).  I've only skimmed bug 500142 and am not sure when to do make this switch or how heavy of a switch this is (sorry, I'm new here).
See Also: → 500142
(In reply to comment #19)
> I've
> only skimmed bug 500142 and am not sure when to do make this switch or how
> heavy of a switch this is (sorry, I'm new here).

I'll do a brief summary of what Thunderbird does and why.

1) We have two make targets built into the build system: "mozmill" and "mozmill-one".

Both of these mean that any developer can run all Thunderbird mozmill tests (or just one) by typing one or two commands (as long as they have mozmill installed, although we're also thinking about revising how we manage that).

Defined at the bottom of this file: http://mxr.mozilla.org/comm-central/source/mail/build.mk

The output of "make mozmill" is very similar to xpcshell-tests. Debug mode has additional output, and there are summaries at the end. Our wrapper around mozmill manages this and profile directories automatically:

http://mxr.mozilla.org/comm-central/source/mail/test/mozmill/runtest.py
http://mxr.mozilla.org/comm-central/source/mail/test/mozmill/runtestlist.py

Having the wrapper produce the similar log means that we get buildbot and tinderbox parsing of the logs for free.

2) Buildbot then runs mozmill tests in exactly the same way as xpcshell-tests:

http://hg.mozilla.org/build/buildbotcustom/file/59119d3afe97/process/factory.py#l4501

This means no extra/special code in buildbot, which in turn means that developers just have to remember/work with one or two commands rather than many if they are trying to resolve issues.

This also means we hook into the crash-stack reporting "free-of-charge".


The only complication in the Firefox case is the separate repository, but IMO, there's no reason you couldn't have a third "mozmill-update" command that gets/updates the mozmill test directory.
It was decided not to use virtualenv to install the mozmill software.
The solution proposed is to use virtualenv and e.g. pip to create binary
environments for each slave platform which are then uploaded to a
mozilla URL upon packaging mozmill (and presumedly any python package
needed for testing, see below).

It is noted that the packaging of binary virtualenvs further precludes
testing on systems outside of the existing test platforms (and in the
same way, adding a new testing platform requires the generation of a
new mozmill package and whatever other packages are generated this
way).  The virtualenv tarballs will also have to be downloaded to the same
path, as virtualenv is path-sensitive.  The --relocatable switch may
be used to work around this, if desired (though since the binaries
will be hard-coded to platform, I'm not sure what if anything is
gained here).

For initial purposes, I will use a subpath of
http://people.mozilla.com/~jhammel to place the tarball for my one testing
platform (CentOS).  However, when it comes time to deploy, a better
URL space should be used.

Another deployment issue for the URL space is what to call the
tarballs.  For something like mozmill, this could be
e.g. mozmill-1.4-linux.tar.bz2 . This is somewhat misleading, however,
as the tarball will not only contain the site-packages for mozmill,
but also for its dependencies (mozrunner, jsbridge, etc). If the
virtualenv is for a particular script or scripts from one package
(like mozmill), then naming after this source package is probably
sufficient.  

It is my opinion that for the sake of unifying process, python
packages required for testing should all be deployed in the same manner.
Right now this might only be mozmill and its dependencies.  (I'm not
sure if this is the case.)  For this approach, I would recommend a
script be written that handles the creation and packaging of these
virtualenvs, possibly pointing to a pip recipe.
Comment on attachment 434790 [details] [diff] [review]
port of attachment 433163 [details] [diff] [review] to the code today, March 24, 2010;  not yet tested

unobseleting this patch since virtualenv will not be used as is done in further patches
Attachment #434790 - Attachment is obsolete: false
Comment on attachment 438468 [details] [diff] [review]
clean version of mozmill+buildbot integration using virtualenv (semi-tested)

obseleting this patch as it uses virtualenv, which is now not the path we're going down
Attachment #438468 - Attachment is obsolete: true
(In reply to comment #21)
> It was decided not to use virtualenv to install the mozmill software.
> The solution proposed is to use virtualenv and e.g. pip to create binary
> environments for each slave platform which are then uploaded to a
> mozilla URL upon packaging mozmill (and presumedly any python package
> needed for testing, see below).

Are there meeting notes available to get more information why this decision has been made? Is it just to not automatically use the new version once it is available?
Summary: Run mozmill tests → Run mozmill tests on buildbot
i updated the original patch to work with today's buildbotcustom.  it doesn't work OOTB (to be expected, as neither did the original patch), but it is syntatically sound.  

I also made one logic change of removing self.super_class and just calling ShellCommandReportTimeout.<method>(...) vs self.super_class.<method>(...) the one place it was used.
Attachment #434790 - Attachment is obsolete: true
(In reply to comment #25)
> Created an attachment (id=439959) [details]
> port of patch to work with buildbotcustom as of today [April 19, 2010]
> 
> i updated the original patch to work with today's buildbotcustom.  it doesn't
> work OOTB (to be expected, as neither did the original patch), but it is
> syntatically sound.  
> 
> I also made one logic change of removing self.super_class and just calling
> ShellCommandReportTimeout.<method>(...) vs self.super_class.<method>(...) the
> one place it was used.

We need to use the self.super_class idiom so that buildbot reconfigs don't break.
Chris, can you or Ben please give a reply to my comment 24? Thanks.
(In reply to comment #21)
> It was decided not to use virtualenv to install the mozmill software.
> The solution proposed is to use virtualenv and e.g. pip to create binary
> environments for each slave platform which are then uploaded to a
> mozilla URL upon packaging mozmill (and presumedly any python package
> needed for testing, see below).

So attempting to put this simpler, I think you're saying:

- Build (or someone) creates a virtualenv binary of the mozmill software
- Mozmill sofware gets deployed onto the slaves by pip.

What happens when if we get the situation that one branch needs to use one version of mozmill, and another branch wants to use a different version?

Is it possible for developers to run/install these without having pip setups?

I'd also like to know why you're not heading down the route that Thunderbird has for the unit tests themselves that really does make it easier for developers to test, and also allow buildbot code re-use and greater inter-op with other unit tests.
(In reply to comment #28)
> (In reply to comment #21)
> > It was decided not to use virtualenv to install the mozmill software.
> > The solution proposed is to use virtualenv and e.g. pip to create binary
> > environments for each slave platform which are then uploaded to a
> > mozilla URL upon packaging mozmill (and presumedly any python package
> > needed for testing, see below).
> 
> So attempting to put this simpler, I think you're saying:
> 
> - Build (or someone) creates a virtualenv binary of the mozmill software
> - Mozmill sofware gets deployed onto the slaves by pip.

There are details to be worked out but what was discussed was

 - a virtualenv is created on each platform
 - pip (or other) is used to install appropriate versions of mozmill and it's dependencies
 - this virtualenv is tarred up and uploaded to some part of the mozilla.org URL space
 - the slaves retrieve the appropriate tarball and unpack it to run the contained python software

> What happens when if we get the situation that one branch needs to use one
> version of mozmill, and another branch wants to use a different version?

in generateBranchObjects, currently the hardcoded string '1.4.1' is used regardless of branch.  This could be made branch-dependent if necessary.

> Is it possible for developers to run/install these without having pip setups?

I'm not sure I understand who the target audience is.  Buildbot developers?  End developers?  Since this approach is tailored very specifically to the buildbot setup, the tarballs are useless outside of this context.  The slaves will not need to install pip (see above).  Of course, other developers will be able to run/install mozmill in the usual way, paying no attention to these tarballs.

> I'd also like to know why you're not heading down the route that Thunderbird
> has for the unit tests themselves that really does make it easier for
> developers to test, and also allow buildbot code re-use and greater inter-op
> with other unit tests.

I will look at this and assess this approach and present the alternatives here.  I'm not sure if it is my decision to make, though again am for any sort of unification of process for the better.
(In reply to comment #28)
> (In reply to comment #21)
> Is it possible for developers to run/install these without having pip setups?
Yes, we'd want to solve that problem too to make life easier too, but first we need to get some kind of consensus on the approach here.  I'm a bit baffled as to what the reasons are for some of the implicit assumptions that seem to be being made.
> I'd also like to know why you're not heading down the route that Thunderbird
> has for the unit tests themselves that really does make it easier for
> developers to test, and also allow buildbot code re-use and greater inter-op
> with other unit tests.

So, let's talk about those implicit assumptions that I mentioned, and I think that the number one reason we're in this predicament is the answer to your quandary.  

Let's get these on the table so we're all on the same page, I'm not saying these aren't justified, but I don't think everyone shares the same mental map for the problem and that's causing us to get tangled up here.  So, let's deal the cards; I see two basic assumptions in the works here:

Implicit Assumption 1:
* We can't install Mozmill tools on buildbot slaves.  Thunderbird has taken this approach (or seems to in the code you linked us to in comment 20).  For Firefox with 800 or so slaves, that's a big headache to keep everything in sync and updated.

Implicit Assumption 2:
* We don't want virtualenv on the slaves either.  I really don't understand this one.  If some of the bugs that Jeff found are fixed (like the PYTHONHOME thing) I don't know why this wouldn't be a good approach because it seems it could be generally useful for future python based projects.

So, I see three possible ways to make this work:
1. Install virtualenv as part of the buildplatform on the slaves, use it for Mozmill.  
* This has a good incremental approach: We only need to download the virtualenv.py file and can use that without adding virtualenv to the reference image and we can get this effort off the ground quickly.
* But this seems to be blocked by Implicit Assumption 2.

2. Package mozmill/virtualenv/anything else as tarballs and download them
* This seems like a winner on the surface.  But, when you look at what has to be packaged, we start seeing that packing up an entire virtualenv could pull in a bunch of binary components and then we'd have to start packaging these for each OS, which is going to be a much bigger maintenance burden (more error prone) than option 1.
* If we just tarball the site-packages directories we might be able to avoid the per-platform binary issue, but that seems pretty hacky.

3. Use the Thunderbird method and install the mozmill tools onto the buildslaves.
* This runs right into Implicit Assumption 1.

Chris, Ben and John, are there other issues you see with these approaches than the ones I called out?  Are there other things to consider?  Let's get them all on the table so we can move forward and get this long-standing item crossed off the list.  

I'll volunteer to set up a call this week so we can talk through everything if that'd be easier.
(In reply to comment #31)
> Implicit Assumption 1:
> * We can't install Mozmill tools on buildbot slaves.  Thunderbird has taken
> this approach (or seems to in the code you linked us to in comment 20).  For
> Firefox with 800 or so slaves, that's a big headache to keep everything in sync
> and updated.

Actually, we have puppet and OPSI which can automatically install software packages on all slaves. It's some work the releng team needs to go through to set up and test things for a new package to be deployed, but it's surely doable in a reasonable manner across all slaves.
Clint: Maybe my concerns have been miss-directed here and I've not done a good enough explanation, I'm sorry if that is the case.

I have two primary concerns:

1) Making it easy for developers to install and run the same MozMill version that is on the slaves.

This seems very difficult to do in the python space. If one branch needs one mozmill version, and a different branch needs a different one, then I think there will be an issue as easy_install doesn't seem to easily support that - not without constantly changing your installation.

If we're always going to have the same MozMill versions supported across all active branches, that's fine. If not, then I think we should be considering a solution that developers can use as well as the buildbots. If it is the case that just isn't possible, which is now my understanding from your comment, then it is just something we will need to live with.

2) Making it easy for developers to run the MozMill tests from within their build environment.

The current patch for buildbot is doing what I can a "manual process". It downloads the required items (that's fine) and then manually runs mozmill via invoking mozmill direct.

The assumption I've got here (implied by comment 0) is that because we're integrating with buildbot, we're also going to be displaying these results on tinderbox - if we're doing that, then I see a couple of sub-issues with the manual process:

- it doesn't fit with any of the ways developer run tests, e.g. make xpcshell-tests, make reftest. This is going to make it hard for developers to remember how to run these tests when they do need to run them.

- I'm fairly sure tinderbox won't parse the results (which typically require TEST_UNEXPECTED_FAIL) so developers and tools won't be able to find the failing tests easily.

This second concern is where I've been coming from with the Thunderbird perspective. Everything we've been doing has been focussed on making it easy for developers to work with the MozMill test harness so that it quicker for them to write the tests.
> Implicit Assumption 1:
> * We can't install Mozmill tools on buildbot slaves.  Thunderbird has taken
> this approach (or seems to in the code you linked us to in comment 20).  For
> Firefox with 800 or so slaves, that's a big headache to keep everything in sync
> and updated.

As KaiRo mentions below, we do have tools that will let us install Mozmill on the build slaves.  However, my preference is to have them downloaded and installed as part of the test run.  It makes the environment cleaner, and it means you can easily have different versions of Mozmill for different branches of Firefox.

> Implicit Assumption 2:
> * We don't want virtualenv on the slaves either.  I really don't understand
> this one.  If some of the bugs that Jeff found are fixed (like the PYTHONHOME
> thing) I don't know why this wouldn't be a good approach because it seems it
> could be generally useful for future python based projects.

Our objection here isn't to virtualenv itself, it's to using easy_install or pip to install Mozmill and its dependencies from an external network service.  It's also difficult to specify exactly which version of all dependencies to use.

> So, I see three possible ways to make this work:
> 1. Install virtualenv as part of the buildplatform on the slaves, use it for
> Mozmill.  
> * This has a good incremental approach: We only need to download the
> virtualenv.py file and can use that without adding virtualenv to the reference
> image and we can get this effort off the ground quickly.
> * But this seems to be blocked by Implicit Assumption 2.

I have no problem with this, it would be generally useful to have available.

> 2. Package mozmill/virtualenv/anything else as tarballs and download them
> * This seems like a winner on the surface.  But, when you look at what has to
> be packaged, we start seeing that packing up an entire virtualenv could pull in
> a bunch of binary components and then we'd have to start packaging these for
> each OS, which is going to be a much bigger maintenance burden (more error
> prone) than option 1.
> * If we just tarball the site-packages directories we might be able to avoid
> the per-platform binary issue, but that seems pretty hacky.

The packages aren't that large, and the procedure to produce such a tarball not that complicated.

> 
> 3. Use the Thunderbird method and install the mozmill tools onto the
> buildslaves.
> * This runs right into Implicit Assumption 1.
> 
> Chris, Ben and John, are there other issues you see with these approaches than
> the ones I called out?  Are there other things to consider?  Let's get them all
> on the table so we can move forward and get this long-standing item crossed off
> the list.

My primary concerns are:
- Dependence on external network services.  Our tests shouldn't start to fail because pypi or the external internet link is down.

- Repeatability of setup.  If we use easy_install or pip, we need to make sure we can get the exact same set of dependencies each time we set up the environment.  This is important regardless of when we're doing the installation (at machine configuration time, or test-run time).  We have new slaves coming online all the time, so a new slave that is getting configured in August should get the same versions as a slave configured in April.

Packaging the virtualenvs into tarballs solves both of these problems for me.

> I'll volunteer to set up a call this week so we can talk through everything if
> that'd be easier.

Releng is having our work week this week, so maybe next week would be better.
(In reply to comment #34)
> 
> 
> > Implicit Assumption 2:
> > * We don't want virtualenv on the slaves either.  I really don't understand
> > this one.  If some of the bugs that Jeff found are fixed (like the PYTHONHOME
> > thing) I don't know why this wouldn't be a good approach because it seems it
> > could be generally useful for future python based projects.
> 
> Our objection here isn't to virtualenv itself, it's to using easy_install or
> pip to install Mozmill and its dependencies from an external network service. 
> It's also difficult to specify exactly which version of all dependencies to
> use.
> 
This is entirely reasonable.  And it also addresses one of Mark's concerns which is how to support different versions of the tool across different branches.  We can't solve that with easy_install or PIP, and so we'd need to solve that by hosting these things in house.

> > So, I see three possible ways to make this work:
> > 1. Install virtualenv as part of the buildplatform on the slaves, use it for
> > Mozmill.  
> > * This has a good incremental approach: We only need to download the
> > virtualenv.py file and can use that without adding virtualenv to the reference
> > image and we can get this effort off the ground quickly.
> > * But this seems to be blocked by Implicit Assumption 2.
> 
> I have no problem with this, it would be generally useful to have available.
> 
Ah cool.  I'm glad I misunderstood things then :)

> > 2. Package mozmill/virtualenv/anything else as tarballs and download them
> > * This seems like a winner on the surface.  But, when you look at what has to
> > be packaged, we start seeing that packing up an entire virtualenv could pull in
> > a bunch of binary components and then we'd have to start packaging these for
> > each OS, which is going to be a much bigger maintenance burden (more error
> > prone) than option 1.
> > * If we just tarball the site-packages directories we might be able to avoid
> > the per-platform binary issue, but that seems pretty hacky.
> 
> The packages aren't that large, and the procedure to produce such a tarball not
> that complicated.
I was under the impression that wasn't the case, for some reason.  I may be confused.  I'll do some experimentation and see.

> 
> My primary concerns are:
> - Dependence on external network services.  Our tests shouldn't start to fail
> because pypi or the external internet link is down.
Makes complete sense. We'd definitely want to avoid that.
> 
> - Repeatability of setup.  If we use easy_install or pip, we need to make sure
> we can get the exact same set of dependencies each time we set up the
> environment.  This is important regardless of when we're doing the installation
> (at machine configuration time, or test-run time).  We have new slaves coming
> online all the time, so a new slave that is getting configured in August should
> get the same versions as a slave configured in April.
Makes sense.
> 
> Packaging the virtualenvs into tarballs solves both of these problems for me.
> 
I see.  Having the version specific tarballs available on a server someplace on our network for download would solve both the "external network" problem and the problem of tying a specific branch to a specific version of the tool.  

I'll have to experiment with packaging these things and see what is involved in doing that.

> > I'll volunteer to set up a call this week so we can talk through everything if
> > that'd be easier.
> 
> Releng is having our work week this week, so maybe next week would be better.
Cool, by then I'll have more experience with creating these packaged tarballs and a clearer picture of what this solution entails.

Thanks for helping more clearly define the problem.
(In reply to comment #33)
> Clint: Maybe my concerns have been miss-directed here and I've not done a good
> enough explanation, I'm sorry if that is the case.
> 
> I have two primary concerns:
> 
> 1) Making it easy for developers to install and run the same MozMill version
> that is on the slaves.
> 
> This seems very difficult to do in the python space. If one branch needs one
> mozmill version, and a different branch needs a different one, then I think
> there will be an issue as easy_install doesn't seem to easily support that -
> not without constantly changing your installation.
> 
I agree.  I think that if catlee's proposed downloadable tarball system works well for the slaves, then we may use that to craft a way for developers to easily run these tests on their local box as well.  I completely agree the standard python packaging schemes don't work well.  Forcing people to constantly change their installation is a not a goal.

> If we're always going to have the same MozMill versions supported across all
> active branches, that's fine. If not, then I think we should be considering a
> solution that developers can use as well as the buildbots. If it is the case
> that just isn't possible, which is now my understanding from your comment, then
> it is just something we will need to live with.
> 
I think I wasn't clear.  I would prefer the developers to use the same solution as the buildbots.  Ideally, they could both use make targets.  The question of how feasible that is will depend on how much hacking we have to do to support these downloadable tarballs.  It may be that it is a make target which then calls a python function which then downloads and installs the tarballs and then calls the proper mozmill stuff -- not unlike what you have in your runtest.py.

However, that said, I'm a bit uneasy with a make target that essentially does a wget to download a tool from a server (any server).  If we were to go down that road, I'd want an option in the make target to not go out and pull the code down from an external resource.

> 2) Making it easy for developers to run the MozMill tests from within their
> build environment.
> 
> The current patch for buildbot is doing what I can a "manual process". It
> downloads the required items (that's fine) and then manually runs mozmill via
> invoking mozmill direct.
> 
> The assumption I've got here (implied by comment 0) is that because we're
> integrating with buildbot, we're also going to be displaying these results on
> tinderbox - if we're doing that, then I see a couple of sub-issues with the
> manual process:
We can work out a better "manual process" for buildbot once we have a clear picture on how to move forward with getting the mozmill toolchain installed on the slave boxes.  All the rest of this is dependent on that.
> 
> - it doesn't fit with any of the ways developer run tests, e.g. make
> xpcshell-tests, make reftest. This is going to make it hard for developers to
> remember how to run these tests when they do need to run them.
I think they should have make targets like every other test harness we have.  Whether we use those make targets in buildbot or not is a separate issue.  I'd like to always use the same mechanism, but that said, I'm not a huge fan of a make target that turns around and wget's some tarballs.  Generally, I like to do my hg pull and then disconnect and run my make commands.

> 
> - I'm fairly sure tinderbox won't parse the results (which typically require
> TEST_UNEXPECTED_FAIL) so developers and tools won't be able to find the failing
> tests easily.
> 
That is something I am working to change in Mozmill so that it has identical logging to the other test harnesses when it is included in buildbot.  I have been trying to get it fixed for a long time, but it's hard to get people to work on something as unsexy as log reporting.  I'll be doing it myself this week or next.

> This second concern is where I've been coming from with the Thunderbird
> perspective. Everything we've been doing has been focussed on making it easy
> for developers to work with the MozMill test harness so that it quicker for
> them to write the tests.
And that is a laudable goal, and one that we want to maintain as we do this integration too.

So to roll it all up:
* We want it to be easy for developers to run the tests.
** They should have some kind of make target like every other test tool we own.
* We want to be able to designate a certain branch runs a certain version of the tool, forever (repeatability)
* We want to download and install the external required tools from some place on our network

And the unresolved questions:
* Do we want make targets that download things?  How do you tell a make target to optionally not download something?  Environment variables?
* How to package those tarballs (I'll be looking into that with Jeff this week).
(In reply to comment #36)
> * Do we want make targets that download things?  How do you tell a make target
> to optionally not download something?  Environment variables?

Can we have some env var point to the location where any versions of mozmill are installed in versioned subdirs (e.g. mozmill-1.4/ and mozmill-1.5/ under the specified dir) and point to those for execution (via PYTHONPATH or so)?
That would not need a make target download it, but just require the user or build machine to unpack a package in a location with that path and set the env var accordingly.
Jeff and I were talking about this some more, when we had an "aha" moment.  And I'm quite ashamed I didn't think of this 6 months ago as it would have made all our lives easier.  But anyhow, there you go.  Here's the idea.

Store the versioned mozmill tarballs in the mozilla source tree with the other test harnesses.  Put them at testing/mozmill.  Also store the virtualenv.py there.

Modify the make files so that when you do a make package-tests you get this directory packaged up into <package>/mozmill

The Buildbot Mozmill Step becomes quite simple:

1. Check out mozmill tests

2. Then inside buildbot we'd call a python runmozmill.py which would:
* Instantiate the virtualenv, placing the momzill dependencies into it
* Run mozmill on those tests from step 1

3. Deactivate the virtualenv, clean up the testing directory, report the log file results

This way we can also have the make mozmill functionality available which would merely perform the same action as step 2 in the buildbot case.

I really like this idea as it solves a bunch of the issues with the previous ideas:
1. It's simple -- it's exactly how all the other tests are run (I want to slap myself for not seeing it sooner)
2. It resolves the external network problem
3. It's easy for developers to run
4. It makes the necessary buildbot changes very simple.
5. It gives us the stability we need.  We can either have each released version of mozmill in that testing/mozmill directory or we can allow the eventual branches of the mozilla source to ensure that we only find the version of mozmill that works for our version of the application under test.

So either you'd have:
* mozilla-central:
/testing/mozmill/ <all mozmill versions>
or 
* mozilla-central:
/testing/mozmill/ <only verison of mozmill that works for mozcentral>

And likewise for 1.9.2, 1.9.1 and so forth.

What do you think?  Jeff and I started putting together a work in progress patch for it this afternoon.  We'll have something here to show so you can check it out soon.
oops, step 2 correction, "then inside buildbot we'd call" should be: "then we'd call".  
Sorry for the spam.
(In reply to comment #38)
> 1. Check out mozmill tests

Would it make sense to have those mozmill tests that should be part of the automated suite also live in the source tree, then, just like xpcshell and mochi* tests are?

> 2. Then inside buildbot we'd call a python runmozmill.py

Could we actually have buildbot call a "make mozmill-tests" that actually cares about what to run and set up? This would closely mirror what other tests are doing right now and therefore would be as intuitive as it gets.
... the other question I'd have is how much of a size-hit for the repo that is for each tarball.
(In reply to comment #41)
> ... the other question I'd have is how much of a size-hit for the repo that is
> for each tarball.

The total size of the tarballs is 4.5M:
 mozmill: 1.2
 jsbridge: 1.6
 mozrunner: 20k
 simplejson: 136k
 virtualenv: 1.5M
That's something like a 1% diskspace increase for mozilla-central, assuming that the version in .hg is about the same size, and a 4% increase in download of fresh clones, judging by the bundle size on ftp.

I'm mostly asking because in particular our localizers need the complete source and are not on beefy machines with beefy connections around the globe.
(In reply to comment #42)
> (In reply to comment #41)
> > ... the other question I'd have is how much of a size-hit for the repo that is
> > for each tarball.
> 
> The total size of the tarballs is 4.5M:
>  mozmill: 1.2
>  jsbridge: 1.6
>  mozrunner: 20k
>  simplejson: 136k
>  virtualenv: 1.5M

Is that just source, or does that include binary components as well?
(In reply to comment #44)
> (In reply to comment #42)
> > (In reply to comment #41)
> > > ... the other question I'd have is how much of a size-hit for the repo that is
> > > for each tarball.
> > 
> > The total size of the tarballs is 4.5M:
> >  mozmill: 1.2
> >  jsbridge: 1.6
> >  mozrunner: 20k
> >  simplejson: 136k
> >  virtualenv: 1.5M
> 
> Is that just source, or does that include binary components as well?

The majority of the space for mozmill and js is in the extensions subdirectory in the xpi files.  For virtualenv, the majority of the space is for the setuptools (and distribute) eggs.  bzipping does not dramatically decrease the space (these are .tar.gzs)
(In reply to comment #45)
> (In reply to comment #44)
> > (In reply to comment #42)
> > > (In reply to comment #41)
> > > > ... the other question I'd have is how much of a size-hit for the repo that is
> > > > for each tarball.
> > > 
> > > The total size of the tarballs is 4.5M:
> > >  mozmill: 1.2
> > >  jsbridge: 1.6
> > >  mozrunner: 20k
> > >  simplejson: 136k
> > >  virtualenv: 1.5M
> > 
> > Is that just source, or does that include binary components as well?
It's everything, and the next version of Mozmill (the one that will be checked in here) will actually be a bit smaller.  I just verified that there is a bunch of crufty bundled extensions inside of jsbridge that I can remove (they're debug-only and no longer used).  So, jsbridge's size becomes 17K.  So the total number becomes 2.8Mb for the entire bundle.

So that's roughly half a percent of the entire build checkout, and I hope that's better.  Is that an OK addition to the build tree?

Axel, in order to help the localizers the most we ought to figure out a way to have them pull down only the true build files (application code, DTD and properties) and exclude the thousands of mochtiest, reftest, and jsreftest files (which are completely irrelevant to their efforts).  If we could do that alone, we'd drop the total download on checkout by about 100Mb.  If you'll file a bug for that (perhaps you already have), we can try to accommodate that. But I think that is certainly a separate issue from this bug.
Hmm, you're talking about tarballs here? Would it be possible to follow the style of other stuff we have in trees to have those things present in unpacked form and bascially ready to go without intermediate steps to unpack/"install"? Or does tha plan/investigation already care about that anyhow?
To chime in very late in the discussion, our other test suites follow the premise that:
1) All prerequisites to run the tests should be in the source tree (mozilla-central) and packaged into the test package
2) Running the tests should only involve invoking a single command, and not any other setup

The primary consumers of our automated tests are developers and buildbots, therefore we should optimize for their convenience. Mozmill has historically taken a different tack here, and made itself fit into the Python world. That's a fine course to take, but not one that fits into the Mozilla world very well, and as a result things like this bug are more difficult than they need to be.
(In reply to comment #47)
> Hmm, you're talking about tarballs here? Would it be possible to follow the
> style of other stuff we have in trees to have those things present in unpacked
> form and bascially ready to go without intermediate steps to unpack/"install"?
> Or does tha plan/investigation already care about that anyhow?
When installing into virtualenv, we need them in the distributable format (i.e. the tarballs).  That's why I was recommending we check them in that way.  Since Mozmill is maintained in a separate repository, the code checked in here would be a version release of the tool, and would not be that useful for hacking on, so this still seems like the right way to go.

@Ted:
I think this approach solves both 1 & 2 for Mozmill.
1 - all the prerequisites to run the test harness will be checked in*
2 - if we have these harnesses checked in, we can supply a make target that will also run the mozmill tests as well, in order to enable there to be only a single command for running the tests.

* Now, all the tests themselves are not in the m-c tree.  Those are stored in hg.mozilla.org/qa/mozmill-tests, but I don't think that's as important as having the harnesses in the tree.  I think we can still support both cases 1 & 2 and still leave the tests in their own repo.  We will just have the script that gets called as a result of "make mozmill" look for the locally checked out tests and if it doesn't find them will optionally ask to download them.

I don't see much point of putting those tests in the m-c tree at this time.  (They are in a different repo so that commit restrictions to that repo can be more relaxed and so that it is easier to allow people to check in those tests).
(In reply to comment #49)
> I don't see much point of putting those tests in the m-c tree at this time. 
> (They are in a different repo so that commit restrictions to that repo can be
> more relaxed and so that it is easier to allow people to check in those tests).

This is a personal view, but I would prefer if they were in the repository, because it prevents the need for downloading tests during builds, developers can easily write tests in with their patches instead of needing a separate patch, and someone wanting to run mozmill tests locally--like a developer--wouldn't want two repositories to manage for development and local testing.  Also, it could mean backouts in two repositories if a developer commits something to m-c that is tied to a new/updated mozmill test: if only one backed-out, it creates extra orange.

I don't see anything wrong with having the other repository as a Mozmill testing ground, but the official tests for mozilla-central probably should be in the m-c repository for simplicity.
I still wonder what this virtualenv stuff is and why the *** we need it at all here. It seems like a large overhead and I don't see us using something similar for any other type of automated tests.

Also, I agree that tests should live in the repository, and actually in the directories of the modules they test. Tooling at build time should copy/symlink them into a place that fits the requirements of running the tests. That's how other tests work, and it allows for ideal modularity and differing of tests across branching as well as having code and tests go in in the same patch.
(In reply to comment #46)
> > > Is that just source, or does that include binary components as well?
> It's everything, and the next version of Mozmill (the one that will be checked
> in here) will actually be a bit smaller.  I just verified that there is a bunch
> of crufty bundled extensions inside of jsbridge that I can remove (they're
> debug-only and no longer used).  So, jsbridge's size becomes 17K.  So the total
> number becomes 2.8Mb for the entire bundle.

See bug 551084.

> Axel, in order to help the localizers the most we ought to figure out a way to
> have them pull down only the true build files (application code, DTD and
> properties) and exclude the thousands of mochtiest, reftest, and jsreftest
> files (which are completely irrelevant to their efforts).  If we could do that
> alone, we'd drop the total download on checkout by about 100Mb.  If you'll file
> a bug for that (perhaps you already have), we can try to accommodate that. But
> I think that is certainly a separate issue from this bug.

If localizers wanna chime in and test their builds, we shouldn't force them to get familiar with the build system. Instead we will have a much more helpful solution by end of this quarter by our cloud testing extension for Mozmill. Project documentation will be added later this week here: https://wiki.mozilla.org/QA/Mozmill_Test_Automation/Cloud_Testing

(In reply to comment #49)
> When installing into virtualenv, we need them in the distributable format (i.e.
> the tarballs).  That's why I was recommending we check them in that way.  Since
> Mozmill is maintained in a separate repository, the code checked in here would
> be a version release of the tool, and would not be that useful for hacking on,
> so this still seems like the right way to go.

As given by our policy we officially run Mozmill tests only with release builds of Mozmill, Jsbridge, and Mozrunner. We could use hg-git to move over the release code of all three repositories to m-c or any other branch.

(In reply to comment #51)
> Also, I agree that tests should live in the repository, and actually in the
> directories of the modules they test. Tooling at build time should copy/symlink
> them into a place that fits the requirements of running the tests. That's how
> other tests work, and it allows for ideal modularity and differing of tests
> across branching as well as having code and tests go in in the same patch.

We created its own repository because of relaxing rules (as Clint already said) and for simplicity to get new people involved into the process of test creation and to run those. Meanwhile we also have a git repository for the tests we could use for this aspect. That means I wouldn't be totally against this approach. We only have to make sure that we still can use hg-git to synchronize tests of both repositories.
(In reply to comment #52)
> We created its own repository because of relaxing rules (as Clint already said)
> and for simplicity to get new people involved into the process of test creation
> and to run those. Meanwhile we also have a git repository for the tests we
> could use for this aspect. That means I wouldn't be totally against this
> approach. We only have to make sure that we still can use hg-git to synchronize
> tests of both repositories.

It is very important to have other users involved in creating tests and encouraging community involvement, but there still need to be strict rules or reviews for tests before they end up in the tree, just as we do for our other tests.  We could certainly use the git repository or the other hg repository as a testing ground that is added to the repository after being reviewed by a developer or qa engineer, and from what little I know of git-hg integration it could work very well for this.

We are probably saying the same thing; I only want to be reassured that this wont lead to more overhead and tree orange than we have already.
That cloud testing project is cool. I hope it'll do all the things the project page announces :-)

Anyway, for our localizers, being able to test locally is pretty important, so maybe having the mozmill tests in there as the only test suite they'd likely run might actually be a plus. And they're in touch with the build sytem for the most part already as soon as they touch a language pack. They don't need to actually compile, though. So if we can hook things up such that 
- getting a nightly
- doing a langpack
- do mozmill l10n tests
works, that'd be a great benefit.

Regarding the "just a repo that has what l10n needs", we used to have that on CVS, and it just merrily broke every other day because nobody used it. Not sure if hg convert can smart-ass us these days, in particular what happens if we add a directory to the requirements with existing history.
@KaiRo:
Virtualenv is a virtual environment for python.  It gives you the ability to install python packages and easily remove them without affecting the rest of your python toolchain.  (You'd think that would naturally be a part of python package management, but it isn't, that's why we need virtualenv.)  This way we can install and remove mozmill very easily.  (Mozmill is a pacakged python application, and it has a couple of dependencies on other python packages that are not part of the standard build configuration).  We used python packaging for mozmill because it makes life easier for 99% of the people using it.  Unfortunately, it makes it more difficult for us (as you can see on this bug).

@Axel, Tanner, Whimboo: I'd love to have the tests in the m-c tree. With that said, we aren't going to put anything in the tree that *increases* orange-ness.  That would be counterproductive to all involved.  I vote that we keep this bug focused on mozmill-buildbot integration and we file a follow on bug for figuring out where & how to put the Mozmill tests into the m-c tree. Bug 562106 tracks this.

@Axel: I'd forgotten about the old CVS days, yeah the separate repo never really worked, did it :(  We should be able to figure out a way to run mozmill separately without a build.  I imagine a simple script could do that quite easily, but that is also a follow-on bug, and is not blocking mozmill-buildbot integration.  Bug 562107 tracks this.

@all: It sounds like the "in the tree" approach is a winner here.  Can we move forward with pursuing mozmill buildbot integration using this approach?
(In reply to comment #55)
> Unfortunately, it makes it more difficult for us (as you can see on this bug).

Hrm, and there's no possibility to easily adapt it to a way that it could run by just adding its path to PYTHONPATH or so? Bummer. Well, I guess we'll need to live with going the complicated route, then. How I "love" python.
(In reply to comment #54)
> That cloud testing project is cool. I hope it'll do all the things the project
> page announces :-)

Due to the reason I got the question twice today lets give a short overview here:

With the cloud testing extension we want to offer users a really simple way to run existing Mozmill tests. They shouldn't have to install or prepare anything on their system regarding Python, setuptools, virtualenv, or whatever else. We want to create an extension which puts an additional toolbar to your running Firefox instance. This toolbar will contain a dropdown to select the type of test you wanna run (default, restart, l10n, software update, add-ons...) and a button to start the test. A temporary environment will be created, the Mozmill tests are downloaded, and a new instance of the current build gets started. All the tests will be run and results will be reported to brasstacks. Afterward everything will be removed again and the system is still clean.

With this extension everyone should be able run tests whether which build he is using. I believe that this is the way you wanna see it Axel, am I correct? By end of the week I will have updated the project page and filed a tracking bug. We can follow-up on that bug then.
For localizers, requiring MozillaBuild would be fine, we're doing that anyway. QA volunteers might be different, though, yeah.
I think the technical aspects of this issue are known at this
point and now the major action item is making decisions as far as how
we want to do things.  I will attempt to outline the major decisions
from this ticket and speak to a few outstanding questions.  At the
end, I will present an abridged form of the decision tree and some notes.

Requirements:

 - The mozmill tests at http://hg.mozilla.org/qa/mozmill-tests should
   be run by buildbot and the results reported

 - The buildbot should not touch network outside of Mozilla addresses;
   The steps in the m-c makefile should not touch net at all

 - The size of the m-c tree should not be significantly increased (out
   of friendliness to l10n workers, amongst other reasons).  For
   reference, the current tree size is 406M.  `hg clone`ing the tree
   brings down an additinal 437M of history.

Answers to questions regarding python packaging:

 - Why does python packaging suck so bad?

It's a long and complicated story.  The stdlib comes with one
packaging solution, distutils (see
http://docs.python.org/distutils/setupscript.html for how setup.py is
done in distutils).  It's not a very good solution.  About python
2.3-2.4, setuptools
(http://peak.telecommunity.com/DevCenter/setuptools) became a much
more functional ad-hoc standard.  But setuptools had/has problems.
Most of these were a human problem;  without going into details or
naming names (though you can look on the internet if your curious),
the maintainer of setuptools did not promptly fix bugs and maintained
ownership of the project even though he had no time to work on it.
setuptools was originally slated for python 2.6, but Guido looked at
the code and said no way.  Setuptools has been forked now, to
distribute, and with packages like virtualenv, virtual-python, and
pip, it seems like after the long delay, python is again moving
forward on the packaging front.

 - What is this virtualenv stuff and why the **** do we need it at all?

virtualenv is a way of installing python packages without touching
system python.  You do 'virtualenv{.py} myvirtualenv' and then
anything installed with the setuptools or pip in that virtualenv gets
installed locally and not in the global site-packages.  virtualenv
does not need to be installed to run, though the entire tree needs to
be downloaded (1.4M, mostly several versions of setuptools and pip and
distribute) or net needs to be available for it to work properly.  If
we want to really install a (setuptools) package, virtualenv makes it
easy.

Top-level Decisions:

 - Should mozmill tests be run via the m-c makefile (e.g. `make
   mozmill`) or should these tests be treated separately for buildbot?

 - Should the mozmill tests live in the m-c tree or should they live
   separately (and possibly be mirrored in the m-c tree)?

 - Should mozmill, jsbridge, and mozrunner be separate packages with
   setuptools (or e.g. distribute) dependencies or should they be
   packaged together?

-- 

Should mozmill tests be run via `make mozmill` or should they be
treated separately?

If we do include the mozmill tests in m-c, there are a few options:

 - assume mozmill, etc is already installed.  This is currently what
   Thunderbird does for its mozmill + buildbot configuration.

 - include mozmill and its dependencies and some way of installing in
   m-c; there should be some sort of installation step (e.g. `make
   mozmill-install`) that `make mozmill` will depend on.  This gives
   rise to the decision "How should mozmill be mirrored and installed
   from m-c"?

--

How should mozmill be made to "live" in the m-c tree?

 - The tarballs of mozmill and its dependencies (mozrunner, jsbridge,
   simplejson) may be included in the m-c tree and updated when these
   packages are updated.

 - The raw source of mozmill and its dependencies may be mirrored in
   the m-c tree and updated per release.

 - mozmill and it's Mozilla-written dependencies (mozrunner +
   jsbridge) will live directly in the m-c tree.  This will make
   running mozmill difficult without obtaining the tree.  In
   addition, simplejson (a third-party dependency) should be mirrored
   in the m-c tree or it should be assumed to be installed.

All three of these options give rise to the question:  "How should
mozmill be installed?"

--

How should mozmill be installed?

There seem to be two schools of thought:  

 1. Install mozmill the pythonic way;  mozmill (and jsbridge and
 mozrunner) are setuptools packages and the normal way of dealing them
 is installing them.  virtualenv (see above) allows one to do this
 in a directory that won't touch system python and may be safely
 deleted after the tests are run.

 A reference implementation (in bash):

/path/to/virtualenv.py mozmill
. mozmill/bin/activate

pip install mozrunner-2.4.2.tar.bz2 jsbridge-2.3.5.tar.bz2 mozmill-1.4.1.tar.bz2 # [maybe simplejson too]
# note that order is very important here (sadly)

python mozmill/bin/mozmill --showall -b /path/to/firefox -t /path/to/tests
python mozmill/bin/mozmill-restart --showall -b /path/to/firefox -t /path/to/restart/tests

deactivate # not really necessary since you're in a subshell

 2. Run mozmill without installation.  It is possible to run mozmill
 without python installed.  Again, a bash reference implementation:

mkdir mozmill
cd mozmill
for tarball in mozrunner-2.4.2.tar.bz2 jsbridge-2.3.5.tar.bz2 mozmill-1.4.1.tar.bz2 # [maybe simplejson too]
do
  DIRECTORY=${tarball%.tar.*}
  PACKAGE=${tarball%-*}
  tar xzvf ../$tarball

  # not sure if this path will be right on windows
  export PYTHONPATH=$PYTHONPATH:$PWD/$DIRECTORY
done

python -c 'import mozmill; import sys; sys.argv[1:] = ["--showall", "-b", "/path/to/firefox", "-t", "/path/to/tests"]; mozmill.cli()'
python -c 'import mozmill; import sys; sys.argv[1:] = ["--showall", "-b", "/path/to/firefox", "-t", "/path/to/restart/tests"]; mozmill.restart_cli()'

This assumes that the packages are tarred up a la `python setup.py
sdist`.  This is an open issue.  See the question "How should mozmill
be packaged if it is not to be installed?"  Alternate methods could
alleviate some of this complication.

Note that this is rather complicated.  This could be made simpler if
the subclasses of CLI were each contained in their own file with a 
`if __name__ == '__main__':` clause and a main function (if we
continue to provide setuptools console_scripts at all.  If we don't,
the function level of abstraction may be forgone).  [-> BUGZILLA?]

In addition, the overriding of `sys.argv` is necessary as the entry
point functions (e.g. `mozmill.cli`) do not take or pass command line
arguments.  If the arguments were passed, then fuxoring `sys.argv`
wouldn't be necessary [-> BUGZILLA?]

I'll mention another option for the sake of thoroughness:

 3. Install mozmill via altering PYTHONHOME.  I'll just give
 pseudocode for this, to be filled out if there is any interest:

PYTHONHOME=$PWD/mozmill
# create directory structure necessary for python
# copy/symlink files needed from active python (e.g. simplejson, setuptools)
for i in mozrunner-2.4.2.tar.bz2 jsbridge-2.3.5.tar.bz2 mozmill-1.4.1.tar.bz2 # [maybe simplejson too]
do
  easy_install $i
done
mozmill/bin/mozmill ...
mozmill/bin/mozmill-restart ...

This assumes that setuptools is available on the system (it is on the
buildslaves).  In the case that it is not, this will either fail or we
can (like virtualenv) package setuptools eggs as part of our setup.
Essentially going down this route is recreating a "cheap" version of
virtualenv (see
http://blog.dscpl.com.au/2007/11/poor-mans-python-virtual-environment.html )

Any other options?

Note that how they are installed affects how they are to be run via
the command line.

There is also the possibility (maybe hard to do with make OOTB? at
least with two different targets) to install mozmill only if the user
doesn't have it already available (on PATH in unix, I have no idea how
to determine on windows).  Maybe worth thinking about, but I won't go
further into it here.

--

How should mozmill be packaged if it is not to be installed?

The examples above use tarballs created as by `python setup.py
sdist`.  However, there are alternatives to this convention:

 - If the code is to be included in m-c, then you wouldn't have to tar
   at all, if you didn't want to.  You could stick the code (with or
   without the top-level directories with setup.py, as appropriate to
   the rest of the setup) into the directories where you wanted them.
   There would be a space cost

 - You could tar up all of the desired packages into one tarball,
   setup as you wished it

 - If you were using virtualenv (or really pip, which comes with
   virtualenv), you could use `pip bundle` to create a bundle with the
   packages you desired.  This only works with pip.

-- 

Where should the mozmill tests for Firefox live?

Currently the mozmill tests live at http://hg.mozilla.org/qa/mozmill-tests.  

Choices:

 - the mozmill tests can continue to live at
   http://hg.mozilla.org/qa/mozmill-tests but will be mirrored
   within m-c

 - the mozmill tests can be moved to m-c and the tree at
   http://hg.mozilla.org/qa/mozmill-tests may be obseleted

 - the mozmill tests can continue to live at
   http://hg.mozilla.org/qa/mozmill-tests and not be mirrored
   within m-c.  This makes running via `make mozmill` impractical, but
   buildbot may still fetch the tests over the net.


--

Should mozmill, jsbridge, and mozrunner be separate packages with
setuptools (or e.g. distribute) dependencies or should they be
packaged together? 

Currently, mozmill depends on jsbridge and mozrunner (and simplejson,
which as of python 2.6 is part of the stdlib, so if we're willing to
demand using python 2.6+ then we can forget about that dependency.  If
not, we will have to do something about it).  These are separate
python packages with the dependencies between them (and to simplejson)
being satisfied via setuptools.  The buildslave (at least my image) is
on python 2.5.1 for reference.

In general, this is a philosophic decision that we should have clarity
over going forward:  is it a goal to have an open and accessible
testing structure?  I have heard conflicting reports from different
people, so I don't really know.  

In my experience, even if there is a single target platform, it is
(almost) always a net win to separate functional infrastructure from
platform-specific infrastructure. In other words, functional
infrastructure should be deployable and consumable
freely. Platform-specific infrastructure should contain hard-coded
paths, lists of types of deployments of interest, etc. that are
useless outside of (in this case) testing on a particular set of
platforms.

I realize this opinion is contentious and that this is a lofty goal
and not one that can be achieved from where we are in a day.  I also
realize that as a new hire that my opinions in this and other
broad-reaching areas may not mean much.  

-- 

(Abridged) Decision tree:

Run tests via makefile or only on buildslaves?
|
+- via makefile: 
|    +-Install via virtualenv, PYTHONHOME, or run uninstalled?
|    |
|    +-Should the tests live in m-c or be mirrored there?
|    |
     +-How should the tarballs be created?
| 
+- on buildslaves: 
      +-Install w/ virtualenv, PYTHONHOME, or run uninstalled?
      |
      +-How should the tarballs be created?
      |
      +-Where should tarballs be kept?
        -> document this as well as who is responsible for generating
--

Notes:

 - Believe it or not, this is an abridged set of decisions.  I have tried
   to be as thorough as possible, but focusing on the major decisions.

 - None of these methods are future proofed.  That is to say, when a new
   version of mozmill (etc) is released, a person must manually change
   code.  We should have a checklist for each product that tells what
   needs to be changed on release or it probably won't get done.  I
   offer to help develop this once these decisions get made.
(In reply to comment #59)
I think you've just won the prize for "longest comment in bugzilla that is not a stack trace". ;)  Ok, so here's my proposal so we can close in on one decision here.
> 
> (Abridged) Decision tree:
> 
> Run tests via makefile or only on buildslaves?
Both.  We need to package mozmill into the make package-tests target.  Buildslaves do not have a build tree, they get their tests and test harnesses from that package so they will need to obtain the mozmill harness by uncompressing a downloaded test package.
> |
> +- via makefile: 
> |    +-Install via virtualenv, PYTHONHOME, or run uninstalled?
> |    |
Install via virtualenv so that running make mozmill doesn't change your working python toolchain.

> |    +-Should the tests live in m-c or be mirrored there?
> |    |
TBD: you don't need to worry about this right now for buildbot stuff.  For the buildbot stuff, assume you will download a tar.gz of the mozmill tests.  This can be solved later when we complete the make mozmill functionality.

>      +-How should the tarballs be created?
python setuptools.py sdist is the easiest option and I don't see any reason to change that.  Three tarballs, one for each component.
> | 
> +- on buildslaves: 
>       +-Install w/ virtualenv, PYTHONHOME, or run uninstalled?
>       |
* Download the packaged tests tar.bz, unpack
* Use virtualenv so that the buildslave python path is not changed 
>       +-How should the tarballs be created?
* setuptools.py sdist, uses the same tarballs that the make system uses (the make system just packages those mozmill tarballs into a distributable package with all the other tests.
>       |
>       +-Where should tarballs be kept?
>         -> document this as well as who is responsible for generating
In hg.  Specifically: hg.mozilla.org/mozilla-central/testing/mozmill/
I'll be the person responsible for uploading these to hg when a new release to Mozmill is made.

> Notes:
> > 
>  - None of these methods are future proofed.  That is to say, when a new
>    version of mozmill (etc) is released, a person must manually change
>    code.  We should have a checklist for each product that tells what
>    needs to be changed on release or it probably won't get done.  I
>    offer to help develop this once these decisions get made.
What do you mean?  Using the above setup, when a new version of mozmill is available, I will:
* pacakge up the three components
* Do my normal release stuff that I do today: (Upload to pyPI, upload to AMO, Host the download on github etc.)
* Additionally, I check in the tarballs to hg, after getting review from folks like ted or catlee about it and running on try server (all the normal things you'd do before checking in a change to the test harnesses).

Because we will continue using setuptools.py, because we will continue supporting the installation, no code will need to change in either the makefile scripts or the buildbot scripts when a new version is installed.

The tests issue can be worked out later.  One of the other reasons the tests are in their own repo is that they are generalized tests and don't really map well to components in m-c.  One mozmill test can fit in many different places.  So, we will handle that later.  For now, the buildbot scripts can assume to find the mozmill tests as a tar.bz file in a well known location (such as: http://hg.mozilla.org/qa/test-agent/archive/tip.tar.bz2).  Once we have a final solution for the mozmill test files we will update that bit of buildslave code and we will complete the "make mozmill" integration (bug 562106 tracks all this).
"Run mozmill without installation.", sticking the untarred code as well as the tests into m-c directly would fit what we are doing for the other test suites right now.
(In reply to comment #60)
> (In reply to comment #59)
> I think you've just won the prize for "longest comment in bugzilla that is not
> a stack trace". ;)  Ok, so here's my proposal so we can close in on one
> decision here.

It took me a long time to research and as long to write.  Believe it or not, it was originally going to be at least double the size (and would have *still* been abridged).

> > (Abridged) Decision tree:
> > 
> > Run tests via makefile or only on buildslaves?
> Both.  We need to package mozmill into the make package-tests target. 
> Buildslaves do not have a build tree, they get their tests and test harnesses
> from that package so they will need to obtain the mozmill harness by
> uncompressing a downloaded test package.
> > |
> > +- via makefile: 
> > |    +-Install via virtualenv, PYTHONHOME, or run uninstalled?
> > |    |
> Install via virtualenv so that running make mozmill doesn't change your working
> python toolchain.

See comment 61 

> > |    +-Should the tests live in m-c or be mirrored there?
> > |    |
> TBD: you don't need to worry about this right now for buildbot stuff.  For the
> buildbot stuff, assume you will download a tar.gz of the mozmill tests.  This
> can be solved later when we complete the make mozmill functionality.

K; sounds good.

> >      +-How should the tarballs be created?
> python setuptools.py sdist is the easiest option and I don't see any reason to
> change that.  Three tarballs, one for each component.
+1 for that; probably have to do simplejson too, unless we're insisting on python 2.6
> > | 
> > +- on buildslaves: 
> >       +-Install w/ virtualenv, PYTHONHOME, or run uninstalled?
> >       |
> * Download the packaged tests tar.bz, unpack
> * Use virtualenv so that the buildslave python path is not changed 
> >       +-How should the tarballs be created?
> * setuptools.py sdist, uses the same tarballs that the make system uses (the
> make system just packages those mozmill tarballs into a distributable package
> with all the other tests.
> >       |
> >       +-Where should tarballs be kept?
> >         -> document this as well as who is responsible for generating
> In hg.  Specifically: hg.mozilla.org/mozilla-central/testing/mozmill/
> I'll be the person responsible for uploading these to hg when a new release to
> Mozmill is made.
> 
> > Notes:
> > > 
> >  - None of these methods are future proofed.  That is to say, when a new
> >    version of mozmill (etc) is released, a person must manually change
> >    code.  We should have a checklist for each product that tells what
> >    needs to be changed on release or it probably won't get done.  I
> >    offer to help develop this once these decisions get made.
> What do you mean?  Using the above setup, when a new version of mozmill is
> available, I will:
> * pacakge up the three components
> * Do my normal release stuff that I do today: (Upload to pyPI, upload to AMO,
> Host the download on github etc.)
> * Additionally, I check in the tarballs to hg, after getting review from folks
> like ted or catlee about it and running on try server (all the normal things
> you'd do before checking in a change to the test harnesses).
> 
> Because we will continue using setuptools.py, because we will continue
> supporting the installation, no code will need to change in either the makefile
> scripts or the buildbot scripts when a new version is installed.

This depends on how it is done;  My bash examples carefully avoided touching versions by using crazy string substitution.  Maybe this is okay.

> The tests issue can be worked out later.  One of the other reasons the tests
> are in their own repo is that they are generalized tests and don't really map
> well to components in m-c.  One mozmill test can fit in many different places. 
> So, we will handle that later.  For now, the buildbot scripts can assume to
> find the mozmill tests as a tar.bz file in a well known location (such as:
> http://hg.mozilla.org/qa/test-agent/archive/tip.tar.bz2).  Once we have a final
> solution for the mozmill test files we will update that bit of buildslave code
> and we will complete the "make mozmill" integration (bug 562106 tracks all
> this).

K, sounds good.
(In reply to comment #61)
> "Run mozmill without installation.", sticking the untarred code as well as the
> tests into m-c directly would fit what we are doing for the other test suites
> right now.

That falls apart though when you realize that the buildslaves are currently running python 2.5 and so we still need to import python libraries (outside of the mozmill libraries) and we'd have to essentially hand craft a virtualenv environment to manage that as Jeff was demonstrating in comment 59.  While it *can* certainly be done this way, I'd rather do something that is more easily maintained in the future and has much more intuitive coding than this.

I know it's a different paradigm than what we've done before, but I think that it's worthwhile.  A few months ago, our python test harnesses were basically very powerful shell scripts.  We have refactored (and continue to refactor) them into classes to make them more usable and more re-usable going forward.  In the same way, pioneering our use of virtualenv inside our buildslaves and using the standard python packaging systems can also help us future proof here and make inroads for code-reuse down the line.
(In reply to comment #61)
> "Run mozmill without installation.", sticking the untarred code as well as the
> tests into m-c directly would fit what we are doing for the other test suites
> right now.

Is there precedence for dealing with setuptools packages on buildslaves?
So, I agree with most of Clint's suggestion in comment 60.

If virtualenv.py and the mozmill code is contained within mozilla-central in some form or other, that makes things really simple for the buildbot automation.  All that needs to happen is for 'make packagetests' to create a relocatable virtualenv that gets packaged up with the rest of the tests.  This package gets uploaded to FTP alongside the build, and then downloaded by the test machines.  They merely have to run mozmill from the virtualenv to run the tests.

The actual format of mozmill in the source tree doesn't matter that much to us...My $0.02 is that it's cleaner to have just the source there instead of tarballs.  My original need for tarballs would go away because the exact versions are in the source tree, and would be packaged up with other tests.
an initial patch integrating mozmill into m-c is here: http://k0s.org/mozilla/hg/mozilla-central-patches/file/629b462bdb1e/bug-516984

This currently only works with `make package-tests`.  Working on extending.
(In reply to comment #65)
> So, I agree with most of Clint's suggestion in comment 60.
> 
> If virtualenv.py and the mozmill code is contained within mozilla-central in
> some form or other, that makes things really simple for the buildbot
> automation.  All that needs to happen is for 'make packagetests' to create a
> relocatable virtualenv that gets packaged up with the rest of the tests.  This
> package gets uploaded to FTP alongside the build, and then downloaded by the
> test machines.  They merely have to run mozmill from the virtualenv to run the
> tests.
> 
> The actual format of mozmill in the source tree doesn't matter that much to
> us...My $0.02 is that it's cleaner to have just the source there instead of
> tarballs.  My original need for tarballs would go away because the exact
> versions are in the source tree, and would be packaged up with other tests.

Sounds like a great way to move forward.  We'll work toward that.  Thanks Chris!
I've completed a first pass of integrating mozmill and the
mozilla-central build system.  See patches in

http://k0s.org/mozilla/hg/mozilla-central-patches/
http://k0s.org/mozilla/hg/mozilla-central-patches/file/8bfc082197c6/bug-516984

The patches are kinda hard to read due to the binary data, so if
anyone wants to see any of the particular files I've patched, lemme
know and I'll post them.  The functionality introduced:

 - the source code to mozmill and its deps + virtualenv have been
   added to m-c
 - `make package-tests` and `make mozmill` from MOZ_OBJDIR now do what
   I hope they're supposed to do
 - I added a bare-bones runmozmill.py.in that gets interpolated to
   runmozmill.py ;  The only thing I need to interpolate is the path
   to the browser, so maybe this is overkill? This script may
   certainly be extended if we want/need more of a front-end.

I haven't fully grokked the build system so there are a number of
things which are probably not preferable.

Currently, mozmill is only packaged (via `make package-tests`) to
$(DIST)/test-package-stage/mozmill . `make mozmill` depends on `stage-mozmill`
in testing/testuite-targets.mk . In other words, mozmill is run from
$(DIST)/test-package-stage/mozmill.  This feels odd and it isn't how
the other tests work, but I don't really grok how and where things are
supposed to go.  I'm not really sure of the intentions behind where
the build system puts tests.

One thing I thought of is to have a meta-target with a variable,
let's call it $(DEST), and then have two other targets for packaging
and in-situ testing (or whatever the intent is) which set $(DEST) to
different values (one of them $(DIST)/test-package/stage/mozmill, the
other one ???) and invoke the meta-target with these two values.
Though, again, as best I can tell this isn't how other tests do it,
though maybe mozmill+virtualenv is a different enough to warrant
separate treatment?

Or is the intention for what I am calling in-situ test running to keep
the code in the src directory?  In this case, maybe using `python
setup.py install` for `make package-tests` and `python setup.py
develop` for the other style of tests (making a virtualenv in both
cases).  The code itself (mozmill, mozrunner, jsbridge, etc) could
continue to live in tarballs or be unpacked (the latter avoids a
little logic, but probably not enough to be a factor).  Again, I'm not
sure if I have my head wrapped around how this should work, so outside
of saying "worksforme" probably the best thing I can do is explain how
I've been doing things and hope to defer to those that know the system
better. 

There is a stub in MOZ_OBJDIR/testing/mozmill with just the rules for
stage-package.  I suppose this is the way things should work?

Relatedly, perhaps, is getting the path to the binary.  This is done
in different ways throughout the makefiles.  (I can audit, if desired,
but I'm not sure if the intention differs or not).
http://mxr.mozilla.org/mozilla-central/source/build/automation-build.mk
has the most thorough treatment, and maybe I should just include
this.  If it were purely my code and I were doing this, I would break
off the browser-finding part of that file into its own file were I to
go down this route.  For now, I did something simpler:

BROWSER_PATH := $(shell $(PYTHON) -c 'import os; print os.path.abspath("$(DIST)/bin/$(MOZ_APP_NAME)$(BIN_SUFFIX)")')

This worksforme.  The shell call using python to find the absolute
path is necessary as, while BROWSER_PATH is correct where it is
interpolated from (MOZ_OBJDIR/testing/mozmill/Makefile), it is not
where it is intepolated to
(MOZ_OBJDIR/dist/test-package-stage/mozmill/runmozmill.py)
. Alternatively, if the target in $(DIST)/test-package-stage is the
only one we care about, I can just interpret relative to the file
(../../../$(MOZ_APP_NAME)$(BIN_SUFFIX) or the like).
Worth noting:  the existing mozmill tests touch many sites (e.g. amazon.com, verisign.com, google, etc).  If it is the position that buildbot should not touch non mozilla-net, then what should be done here?

Also, currently only the firefox and firefox restart tests are run which are both in http://hg.mozilla.org/qa/mozmill-tests/file/tip/firefox ; however, the other directories (scripts, addons, shared-modules, templates) are also included.  Do we need these? Or > /dev/null ?
(In reply to comment #69)
> Worth noting:  the existing mozmill tests touch many sites (e.g. amazon.com,
> verisign.com, google, etc).  If it is the position that buildbot should not
> touch non mozilla-net, then what should be done here?

I remember somebody blogging about a proxy that records what sites a browser hits, and caches the results so it can replay them later.  Maybe we could use that?
(In reply to comment #69)
> Worth noting:  the existing mozmill tests touch many sites (e.g. amazon.com,
> verisign.com, google, etc).  If it is the position that buildbot should not
> touch non mozilla-net, then what should be done here?

Correct. We have already some pages moved to our repository but to be able to have everything stored locally we need bug 563748 fixed.

> Also, currently only the firefox and firefox restart tests are run which are
> both in http://hg.mozilla.org/qa/mozmill-tests/file/tip/firefox ; however, the
> other directories (scripts, addons, shared-modules, templates) are also
> included.  Do we need these? Or > /dev/null ?

You will only need shared-modules. The other ones are not relevant to the tests we wanna run on buildbot. Please keep in mind that this repository is using named branches. So using tip is not the best way here. See https://developer.mozilla.org/en/Mozmill_Tests#Handling_Branches
(In reply to comment #71)
> (In reply to comment #69)
> > Worth noting:  the existing mozmill tests touch many sites (e.g. amazon.com,
> > verisign.com, google, etc).  If it is the position that buildbot should not
> > touch non mozilla-net, then what should be done here?
> 
> Correct. We have already some pages moved to our repository but to be able to
> have everything stored locally we need bug 563748 fixed.

K.  I'll wait on this one then.

> 
> > Also, currently only the firefox and firefox restart tests are run which are
> > both in http://hg.mozilla.org/qa/mozmill-tests/file/tip/firefox ; however, the
> > other directories (scripts, addons, shared-modules, templates) are also
> > included.  Do we need these? Or > /dev/null ?
> 
> You will only need shared-modules. The other ones are not relevant to the tests
> we wanna run on buildbot. Please keep in mind that this repository is using
> named branches. So using tip is not the best way here. See
> https://developer.mozilla.org/en/Mozmill_Tests#Handling_Branches

I just gave the 'tip' URL for reference.  The tests were exported from the tip default branch sometime last week.  To my knowledge, propagating changes upstream has not been addressed.
(In reply to comment #70)
> 
> I remember somebody blogging about a proxy that records what sites a browser
> hits, and caches the results so it can replay them later.  Maybe we could use
> that?

I've been wanting to find out what proxy this is but none of us seem to know.
I believe I've used http-replicator for this in the past:
http://sourceforge.net/projects/http-replicator/
(In reply to comment #70)
> (In reply to comment #69)
> > Worth noting:  the existing mozmill tests touch many sites (e.g. amazon.com,
> > verisign.com, google, etc).  If it is the position that buildbot should not
> > touch non mozilla-net, then what should be done here?
> 
> I remember somebody blogging about a proxy that records what sites a browser
> hits, and caches the results so it can replay them later.  Maybe we could use
> that?

So way back up there, I mentioned that we weren't going to run all the tests in hg.mozilla.org/qa/mozmill-tests as part of the buildbot automation.  And these specifically were the tests that I was thinking about when I made that statement.  I think we can plan to resolve that within the tests themselves and inside mozmill (i.e. a test manifest!) and since skipping these tests will be handled by the tool, buildbot will not need to worry about this.
So the two big changes that seem to be in order right now (outside of whatever needs to be done to the tests themselves):

 - unpacking the tarballs and installing them that way
 - finding the firefox browser path in a way compatible with more operating systems (a la http://mxr.mozilla.org/mozilla-central/source/build/automation-build.mk)

Outside of this, and making the tests local (however), is there anything else that is needed to get this side ready for buildbot?  If keeping mozmill in $(DIST) is fine (at least for now), then that is the path of least resistance.
(In reply to comment #76)
> So the two big changes that seem to be in order right now (outside of whatever
> needs to be done to the tests themselves):
> 
>  - unpacking the tarballs and installing them that way

Done.

>  - finding the firefox browser path in a way compatible with more operating
> systems (a la
> http://mxr.mozilla.org/mozilla-central/source/build/automation-build.mk)

Done.  My original intention was to `include` automation-build.mk directly into the mozmill Makefile.  However, due to the fact that the string is pre-quoted, both core_abspath and the previous way I was using (python -c "import os; os.path.abspath(...)") had problems interpolating the string.  So I created a new file, build/browser-path.mk whose entire purpose is to get the BROWSER_PATH (and the ABSOLUTE_BROWSER_PATH) in the makefiles.  I cut off the -bin from program (i.e. firefox vs. firefox-bin) as the latter did not work when running runmozmill.py.  I'm not sure where/if it would be needed if this script was used as general purpose.  I suggest that the logic in build/automation-build.mk consume is replaced by including build/browser-path.mk.  This might also be used to replace other places where (e.g.) $(DIST)/bin/$(MOZ_APP_NAME)$(SUFFIX) is used in the makefiles.  Worth ticketing
(In reply to comment #77)
> So I created a
> new file, build/browser-path.mk whose entire purpose is to get the BROWSER_PATH
> (and the ABSOLUTE_BROWSER_PATH) in the makefiles.

Can I suggest you use app-path.mk - we normally try and keep browser specific references out of core code as its not just FF that is likely to use it...
(In reply to comment #78)
> (In reply to comment #77)
> > So I created a
> > new file, build/browser-path.mk whose entire purpose is to get the BROWSER_PATH
> > (and the ABSOLUTE_BROWSER_PATH) in the makefiles.
> 
> Can I suggest you use app-path.mk - we normally try and keep browser specific
> references out of core code as its not just FF that is likely to use it...

Done: 

http://k0s.org/mozilla/hg/mozilla-central-patches/rev/ccc592048781
http://k0s.org/mozilla/hg/mozilla-central-patches/rev/b4d13ebe2934
So, we are ready to land these changes for mozmill support in m-c.

I'm proposing we land this in two pieces:
* Piece 1: Jeff's changes to makefiles and his new supporting files for mozmill, because this is mostly make * test and test packaging stuff, we're going to ask Ted to review that.
* Piece 2: A wholesale move of the mozmill code from its other repositories into this one.  We'll be moving version 1.4.1 of the mozmill libraries into the tree. The move of all the mozmill code is a giant one.  Does anyone really want to review that or should we just land it as it is very much NPOTB right now.
these are the changes to mozilla-central minus the python packages (virtualenv, mozmill, jsbridge, simplejson, mozrunner)
Attachment #444120 - Flags: review?(ted.mielczarek)
This is the import of the Mozmill library and all its dependencies.  It's too big to attach as a patch (even compressed):
http://people.mozilla.org/~ctalbert/mozmill/mozmill_import2.diff

* Mozrunner version 2.4.2
* Jsbridge version 2.3.5
* Mozmill version 1.4.1

And the external dependencies:
* simplejson version 2.1.1
* virtualenv version 1.4.8

This is a 4mb patch.  All of the Mozmill code has been reviewed previously, so I don't know if review here is strictly necessary.  I also feel really bad about saddling someone with a 4Mb patch.  Are there any takers?
Comment on attachment 444120 [details] [diff] [review]
patch to mozilla-central without python packages

>+# The Initial Developer of the Original Code is
>+# Mozilla.org.

The initial developer is "the Mozilla Foundation". You should update that for all instances.

>+# mozmill options
>+OPTIONS = [ '--showall', '-b', options.binary ]
>+OPTIONS.extend(args) # add command line arguments
>+
>+# commands to be executed
>+MOZMILL_COMMAND = [ MOZMILL ] + OPTIONS + [ '-t', FIREFOX_TESTS ]
>+MOZMILL_RESTART_COMMAND = [ MOZMILL ] + OPTIONS + [ '-t',
>+                                                    FIREFOX_RESTART_TESTS ]
>+
>+# execute the command
>+code = 0
>+for command in MOZMILL_COMMAND, MOZMILL_RESTART_COMMAND:
>+  code += subprocess.call(command)

While I'm writing the scripts for our QA release testing I have seen that using the mozmill class directly is much easier. I will let you know once those scripts are ready or check bug 562027 on yourself.
Clint: there's no point in anyone trying to review a 4MB patch that's just "import the current mozmill + dependencies". We'll review the integration parts, and you can just land the import when the other parts are ready.
How are subsequent updates of mozmill going to be handled?
@Ted: yeah, that was my thought as well, but I thought it ought to be made available on the bug in case anyone is insane enough to try it. :)

@Al: Subsequent updates to Mozmill will be rolled in as new versions are released.  The version in m-c will be handled like any of our other external libraries.  So, updating this will become part of the Mozmill release process.
Attachment #444120 - Flags: review?(ted.mielczarek) → review-
Comment on attachment 444120 [details] [diff] [review]
patch to mozilla-central without python packages

>diff --git a/Makefile.in b/Makefile.in
>--- a/Makefile.in
>+++ b/Makefile.in
>@@ -74,16 +74,17 @@ ifdef COMPILE_ENVIRONMENT
> include $(topsrcdir)/$(MOZ_BUILD_APP)/build.mk
> endif
> 
> TIERS += testharness
> 
> # test harnesses
> ifdef ENABLE_TESTS
> tier_testharness_dirs += testing/xpcshell
>+tier_testharness_dirs += testing/mozmill

While you're here, can you just remove this entire tier_testharness bit (and the TIERS += testharnesses), and just put both of these dirs into toolkit-tiers where mochitest gets built:
http://mxr.mozilla.org/mozilla-central/source/toolkit/toolkit-tiers.mk#263

>diff --git a/build/app-path.mk b/build/app-path.mk
>new file mode 100644
>--- /dev/null
>+++ b/build/app-path.mk
>+# include this file to find the path to firefox 

This is a fine place to consolidate this logic. Could you also refactor automation-build.mk to include this file instead of having the same logic duplicated?
http://mxr.mozilla.org/mozilla-central/source/build/automation-build.mk

>diff --git a/testing/mozmill/Makefile.in b/testing/mozmill/Makefile.in
>new file mode 100644
>--- /dev/null
>+++ b/testing/mozmill/Makefile.in
>+MODULE		= testing_mozmill
>+
>+include $(topsrcdir)/config/rules.mk
>+
>+# simplejson is not a dependency for python 2.6
>+# see http://github.com/mikeal/jsbridge/blob/master/setup.py
>+PYTHON26 = $(shell python -c 'import sys; print sys.version.startswith("2.6")')

You'll want to call $(PYTHON) instead of just 'python' in makefiles. ($(PYTHON) is the path to the Python detected by configure.)

>+ifneq ($(PYTHON26),True)
>+TEST_HARNESS_FILES := simplejson-2.1.1
>+endif

This is a little ugly. Is it harmful to package simplejson even if we don't need it for Python 2.6?

>+
>+# Harness files from the srcdir
>+# python packages to be installed IN INSTALLATION ORDER

Is the installation order note because of the way that `pip install` works? Would be good to note that explicitly.

>+TEST_HARNESS_FILES += \

These aren't really files, they're package directories, right? I'd probably name it _PACKAGES or something.

>+ifdef MOZ_CRASHREPORTER
>+TEST_HARNESS_COMPONENTS += crashreporter_test.xpt
>+endif

Do you actually need this for mozmill tests, or are you just copying this from the other harness dirs?

>+stage-package:
>+	$(NSINSTALL) -D $(PKG_STAGE)/mozmill/tests
>+	@(python $(srcdir)/virtualenv/virtualenv.py $(MOZMILL_VENV))

What's with the extra set of enclosing parentheses? (They don't actually do anything, do they?)

>+	@(. $(MOZMILL_VENV)/bin/activate && cd $(srcdir) && pip install $(TEST_HARNESS_FILES))
>+	@(python $(srcdir)/virtualenv/virtualenv.py --relocatable $(MOZMILL_VENV))
>+	@(cd $(srcdir) && tar $(TAR_CREATE_FLAGS) - $(TEST_FILES)) | (cd $(PKG_STAGE)/mozmill && tar -xf -)
>+	$(PYTHON) $(topsrcdir)/config/Preprocessor.py \
>+	-DBROWSER_PATH="$(ABSOLUTE_APP_PATH)" \
>+	$(srcdir)/runmozmill.py.in > $(MOZMILL_VENV)/runmozmill.py

What's the point of preprocessing this script if it only happens in the stage-package step? The only reason we preprocess scripts like runtests.py is so that developers can run them directly from their objdir (like python _tests/testing/mochitest/runtests.py) without providing the path to the app binary. With packaged tests we explicitly pass the path to the app binary to the script (--appname=/path/to/firefox).

>diff --git a/testing/mozmill/runmozmill.py.in b/testing/mozmill/runmozmill.py.in
>new file mode 100644
>--- /dev/null
>+++ b/testing/mozmill/runmozmill.py.in

Generally, I like all of our Python scripts to avoid doing things at the top-level scope, and instead use the if __name__ == '__main__': main() design.


>+# parse options
>+# XXX could use the parser already in mozmill
>+parser = OptionParser(description=__doc__)
>+parser.add_option('-b', '--binary', default=FIREFOX_BIN,
>+                  help="Binary path.")

For consistency with our other test harnesses, you should probably use --appname. (I guess this is an impedance mismatch with Mozmill, which is unfortunate.)

>+
>+# commands to be executed
>+MOZMILL_COMMAND = [ MOZMILL ] + OPTIONS + [ '-t', FIREFOX_TESTS ]
>+MOZMILL_RESTART_COMMAND = [ MOZMILL ] + OPTIONS + [ '-t',
>+                                                    FIREFOX_RESTART_TESTS ]
>+
>+# execute the command
>+code = 0
>+for command in MOZMILL_COMMAND, MOZMILL_RESTART_COMMAND:
>+  code += subprocess.call(command)

That's a little..weird.

>+# exit with the sum of the status code
>+sys.exit(code) # XXX is this adequate for error-reporting?

Returning anything non-zero is probably fine. Personally I'd just sys.exit(1) if anything failed.

r- for some fixes, but overall the concept looks good.

I note that this doesn't provide any way to run the tests in an objdir (for example, if a developer wanted to run the tests against their local changes). Is that by design?
I've separated my patch into two components in my patch queue for easy auditing:
 - the packages for mozmill, mozrunner, jsbridge, virtualenv, and the tests (that is, things taken from other sources)
 - my actual modifications to the build system

See http://k0s.org/mozilla/hg/mozilla-central-patches/
See Also: → 563748
> (From update of attachment 444120 [details] [diff] [review])
> >diff --git a/Makefile.in b/Makefile.in
> >--- a/Makefile.in
> >+++ b/Makefile.in
> >@@ -74,16 +74,17 @@ ifdef COMPILE_ENVIRONMENT
> > include $(topsrcdir)/$(MOZ_BUILD_APP)/build.mk
> > endif
> > 
> > TIERS += testharness
> > 
> > # test harnesses
> > ifdef ENABLE_TESTS
> > tier_testharness_dirs += testing/xpcshell
> >+tier_testharness_dirs += testing/mozmill
> 
> While you're here, can you just remove this entire tier_testharness bit (and
> the TIERS += testharnesses), and just put both of these dirs into toolkit-tiers
> where mochitest gets built:
> http://mxr.mozilla.org/mozilla-central/source/toolkit/toolkit-tiers.mk#263
 
You mean like this?
http://k0s.org/mozilla/hg/mozilla-central-patches/rev/dbb0dd448f0f 
and
http://k0s.org/mozilla/hg/mozilla-central-patches/rev/0e5262aa3b6f
Honestly, I'm not sure what any of these mean.  What are these and why
is it better to have them in toolkit-tiers.mk ?

> >diff --git a/build/app-path.mk b/build/app-path.mk
> >new file mode 100644
> >--- /dev/null
> >+++ b/build/app-path.mk
> >+# include this file to find the path to firefox 
> 
> This is a fine place to consolidate this logic. Could you also refactor
> automation-build.mk to include this file instead of having the same logic
> duplicated?
> http://mxr.mozilla.org/mozilla-central/source/build/automation-build.mk
 
There are two differences in the files:  automation-build.mk quotes
the string for `browser_path` whereas `APP_PATH` is unquoted.  This is
very deliberate as I could not get `runmozmill.py.in` to correctly
deal with the interpolation when it was a string.  Another difference
is that automation-build.mk appends `-bin` to non-windows
architecture.  mozmill will not correctly run as is using firefox-bin.

automation-build.mk is used in 4 places (see
http://mxr.mozilla.org/mozilla-central/search?string=automation-build.mk ).

However, if we only care about the package-tests, then both
runmozmill.py and the associated interpolation wherein app-path.mk is
used can probably go away entirely (see below).  It may be worth
abstracting the APP_PATH stuff anyway.  Should I file a bug?

> >diff --git a/testing/mozmill/Makefile.in b/testing/mozmill/Makefile.in
> >new file mode 100644
> >--- /dev/null
> >+++ b/testing/mozmill/Makefile.in
> >+MODULE		= testing_mozmill
> >+
> >+include $(topsrcdir)/config/rules.mk
> >+
> >+# simplejson is not a dependency for python 2.6
> >+# see http://github.com/mikeal/jsbridge/blob/master/setup.py
> >+PYTHON26 = $(shell python -c 'import sys; print sys.version.startswith("2.6")')
>
> You'll want to call $(PYTHON) instead of just 'python' in makefiles. ($(PYTHON)
> is the path to the Python detected by configure.)

This code is now gone.  See next comment.  I did fix the other places
where I used just `python` to now use `$(PYTHON)`
 
> >+ifneq ($(PYTHON26),True)
> >+TEST_HARNESS_FILES := simplejson-2.1.1
> >+endif
> 
> This is a little ugly. Is it harmful to package simplejson even if we don't
> need it for Python 2.6?

Installing it doesn't seem to cause any trouble.  I've made this
change at
http://k0s.org/mozilla/hg/mozilla-central-patches/rev/d10120b21299 . I
can keep it this way, if desired.
 
> >+
> >+# Harness files from the srcdir
> >+# python packages to be installed IN INSTALLATION ORDER
> 
> Is the installation order note because of the way that `pip install` works?
> Would be good to note that explicitly.

Not `pip install` per se, but dependency order.  jsbridge depends on
simplejson and mozrunner, so those need to be installed first, and
mozmill depends on jsbridge.  Hence the order.  Ideally, pip would
consider the packages specified on the command line and inspect them
before inspecting dependencies.  But it doesn't.  So if you aren't
careful about keeping the correct order here, pip will attempt to
install package dependencies from the net instead of from the local
files.  Alternate text welcome.

> >+TEST_HARNESS_FILES += \
> 
> These aren't really files, they're package directories, right? I'd probably
> name it _PACKAGES or something.

Done.
 
> >+ifdef MOZ_CRASHREPORTER
> >+TEST_HARNESS_COMPONENTS += crashreporter_test.xpt
> >+endif
> 
> Do you actually need this for mozmill tests, or are you just copying this from
> the other harness dirs?

Just copying.  I have no idea what this does, to be honest.  Can you
shed any light?  Should it be removed?
 
> >+stage-package:
> >+	$(NSINSTALL) -D $(PKG_STAGE)/mozmill/tests
> >+	@(python $(srcdir)/virtualenv/virtualenv.py $(MOZMILL_VENV))
> 
> What's with the extra set of enclosing parentheses? (They don't actually do
> anything, do they?)

Again, this is cargo-culted.  I don't believe they do anything in make
except act as delimeters.  Should I get rid of them?  I'm not sure if
I need to suppress echoing with the `@` either.

I also fixed the python vs. $(PYTHON) references that remained:

http://k0s.org/mozilla/hg/mozilla-central-patches/rev/2d8131df35fc

> >+	@(. $(MOZMILL_VENV)/bin/activate && cd $(srcdir) && pip install $(TEST_HARNESS_FILES))
> >+	@(python $(srcdir)/virtualenv/virtualenv.py --relocatable $(MOZMILL_VENV))
> >+	@(cd $(srcdir) && tar $(TAR_CREATE_FLAGS) - $(TEST_FILES)) | (cd $(PKG_STAGE)/mozmill && tar -xf -)
> >+	$(PYTHON) $(topsrcdir)/config/Preprocessor.py \
> >+	-DBROWSER_PATH="$(ABSOLUTE_APP_PATH)" \
> >+	$(srcdir)/runmozmill.py.in > $(MOZMILL_VENV)/runmozmill.py
> 
> What's the point of preprocessing this script if it only happens in the
> stage-package step? The only reason we preprocess scripts like runtests.py is
> so that developers can run them directly from their objdir (like python
> _tests/testing/mochitest/runtests.py) without providing the path to the app
> binary. With packaged tests we explicitly pass the path to the app binary to
> the script (--appname=/path/to/firefox).

After discussion with :ted and :ctalbert, it was decided that running
outside of packaged tests is unnecessary for this bug.  I'll follow up
with additional information on this, a revised patch that removes
anything not needed for packaged tests, and a follow-up bug to run
mozmill in the object directory.

So, in conclusion, for now there is no point.

> >diff --git a/testing/mozmill/runmozmill.py.in b/testing/mozmill/runmozmill.py.in
> >new file mode 100644
> >--- /dev/null
> >+++ b/testing/mozmill/runmozmill.py.in
> 
> Generally, I like all of our Python scripts to avoid doing things at the
> top-level scope, and instead use the if __name__ == '__main__': main() design.

Normally, I agree.  In this case, there's really nothing consumable in
this file and it's only usable via the command line.  I implemented
this in
http://k0s.org/mozilla/hg/mozilla-central-patches/rev/015186cadf85 ;
not sure what much this buys.
 
But again, for the packaged tests, this script probably isn't needed
at all.

> >+# parse options
> >+# XXX could use the parser already in mozmill
> >+parser = OptionParser(description=__doc__)
> >+parser.add_option('-b', '--binary', default=FIREFOX_BIN,
> >+                  help="Binary path.")
> 
> For consistency with our other test harnesses, you should probably use
> --appname. (I guess this is an impedance mismatch with Mozmill, which is
> unfortunate.)

I'm glad you know about impedance mismatches.  My electrical
engineering years have not gone to waste.  Done, http://k0s.org/mozilla/hg/mozilla-central-patches/rev/7248109a1d0f
 
> >+
> >+# commands to be executed
> >+MOZMILL_COMMAND = [ MOZMILL ] + OPTIONS + [ '-t', FIREFOX_TESTS ]
> >+MOZMILL_RESTART_COMMAND = [ MOZMILL ] + OPTIONS + [ '-t',
> >+                                                    FIREFOX_RESTART_TESTS ]
> >+
> >+# execute the command
> >+code = 0
> >+for command in MOZMILL_COMMAND, MOZMILL_RESTART_COMMAND:
> >+  code += subprocess.call(command)
> 
> That's a little..weird.

Any idea how to make this better?  I could see three options here and
went with the one i disliked the least:

 - the script could err out on first failure. In this case,
   mozmill-restart won't run if mozmill fails

 - the script could be called twice from the makefile, one for each of
   mozmill and mozmill-restart

 - the script could take some combination of the exit codes and exit
   with them.  Here, I just sum them (presumedly '2' normally means
   they both fail, since they normaly exit with '1' currently).  But I
   could use logic like `code = subprocess.call(command) and 1 or code`
   which would fit your sys.exit(1) recommendation below.

(Again, probably an obselete issue as this script can probably go away.)

> >+# exit with the sum of the status code
> >+sys.exit(code) # XXX is this adequate for error-reporting?
> 
> Returning anything non-zero is probably fine. Personally I'd just sys.exit(1)
> if anything failed.

Cool.

> r- for some fixes, but overall the concept looks good.

> I note that this doesn't provide any way to run the tests in an objdir (for
> example, if a developer wanted to run the tests against their local changes).
> Is that by design?

Currently, the tests only run in the packaged-tests directory (running
`make mozmill`).  The only reason I did not include running this in
situ is because I'm not really sure how to do that (yet) and it has
been implied this wasn't important.  After discussion with :ted and
:ctalbert, it sounds like this is a separate issue (see above).
The scope of this bug is limited to getting mozmill working on
buildbot.  Therefore, the for getting mozmill into mozilla-central
(the first step of this bug, the second being adding the code to
buildbot to run the mozmill tests), it is only necessary to get
mozmill working from the packaged tests.  Currently, the state of the
patch implements functionality unnecessary to this.  `make mozmill`
provides a way to run the tests, but it is dependent on `make
package-tests` step.

Making `make mozmill` work correctly, and in a way consistent with the
other tests, will be moved to a separate bug.  This bug will be
updated with a link to this other bug once filed.

Currently, the patch to the build system (ignoring the addition of
mozmill + dependencies, virtualenv, and the tests from their canonical
sources) consists of the following files:

Makefile.in
build/app-path.mk
testing/mozmill/Makefile.in
testing/mozmill/README.txt
testing/mozmill/runmozmill.py.in
testing/testsuite-targets.mk
toolkit/toolkit-tiers.mk

For the packaged tests, runmozmill.py.in is not required at all.
Since app-path.mk is only interpolated for this file, it can probably
go away too (although, ideally, this should be abstracted from
automation-build.mk and the latter consume it, but it is not needed
for this purpose).  `testing/mozmill/Makefile.in` should be updated to
account for these changes, and the mozmill target may be removed from
testsuite-targets.mk. The rest can remain.

I'll make these changes and upload a revised (truncated) patch and
follow up with a bug tracking running mozmill from the object
directory as a distinct issue.
only have mozmill in packaged tests; changes as per https://bugzilla.mozilla.org/show_bug.cgi?id=516984#c90
Attachment #444120 - Attachment is obsolete: true
Attachment #447580 - Flags: review?(ted.mielczarek)
(In reply to comment #89)
> > What's with the extra set of enclosing parentheses? (They don't actually do
> > anything, do they?)
>
> Again, this is cargo-culted.  I don't believe they do anything in make
> except act as delimeters.  Should I get rid of them?  I'm not sure if
> I need to suppress echoing with the `@` either.

They're not interpreted by make at all, make just passes that all down to the shell, so it's just like typing (foo) at the shell, which is equivalent to 'foo' by itself. Get rid of them.

@ echo suppressing is a matter of choice. It's obviously useful for @echo foo, but if you're running actual commands you might want to drop it, since it makes it easier to see what commands are failing in build logs.

I think I answered all the other questions via IRC.
Comment on attachment 447580 [details] [diff] [review]
build modifications to mozilla-central

>diff --git a/testing/mozmill/Makefile.in b/testing/mozmill/Makefile.in
>new file mode 100644
>--- /dev/null
>+++ b/testing/mozmill/Makefile.in
>+# Harness packages from the srcdir;
>+# python packages to be installed IN INSTALLATION ORDER

I asked you to clarify this, but you asked me a question in return and I didn't respond. I think you should add something like "packages later in the list can depend only on packages earlier in the list."

>diff --git a/toolkit/toolkit-tiers.mk b/toolkit/toolkit-tiers.mk
>--- a/toolkit/toolkit-tiers.mk
>+++ b/toolkit/toolkit-tiers.mk
>@@ -254,12 +254,18 @@ endif
> ifdef MOZ_LEAKY
> tier_platform_dirs        += tools/leaky
> endif
> 
> ifdef MOZ_MAPINFO
> tier_platform_dirs	+= tools/codesighs
> endif
> 
>+# test harnesses
>+
>+TIERS += testharness
>+

Get rid of this, mochitest is already in tier_platform, so it doesn't seem necessary.

> ifdef ENABLE_TESTS
> tier_platform_dirs	+= testing/mochitest
>+tier_testharness_dirs += testing/xpcshell 
>+tier_testharness_dirs += testing/mozmill

and make these both tier_platform_dirs.

r=me with those changes (+ the minor tweaks from my previous comment)
Attachment #447580 - Flags: review?(ted.mielczarek) → review+
(In reply to comment #92)
> (In reply to comment #89)
> > > What's with the extra set of enclosing parentheses? (They don't actually do
> > > anything, do they?)
> >
> > Again, this is cargo-culted.  I don't believe they do anything in make
> > except act as delimeters.  Should I get rid of them?  I'm not sure if
> > I need to suppress echoing with the `@` either.
> 
> They're not interpreted by make at all, make just passes that all down to the
> shell, so it's just like typing (foo) at the shell, which is equivalent to
> 'foo' by itself. Get rid of them.

I stole this from http://mxr.mozilla.org/mozilla-central/source/testing/xpcshell/Makefile.in which seems to have the same issues.  I wasn't sure if enclosing parens were passed to shell or if they had a special meaning to make.  Thanks for the clarification.

> @ echo suppressing is a matter of choice. It's obviously useful for @echo foo,
> but if you're running actual commands you might want to drop it, since it makes
> it easier to see what commands are failing in build logs.

Done so.

> I think I answered all the other questions via IRC.

Fixed:
http://k0s.org/mozilla/hg/mozilla-central-patches/rev/10e0fad404ac
(In reply to comment #93)
> (From update of attachment 447580 [details] [diff] [review])
> >diff --git a/testing/mozmill/Makefile.in b/testing/mozmill/Makefile.in
> >new file mode 100644
> >--- /dev/null
> >+++ b/testing/mozmill/Makefile.in
> >+# Harness packages from the srcdir;
> >+# python packages to be installed IN INSTALLATION ORDER
> 
> I asked you to clarify this, but you asked me a question in return and I didn't
> respond. I think you should add something like "packages later in the list can
> depend only on packages earlier in the list."

Done, http://k0s.org/mozilla/hg/mozilla-central-patches/rev/93a870432e8c

> 
> >diff --git a/toolkit/toolkit-tiers.mk b/toolkit/toolkit-tiers.mk
> >--- a/toolkit/toolkit-tiers.mk
> >+++ b/toolkit/toolkit-tiers.mk
> >@@ -254,12 +254,18 @@ endif
> > ifdef MOZ_LEAKY
> > tier_platform_dirs        += tools/leaky
> > endif
> > 
> > ifdef MOZ_MAPINFO
> > tier_platform_dirs	+= tools/codesighs
> > endif
> > 
> >+# test harnesses
> >+
> >+TIERS += testharness
> >+
> 
> Get rid of this, mochitest is already in tier_platform, so it doesn't seem
> necessary.
> 
> > ifdef ENABLE_TESTS
> > tier_platform_dirs	+= testing/mochitest
> >+tier_testharness_dirs += testing/xpcshell 
> >+tier_testharness_dirs += testing/mozmill
> 
> and make these both tier_platform_dirs.

Done, http://k0s.org/mozilla/hg/mozilla-central-patches/rev/78631196458e
 
> r=me with those changes (+ the minor tweaks from my previous comment)

Thanks!  Will upload the new patch and obselete the old one
update of the patch with :ted 's recommendations
Attachment #447580 - Attachment is obsolete: true
Blocks: 568642
(In reply to comment #90)
> The scope of this bug is limited to getting mozmill working on
> buildbot.  Therefore, the for getting mozmill into mozilla-central
> (the first step of this bug, the second being adding the code to
> buildbot to run the mozmill tests), it is only necessary to get
> mozmill working from the packaged tests.  Currently, the state of the
> patch implements functionality unnecessary to this.  `make mozmill`
> provides a way to run the tests, but it is dependent on `make
> package-tests` step.
> 
> Making `make mozmill` work correctly, and in a way consistent with the
> other tests, will be moved to a separate bug.  This bug will be
> updated with a link to this other bug once filed.
> 
> Currently, the patch to the build system (ignoring the addition of
> mozmill + dependencies, virtualenv, and the tests from their canonical
> sources) consists of the following files:
> 
> Makefile.in
> build/app-path.mk
> testing/mozmill/Makefile.in
> testing/mozmill/README.txt
> testing/mozmill/runmozmill.py.in
> testing/testsuite-targets.mk
> toolkit/toolkit-tiers.mk
> 
> For the packaged tests, runmozmill.py.in is not required at all.
> Since app-path.mk is only interpolated for this file, it can probably
> go away too (although, ideally, this should be abstracted from
> automation-build.mk and the latter consume it, but it is not needed
> for this purpose).  `testing/mozmill/Makefile.in` should be updated to
> account for these changes, and the mozmill target may be removed from
> testsuite-targets.mk. The rest can remain.
> 
> I'll make these changes and upload a revised (truncated) patch and
> follow up with a bug tracking running mozmill from the object
> directory as a distinct issue.

The additional step necessary to run `make mozmill` has been noted as bug 568642
Comment on attachment 447815 [details] [diff] [review]
build modifications to mozilla-central

Jhammel forgot to carry Ted's r+ forward.
Attachment #447815 - Flags: review+
(In reply to comment #94)
> I stole this from
> http://mxr.mozilla.org/mozilla-central/source/testing/xpcshell/Makefile.in
> which seems to have the same issues.  I wasn't sure if enclosing parens were
> passed to shell or if they had a special meaning to make.  Thanks for the
> clarification.

Right, they're used there because those lines are of the form:
(cd foo && tar cf - *) | (cd bar && tar xf -)
Where we're tarring the contents of one directory to stdout, then back from stdin to another directory.
removes extraneous trailing slash which fails on windows builds (linux collapses multiple slashes)
Attachment #447815 - Attachment is obsolete: true
Attachment #447878 - Flags: review+
Pushing to the try server, there is some issue:

"""
if test -d obj-firefox/ppc/dist/test-package-stage -a                 \
                -d obj-firefox/i386/dist/test-package-stage; then              \
           cp obj-firefox/ppc/dist/test-package-stage/mochitest/automation.py \
             obj-firefox/i386/dist/test-package-stage/mochitest/;              \
           cp obj-firefox/ppc/dist/test-package-stage/reftest/automation.py   \
             obj-firefox/i386/dist/test-package-stage/reftest/;                \
           /builds/slave/tryserver-macosx/build/build/macosx/universal/unify                 \
             --unify-with-sort "all-test-dirs\.list$"               \
             obj-firefox/ppc/dist/test-package-stage                          \
             obj-firefox/i386/dist/test-package-stage                          \
             obj-firefox/ppc/dist/universal/test-package-stage; fi
/builds/slave/tryserver-macosx/build/build/macosx/universal/unify: copyIfIdentical: files differ:
  obj-firefox/ppc/dist/test-package-stage/mozmill/bin/activate,
  obj-firefox/i386/dist/test-package-stage/mozmill/bin/activate
make[1]: *** [postflight_all] Error 1
make: *** [build] Error 2
program finished with exit code 2
"""

I'm not exactly exactly sure what this error really means.  Obviously the files differ, but why is that a show-stopper?
For OS X, we build a universal binary. This means that we also build a universal test package to go along with it. the 'unify' script is what we use to build those universal packages. It has some pretty simple rules:
1) Mach-O binaries get lipo'ed into universal binaries
2) Other plain files must be identical (because we are only going to ship one copy)
3) zip files are identical if they have the same list of files with the same sizes and checksums
4) Directories are processed recursively
5) As a special case, filenames matching a pattern passed to the script as --unify-with-sort are considered equal if their contents are the same after sorting the lines alphabetically.
(In reply to comment #102)
> For OS X, we build a universal binary. This means that we also build a
> universal test package to go along with it. the 'unify' script is what we use
> to build those universal packages. It has some pretty simple rules:
> 1) Mach-O binaries get lipo'ed into universal binaries
> 2) Other plain files must be identical (because we are only going to ship one
> copy)
> 3) zip files are identical if they have the same list of files with the same
> sizes and checksums
> 4) Directories are processed recursively
> 5) As a special case, filenames matching a pattern passed to the script as
> --unify-with-sort are considered equal if their contents are the same after
> sorting the lines alphabetically.

Thanks for the info, Ted.  Since the `activate` script is generated according to the given version of python, it does not surprise me that the two would be different for different platforms.  In light of this, I think a better solution is to not generated the virtualenv on stage-package and to push this off to buildbot (or the developer) but to have a script to do this that will be run by e.g. buildbot.  

I think this actually solves a few conceptual problems that I've only recently become aware of:
 - virtualenv symlinks the .pyc files and not just the .py files from system.  This is probably bad if the python interpreters don't match.  The above approach ensures the python versions will match
 - currently, there is only the stage-package target in the Makefile.in . If make mozmill is to work in the standard way (see https://bugzilla.mozilla.org/show_bug.cgi?id=568642), this should make it easy to abstract this into three dummy targets:  a master (e.g. install-mozmill) and two sub-targets (stage-package and a new one) that just set a variable and call install-mozmill to that directory

If we go start using virtualenv for more stuff than mozmill, it should probably live somewhere else (e.g. in build), but for now it can live where it's at.  Patch forthcoming
version that does not actually create a virtualenv, just packages the software needed to create one and adds a courtesy install script.  Notably, this script is pretty generic and has little/nothing to do with mozmill specifically.

This should solve the previous issues of files differening
Attachment #447878 - Attachment is obsolete: true
Attachment #448104 - Flags: review?(ted.mielczarek)
(In reply to comment #104)
> Created an attachment (id=448104) [details]
> build modifications to mozilla-central, does not install the packages, just
> copies them
> 
> version that does not actually create a virtualenv, just packages the software
> needed to create one and adds a courtesy install script.  Notably, this script
> is pretty generic and has little/nothing to do with mozmill specifically.
> 
> This should solve the previous issues of files differening

Pushed to try.  While the patch doesn't actually do much (all it does is include and package the tests), things seem to pass.   There is burning on maemo and some oranges, but they look unrelated as best i can tell from the logs. Downloading the packaged tests, mozmill is there and it installs and runs fine.
Comment on attachment 448104 [details] [diff] [review]
build modifications to mozilla-central, does not install the packages, just copies them

>diff --git a/testing/mozmill/installmozmill.py b/testing/mozmill/installmozmill.py
>new file mode 100755
>--- /dev/null
>+++ b/testing/mozmill/installmozmill.py
>+  # packages to install in dependency order
>+  PACKAGES=file('PACKAGES').read().split()

Is this better than .readlines()? (I'm honestly curious.)
Attachment #448104 - Flags: review?(ted.mielczarek) → review+
(In reply to comment #106)
> (From update of attachment 448104 [details] [diff] [review])
> >diff --git a/testing/mozmill/installmozmill.py b/testing/mozmill/installmozmill.py
> >new file mode 100755
> >--- /dev/null
> >+++ b/testing/mozmill/installmozmill.py
> >+  # packages to install in dependency order
> >+  PACKAGES=file('PACKAGES').read().split()
> 
> Is this better than .readlines()? (I'm honestly curious.)

I don't know about "better", but the intention is different.  The PACKAGES file looks like this:

(mozmill)> cat PACKAGES 
simplejson-2.1.1 mozrunner-2.4.2 jsbridge-2.3.5 mozmill-1.4.1

So it's not splitting over newlines, it's splitting over all whitespace (that is, if you had one per line or however many per line, it would work the same).
I could also have done .readline().split() for this one-line case and it would have been the same
PS: readlines() leaves the newline whitespace intact, which you'd have to strip, if this was in multiple lines.
(In reply to comment #108)
> PS: readlines() leaves the newline whitespace intact, which you'd have to
> strip, if this was in multiple lines.

it does, and i'd also have to check for empty lines, as even for a one-line file, there is often an ending empty line if human-touched.  so as usual laziness is why i did what i did
Status: NEW → ASSIGNED
Checked in to mozilla-central (thanks, ctalbert!):

http://hg.mozilla.org/mozilla-central/rev/d0f822ada75f
http://hg.mozilla.org/mozilla-central/rev/7d5c3c3647c3

If the commit goes fine, next up, the buildbot side of things.  Then, we're done :)
The previous commit to mozilla-central does not work on the buildbot platform due to a subtle bug (or API change?) in python.  Namely, popping the environment key still has it remain in a subprocess.  The following corrects this deficiency
This patch seems to work for buildbotcustom, ignoring the obvious caveats like hitting external servers and the tests not all passing
Attachment #439973 - Attachment is obsolete: true
Summary of progress:

As suggested in comment 112 and comment 113, the basic infrastructure is getting there.  What is left to do?  

Immediately:
 - add the patch from comment 112 to mozilla-central;  an unfortunate side-effect of having mozmill (etc) live in m-c is that patches to m-c must occur before buildbot can be reliably tested
 - confirm the patch from 113 works (basically); add this to buildbot-custom if it won't break anything

Next:
 - look at all the failing tests; decide what to do about them
 - look at all the tests that go off mozilla.org net.  decide what to do about them
Attachment #448872 - Flags: review?(ted.mielczarek)
Attachment #448872 - Flags: review?(ted.mielczarek) → review+
Attachment #448875 - Flags: review?(jhford)
(In reply to comment #113)
> Created an attachment (id=448875) [details]
> run mozmill via buildbot from packaged tests
> 
> This patch seems to work for buildbotcustom, ignoring the obvious caveats like
> hitting external servers and the tests not all passing

With https://bugzilla.mozilla.org/attachment.cgi?id=448872 landed in m-c, the patch to buildbotcustom now works on the CentOS VM I'm using, which is to say that it gets to and executes the mozmill and mozmill-restart tests.  

However, Minefield hangs on test execution:

"""
...
DEBUG:mozmill:Test Pass: {'function': 'Controller.waitForElement()'}
DEBUG:mozmill:Test Pass: {'function': 'Controller.click()'}
DEBUG:mozmill:Test Pass: {'function': 'Controller.waitForElement()'}
DEBUG:mozmill:Test Pass: {'function': 'Controller.click()'}
DEBUG:mozmill:Test Pass: {'function': 'Controller.check(ID: showWhenDownloading, state: false)'}
DEBUG:mozmill:Test Pass: {'function': 'Controller.keypress()'}
DEBUG:mozmill:Test Pass: {'function': 'Controller.open()'}
DEBUG:mozmill:Test Pass: {'function': 'Controller.waitForElement()'}
DEBUG:mozmill:Test Pass: {'function': 'Controller.click()'}
DEBUG:mozmill:Test Pass: {'function': 'Controller.waitForElement()'}

command timed out: 600 seconds without output, killing pid 7967
process killed by signal 9
program finished with exit code -1
elapsedTime=701.153780
"""

The browser itself goes gray (does not refresh itself) and is unresponsive.

In addition, this causes the restart tests not to run:

"""
bin/python bin/mozmill-restart --showall -b /builds/testor/slave/full/build/firefox/firefox -t tests/firefox/restartTests
...
(Gecko:8074): GnomeUI-WARNING **: While connecting to session manager:
Authentication Rejected, reason : None of the authentication protocols specified are supported and host-based authentication failed.

(Gecko:8074): GnomeUI-WARNING **: While connecting to session manager:
Authentication Rejected, reason : None of the authentication protocols specified are supported and host-based authentication failed.
jsbridge: Exception: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIServerSocket.init]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: file:///tmp/tmpvgVBh4.mozrunner/extensions/jsbridge@mozilla.com/resource/modules/server.js :: anonymous :: line 274"  data: no]

command timed out: 300 seconds without output, killing pid 8065
process killed by signal 9
program finished with exit code -1
elapsedTime=302.664390
"""

I am not sure why this happens.  I have not seen this behaviour outside of the VM.
includes a larger timeout and corrects paths for the restart tests
Attachment #448875 - Attachment is obsolete: true
Attachment #450217 - Flags: review?(jhford)
Attachment #448875 - Flags: review?(jhford)
(In reply to comment #115)

This is not a bug in the patch included here nor a buildbot problem
Depends on: 571630
Adding blocking of bug 571630 to this bug;  as this bug has become more of a beginning to end tracking bug, the "mystical stopping" issue should be corrected before tests are run in production.  It does not block any actionable parts of this bug, e.g. landing on buildbotcustom
This bug actually covers several issues.  They weren't known to be separate issues at the time, so they all got lumped in here.  We could break this bug up into separate issues ([DONE] package mozmill tests in m-c, add MozmillFactory to buildbotcustom, use Mozmill on production buildbots).

Unless we do that, I'll use this as a tracking bug for these three issues. The immediate roadmap is now presented:

Roadmap:

 - [DONE] working packaged tests have landed on mozilla-central:
   https://bug516984.bugzilla.mozilla.org/attachment.cgi?id=448104

 - a MozmillFactory should be written that integrates mozmill into
   buildbot:
   https://bug516984.bugzilla.mozilla.org/attachment.cgi?id=450217
   - currently this is blocked by
     https://bugzilla.mozilla.org/show_bug.cgi?id=557336

 - the mozmill tests should be passing; the passing tests should be
   committed to m-c simultaneously with whatever stable fixes to
   momzill+jsbridge+mozrunner

 - the MozmillFactory should be tested on the staging buildbot server

 - the MozmillFactory should be turned on in production

What Needs to Happen:

 - need to do the math to figure out how much resources are needed for
   production testing:

   test run time * platforms * branches * numbers of builds per branch
   per day

   The test run time (end to end via buildbot + MozmillFactory) is

   I would recommend running mozmill on all standard platforms:
         linux, linux64, win32, macosx, macosx-snow
   Possibly more than this (android, win64, etc), but 5 is a good
   starting number.

   For branches, I would be content to start initially with
   mozilla-central.  It is my inclination to start off with one known
   target and then expand as the utility is needed.

   This gives:

    15 min * 5 platforms * 1 branch = 1.25 machine hours / push

 - the mozmill tests should not hit external URLs;  currently, most of
   the mozmill tests hit external URLs.
   - for initial production deployment, the expedient solution is to
     cut all tests from mozilla-central that hit external URLs
   v-post production-v
   - mozmill should not be bundled with http.js; instead, it should
     make use of http.js in the same way as the other packaged tests.
     In general, treating mozmill as close to what existing harness
     infrastructure we have so that the can be collectively curated
   - a patch should be made to run mozmill w/ http.js (at least in
     m-c, maybe in mozmill too)
   - script(s) should be written to convert live mozmill tests to
     static tests; this *could* initially be done by hand, but
     doing by hand probably isn't maintainable or at least cheap to
     maintain
   - existing off-site tests (and maybe even mozilla URLs) should be
     converted to locally served tests

Blockers:

 - Mozmill focus (?) issue:
   https://bugzilla.mozilla.org/show_bug.cgi?id=571630 ; this should
   be corrected before the tests are run in production

Potential Slowdowns:

 - migration of tests from http://hg.mozilla.org/qa/mozmill-tests/ to
   m-c : currently, the test source lives at
   http://hg.mozilla.org/qa/mozmill-tests/ ; however, to make it into
   the packaged tests, these tests (or some subset of them) should be

Potential Speedups:

 - in general, refactorings should not block quarter goals.  A
   refactoring should be identified as blocking or non-blocking of
   quarterly goal bugs.  If they are blocking, they should be
   addressed as a high priority.

My Immediate Priorities:

 - in order to expedite the process to deployment, I will look at
   several issues that must be addressed:

   - the mozmill hanging + focus issue: bug 571630

   - having mozmill follow the same test output structure as
     e.g. mochitest: bug 573589

   - whatever I can do to help on the UnittestPackagedBuildFactory
     refactor: bug 557336

   - testing the setup from end to end as best possible
Depends on: 573589
> My Immediate Priorities:
> 
>  - in order to expedite the process to deployment, I will look at
>    several issues that must be addressed:

Also:

   - get a set of working tests;  prune those not that hit non mozilla urls
>    - script(s) should be written to convert live mozmill tests to
>      static tests; this *could* initially be done by hand, but
>      doing by hand probably isn't maintainable or at least cheap to
>      maintain

Really, this is a manual step we have to do. There are needs for local test data files which you can't create automatically. We already have a couple of those files and more will be created beginning of next quarter. It will be one of the top priorities for my team to get those Mozmill tests use local test data.

>    - existing off-site tests (and maybe even mozilla URLs) should be
>      converted to locally served tests

That's part of the last bullet point. But lets further discuss that topic on bug 562106.
(In reply to comment #121)
> >    - script(s) should be written to convert live mozmill tests to
> >      static tests; this *could* initially be done by hand, but
> >      doing by hand probably isn't maintainable or at least cheap to
> >      maintain
> 
> Really, this is a manual step we have to do. There are needs for local test
> data files which you can't create automatically. We already have a couple of
> those files and more will be created beginning of next quarter. It will be one
> of the top priorities for my team to get those Mozmill tests use local test
> data.

There are tests which can and should exist purely as local files.  Here, I am just talking about the tests that are canonically on live web data but that you want to run locally, basically to ensure a known set of good tests (the pages don't change, servers won't go down, etc).
(In reply to comment #122)
> There are tests which can and should exist purely as local files.  Here, I am
> just talking about the tests that are canonically on live web data but that you
> want to run locally, basically to ensure a known set of good tests (the pages
> don't change, servers won't go down, etc).

Those tests will not be part of our bft/fft test suite.
Depends on: 574043
Instead of reimplementing many of the common tasks, uses the proposed class from bug 557336 .
Attachment #455155 - Flags: review?(jhford)
No longer depends on: 574043
Comment on attachment 450217 [details] [diff] [review]
run mozmill via buildbot from packaged tests

As discussed, we would like to use a base class instead of duplicating UnittestPackagedBuildFactory logic in a new class.
Attachment #450217 - Flags: review?(jhford) → review-
(In reply to comment #125)
> Comment on attachment 450217 [details] [diff] [review]
> run mozmill via buildbot from packaged tests
> 
> As discussed, we would like to use a base class instead of duplicating
> UnittestPackagedBuildFactory logic in a new class.

see attachment https://bugzilla.mozilla.org/attachment.cgi?id=455155 which does this
update of patch compatible with https://bugzilla.mozilla.org/attachment.cgi?id=461278 on bug 557336
Attachment #450217 - Attachment is obsolete: true
Attachment #455155 - Attachment is obsolete: true
Attachment #461282 - Flags: review?(jhford)
Attachment #455155 - Flags: review?(jhford)
update to patch noting rename of class from https://bugzilla.mozilla.org/attachment.cgi?id=461391
Attachment #461282 - Attachment is obsolete: true
Attachment #461392 - Flags: review?(jhford)
Attachment #461282 - Flags: review?(jhford)
Just a status update: the main blocker for this bug is bug 557336.  One of the patches there, or variation thereon, is tentatively scheduled to land next week during a releng downtime.  Once that is done, a new patch can be easily constructed here (if necessary, but likely) and then hopefully we can move to landing this in buildbotcustom.
Comment on attachment 461392 [details] [diff] [review]
run mozmill via buildbot inheriting from MozillaPackagedTest factory

out of date patch.  will post a new one soon
Attachment #461392 - Flags: review?(jhford)
Attached patch mozmill + buildbot patch (obsolete) — Splinter Review
This successfully runs the needed mozmill commands via buildbot.   the mozmill run is not successful due to https://bugzilla.mozilla.org/show_bug.cgi?id=585086 but this should be landed such that mozmill may be turned on when the software and the tests are updated and reliably green
Attachment #472674 - Flags: review?(jhford)
Comment on attachment 472674 [details] [diff] [review]
mozmill + buildbot patch

>diff --git a/misc.py b/misc.py
>+        # Mozmill tests
>+        if pf.get('enable_mozmill_tests') and pf.get('enable_opt_unittests'):
>+            mozmill_factory = MozmillFactory(
>+                platform=platform,
>+                productName=config['product_name'],
>+                hgHost=config['hghost'],
>+                repoPath=config['repo_path'],
>+                buildToolsRepoPath=config['build_tools_repo_path'],
>+                buildSpace=0.5,
>+                buildsBeforeReboot=pf['builds_before_reboot'],
>+                )
>+            mozmill_builder = {
>+                'name': '%s test mozmill' % pf['base_name'],
>+                'slavenames': pf['slaves'],
>+                'builddir': '%s-%s-mozmill' % (name, platform),
>+                'factory': mozmill_factory,
>+                'category': name,
>+                }
>+            branchObjects['builders'].append(mozmill_builder)

This bit of code is in the generateBranchObjects section.  This function is only run for builder masters.  I am not sure if there is a dependency of running on a machine capable of running builds, but there is a general movement towards having tests run only on the test pool.  To do this, you'd want to modify generateTalosBranchObjects

                   config_repo_path=config['config_repo_path'],
>diff --git a/process/factory.py b/process/factory.py
>--- a/process/factory.py
>+++ b/process/factory.py
>@@ -6383,16 +6383,25 @@ class MozillaTestFactory(MozillaBuildFac
>             ))
> 
>         def get_exedir(build):
>             return os.path.dirname(build.getProperty('exepath'))
>         self.addStep(SetBuildProperty(
>          property_name="exedir",
>          value=get_exedir,
>         ))
>+        def get_absolute_exepath(build):
>+            return os.path.join(build.getProperty('basedir'),
>+                                'build',
>+                                build.getProperty('exepath'),
>+                                )

If this code is executed on the master (which I believe it is), the os.path.join will return a string based on the master's os instead of the slave's os.  Msys should let this work but string joining would avoid the subtleties of where the function is executed.

>+        self.addStep(SetBuildProperty(
>+            property_name="absolute_exepath",
>+            value=get_absolute_exepath
>+            )) 

If this property is the basedir property and exepath joined in a cosistent manner (i.e. always unix path joining as above), couldn't we use WithProperties("%(basedir)s/%(exepath)s")?
       
>+class MozmillFactory(MozillaTestFactory):
>+    """factory to run Mozmill tests"""
>+
>+    def __init__(self, platform, env=None, productName='firefox', **kwargs):
>+
>+        # environment
>+        if env is None:
>+            env = MozillaEnvironments['%s-unittest' % platform].copy()
>+        env['MOZ_CRASHREPORTER_NO_REPORT'] = '1'
>+        if 'NO_EM_RESTART' in env:
>+            del self.env['NO_EM_RESTART']
>+
>+        # intialize parent class
>+        MozillaTestFactory.__init__(self, platform,
>+                                    productName,
>+                                    downloadTests=True,
>+                                    downloadSymbols=False,
>+                                    **kwargs)
>+
>+        # set platform-dependent python variables
>+        if self.platform == "win32":
>+            self.addStep(SetBuildProperty(
>+             property_name="pythonpath",
>+             value="Scripts\\python",
>+             name="set_pythonpath",
>+            ))
>+            self.addStep(SetBuildProperty(
>+             property_name="mozmillpath",
>+             value="Scripts\\mozmill-script.py",
>+             name="set_mozmillpath",
>+                ))
>+            self.addStep(SetBuildProperty(
>+             property_name="mozmillrestart",
>+             value="Scripts\\mozmill-restart-script.py",
>+             name="set_mozmillrestart_path"
>+            ))
>+
>+        else:
>+            self.addStep(SetBuildProperty(
>+             property_name="pythonpath",
>+             value="bin/python",
>+             name="set_pythonpath",
>+            ))
>+            self.addStep(SetBuildProperty(
>+             property_name="mozmillpath",
>+             value="bin/mozmill",
>+             name="set_mozmillpath",
>+            ))
>+            self.addStep(SetBuildProperty(
>+                property_name="mozmillrestart",
>+                value="bin/mozmill-restart",
>+                name="set_mozmillrestart_path",
>+                ))

If these are hardcoded, non-runtime dependent strings, why not use factory arguments and factory logic instead of properties?

>+        # set absolute path of the mozmill environment's root
>+        self.addStep(SetProperty(
>+            name='set_mozmillenv',
>+            command=['pwd'],
>+            property='mozmillenv',
>+            workdir='build/mozmill',
>+            ))

Relative paths don't work?

>+
>+        # install mozmill into its virtualenv
>+        self.addStep(ShellCommand(
>+            name='install mozmill',
>+            command=['python',
>+                     os.path.join('mozmill', 'installmozmill.py')],'

This is definitely master side os.path.join, which is going to do it based on the master's os instead of the slave's os, can we use string joining to make this subtle issue more noticeable?


>+        # run mozmill tests
>+        self.addStep(unittest_steps.MozmillStep(
>+            name="run_mozmill",
>+            mozmill=WithProperties('%(mozmillpath)s'),
>+            tests_dir='tests/firefox',
>+            env={'PYTHONHOME': WithProperties('%(mozmillenv)s')},
>+            workdir=WithProperties('%(mozmillenv)s'),
>+            timeout=10*60,
>+            flunkOnFailure=True
>+            ))
>+        self.addStep(unittest_steps.MozmillStep(
>+            name="run_mozmill_restart",
>+            mozmill=WithProperties('%(mozmillrestart)s'),
>+            tests_dir='tests/firefox/restartTests',
>+            env={'PYTHONHOME': WithProperties('%(mozmillenv)s')},
>+            workdir=WithProperties('%(mozmillenv)s'),
>+            timeout=5*60,
>+            flunkOnFailure=True
>+            ))
>+

This logic should really be in the self.addSetupSteps() and self.addRunTestSteps().  As it is right now, the reboot steps will occur before any of the setup or testing occurs because the steps are added after self.addTearDownSteps() is called.
Attachment #472674 - Flags: review?(jhford) → review-
Thanks for the quick review.  I'll probably have more questions or comments as I investigate these issues, but just a few things at first.


> >+        self.addStep(SetBuildProperty(
> >+            property_name="absolute_exepath",
> >+            value=get_absolute_exepath
> >+            )) 
> 
> If this property is the basedir property and exepath joined in a cosistent
> manner (i.e. always unix path joining as above), couldn't we use
> WithProperties("%(basedir)s/%(exepath)s")?

It would be WithProperties("%(basedir)s/build/%(exepath)s"), but in theory.  Not sure why that would be preferable.

> >+        # set platform-dependent python variables
> >+        if self.platform == "win32":
> >+            self.addStep(SetBuildProperty(
> >+             property_name="pythonpath",
> >+             value="Scripts\\python",
> >+             name="set_pythonpath",
> >+            ))
> >+            self.addStep(SetBuildProperty(
> >+             property_name="mozmillpath",
> >+             value="Scripts\\mozmill-script.py",
> >+             name="set_mozmillpath",
> >+                ))
> >+            self.addStep(SetBuildProperty(
> >+             property_name="mozmillrestart",
> >+             value="Scripts\\mozmill-restart-script.py",
> >+             name="set_mozmillrestart_path"
> >+            ))
> >+
> >+        else:
> >+            self.addStep(SetBuildProperty(
> >+             property_name="pythonpath",
> >+             value="bin/python",
> >+             name="set_pythonpath",
> >+            ))
> >+            self.addStep(SetBuildProperty(
> >+             property_name="mozmillpath",
> >+             value="bin/mozmill",
> >+             name="set_mozmillpath",
> >+            ))
> >+            self.addStep(SetBuildProperty(
> >+                property_name="mozmillrestart",
> >+                value="bin/mozmill-restart",
> >+                name="set_mozmillrestart_path",
> >+                ))
> 
> If these are hardcoded, non-runtime dependent strings, why not use factory
> arguments and factory logic instead of properties?

I'm not sure what is meant here.  Care to elaborate your suggestion?

> >+        # set absolute path of the mozmill environment's root
> >+        self.addStep(SetProperty(
> >+            name='set_mozmillenv',
> >+            command=['pwd'],
> >+            property='mozmillenv',
> >+            workdir='build/mozmill',
> >+            ))
> 
> Relative paths don't work?

To override PYTHONHOME?  I doubt it, but could be wrong:

http://k0s.org/mozilla/hg/buildbotcustom-patches/file/3f5f085160b5/bug-516984-2#l209
>>diff --git a/misc.py b/misc.py
>>+        # Mozmill tests
>>+        if pf.get('enable_mozmill_tests') and pf.get('enable_opt_unittests'):
>>+            mozmill_factory = MozmillFactory(
>>+                platform=platform,
>>+                productName=config['product_name'],
>>+                hgHost=config['hghost'],
>>+                repoPath=config['repo_path'],
>>+                buildToolsRepoPath=config['build_tools_repo_path'],
>>+                buildSpace=0.5,
>>+                buildsBeforeReboot=pf['builds_before_reboot'],
>>+                )
>>+            mozmill_builder = {
>>+                'name': '%s test mozmill' % pf['base_name'],
>>+                'slavenames': pf['slaves'],
>>+                'builddir': '%s-%s-mozmill' % (name, platform),
>>+                'factory': mozmill_factory,
>>+                'category': name,
>>+                }
>>+            branchObjects['builders'].append(mozmill_builder)

>This bit of code is in the generateBranchObjects section.  This function is
>only run for builder masters.  I am not sure if there is a dependency of
>running on a machine capable of running builds, but there is a general movement
>towards having tests run only on the test pool.  To do this, you'd want to
>modify generateTalosBranchObjects

Okay, I'll try to make this modification if that is what is desired.  I'm a bit perplexed looking at the code though, but I'll continue to look through it.
(In reply to comment #135)
> >>diff --git a/misc.py b/misc.py
> >>+        # Mozmill tests
> >>+        if pf.get('enable_mozmill_tests') and pf.get('enable_opt_unittests'):
> >>+            mozmill_factory = MozmillFactory(
> >>+                platform=platform,
> >>+                productName=config['product_name'],
> >>+                hgHost=config['hghost'],
> >>+                repoPath=config['repo_path'],
> >>+                buildToolsRepoPath=config['build_tools_repo_path'],
> >>+                buildSpace=0.5,
> >>+                buildsBeforeReboot=pf['builds_before_reboot'],
> >>+                )
> >>+            mozmill_builder = {
> >>+                'name': '%s test mozmill' % pf['base_name'],
> >>+                'slavenames': pf['slaves'],
> >>+                'builddir': '%s-%s-mozmill' % (name, platform),
> >>+                'factory': mozmill_factory,
> >>+                'category': name,
> >>+                }
> >>+            branchObjects['builders'].append(mozmill_builder)
> 
> >This bit of code is in the generateBranchObjects section.  This function is
> >only run for builder masters.  I am not sure if there is a dependency of
> >running on a machine capable of running builds, but there is a general movement
> >towards having tests run only on the test pool.  To do this, you'd want to
> >modify generateTalosBranchObjects
> 
> Okay, I'll try to make this modification if that is what is desired.  I'm a bit
> perplexed looking at the code though, but I'll continue to look through it.

Also, short of :anode getting the staging buildbot up and working, I have no sensible way of testing misc.py.  My staging setup does not test this code:  http://k0s.org/mozilla/hg/TestMozillaBuildbot/ . Is there a sensible way of testing this code?
If you want a staging environment on your laptop you can run:

virtualenv test-env
source test-env/bin/activate
easy_install twisted==10
easy_install pyasn1
easy_install pycrypto
easy_install pyopenssl
easy_install argparse
hg clone http://hg.mozilla.org/build/buildbotcustom
hg up -r buildbot-0.8.0 -R buildbotcustom
hg clone http://hg.mozilla.org/build/buildbot
(cd buildbot/master && python setup.py install)
hg clone http://hg.mozilla.org/build/buildbot-configs
(cd test-env/lib/python2.6/site-packages/ && ln -s ../../../../buildbotcustom) #this mightn't be right
cd buildbot-configs
./setup-master.py -8 test-master staging-tests_master2

At this point you should be able to buildbot checkconfig which will instantiate any objects (including factories) that you are creating.  You can print things out, or alternatively, you could start the buildbot master up.  This master's http port is 8010 and slave port is 9010.
(In reply to comment #134)
> It would be WithProperties("%(basedir)s/build/%(exepath)s"), but in theory. 
> Not sure why that would be preferable.

The properties are tracked per run and add overhead to the overloaded test masters.  Not sure if it is significant.

> > >+        # set platform-dependent python variables
> > >+        if self.platform == "win32":
> > >+            self.addStep(SetBuildProperty(
> > >+             property_name="pythonpath",
> > >+             value="Scripts\\python",
> > >+             name="set_pythonpath",
> > >+            ))
> > >+            self.addStep(SetBuildProperty(
> > >+             property_name="mozmillpath",
> > >+             value="Scripts\\mozmill-script.py",
> > >+             name="set_mozmillpath",
> > >+                ))
> > >+            self.addStep(SetBuildProperty(
> > >+             property_name="mozmillrestart",
> > >+             value="Scripts\\mozmill-restart-script.py",
> > >+             name="set_mozmillrestart_path"
> > >+            ))
> > >+
> > >+        else:
> > >+            self.addStep(SetBuildProperty(
> > >+             property_name="pythonpath",
> > >+             value="bin/python",
> > >+             name="set_pythonpath",
> > >+            ))
> > >+            self.addStep(SetBuildProperty(
> > >+             property_name="mozmillpath",
> > >+             value="bin/mozmill",
> > >+             name="set_mozmillpath",
> > >+            ))
> > >+            self.addStep(SetBuildProperty(
> > >+                property_name="mozmillrestart",
> > >+                value="bin/mozmill-restart",
> > >+                name="set_mozmillrestart_path",
> > >+                ))
> > 
> > If these are hardcoded, non-runtime dependent strings, why not use factory
> > arguments and factory logic instead of properties?
> 
> I'm not sure what is meant here.  Care to elaborate your suggestion?

something like:

if 'win' in self.platform:
  self.pythonpath = 'Scripts\\python'
else:
  self.pythonpath = 'bin/python'

to set it the value in the factory, then you can consume it in MozmillStep with something like: 

class MozmillStep(ShellCommandReportTimeout):
  def __init__(self, mozmill, tests_dir, pythonpath, **kwargs):
    <snip>
    self.addFactoryArguments(mozmill=mozmill, tests_dir=tests_dir,
                             pythonpath=pythonpath)
    self.command = [pythonpath, mozmill, '--showall',
    <snip>

Alternately, you could pass the platform into the step and make those decisions in the step.

> 
> > >+        # set absolute path of the mozmill environment's root
> > >+        self.addStep(SetProperty(
> > >+            name='set_mozmillenv',
> > >+            command=['pwd'],
> > >+            property='mozmillenv',
> > >+            workdir='build/mozmill',
> > >+            ))
> > 
> > Relative paths don't work?
> 
> To override PYTHONHOME?  I doubt it, but could be wrong:
> 
> http://k0s.org/mozilla/hg/buildbotcustom-patches/file/3f5f085160b5/bug-516984-2#l209

wfm,
jhford-wifi:test-for-mozmill jhford$ export PYTHONHOME=~/test-for-mozmill/test-env
jhford-wifi:test-for-mozmill jhford$ python
Python 2.6.1 (r261:67515, Feb 11 2010, 00:51:29) 
[GCC 4.2.1 (Apple Inc. build 5646)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import buildbotcustom
>>> buildbotcustom.__file__
'/Users/jhford/test-for-mozmill/test-env/lib/python2.6/site-packages/buildbotcustom/__init__.pyc'
>>>

(In reply to comment #135)
> >>diff --git a/misc.py b/misc.py
> >This bit of code is in the generateBranchObjects section.  This function is
> >only run for builder masters.  I am not sure if there is a dependency of
> >running on a machine capable of running builds, but there is a general movement
> >towards having tests run only on the test pool.  To do this, you'd want to
> >modify generateTalosBranchObjects
> 
> Okay, I'll try to make this modification if that is what is desired.  I'm a bit
> perplexed looking at the code though, but I'll continue to look through it.

Yes, this is complicated code.  If you have a working factory and step I can take a look at fitting it into misc.py
Comment on attachment 461285 [details]
buildbot master.cfg used for testing, for reference

My new master.cfg is somewhat different. Can upload if needed
Attachment #461285 - Attachment is obsolete: true
Attachment #461285 - Attachment mime type: application/octet-stream → text/plain
Attachment #461392 - Attachment is obsolete: true
Attached patch mozmill + buildbot patch (obsolete) — Splinter Review
Addresses most issues from comment 133 ; does not move from generateBranchObjects to generateTalosBranchObjects so its not ready for review yet, but putting it here for reference
Attachment #472674 - Attachment is obsolete: true
So I've put up attachment 473287 [details] [diff] [review] which addresses all of these issues except the generateBranchObjects -> generateTalosBranchObjects . Comments inline:

(In reply to comment #133)
> Comment on attachment 472674 [details] [diff] [review]
> mozmill + buildbot patch
> 
> >diff --git a/misc.py b/misc.py
> >+        # Mozmill tests
> >+        if pf.get('enable_mozmill_tests') and pf.get('enable_opt_unittests'):
> >+            mozmill_factory = MozmillFactory(
> >+                platform=platform,
> >+                productName=config['product_name'],
> >+                hgHost=config['hghost'],
> >+                repoPath=config['repo_path'],
> >+                buildToolsRepoPath=config['build_tools_repo_path'],
> >+                buildSpace=0.5,
> >+                buildsBeforeReboot=pf['builds_before_reboot'],
> >+                )
> >+            mozmill_builder = {
> >+                'name': '%s test mozmill' % pf['base_name'],
> >+                'slavenames': pf['slaves'],
> >+                'builddir': '%s-%s-mozmill' % (name, platform),
> >+                'factory': mozmill_factory,
> >+                'category': name,
> >+                }
> >+            branchObjects['builders'].append(mozmill_builder)
> 
> This bit of code is in the generateBranchObjects section.  This function is
> only run for builder masters.  I am not sure if there is a dependency of
> running on a machine capable of running builds, but there is a general movement
> towards having tests run only on the test pool.  To do this, you'd want to
> modify generateTalosBranchObjects

Working on it.

>                    config_repo_path=config['config_repo_path'],
> >diff --git a/process/factory.py b/process/factory.py
> >--- a/process/factory.py
> >+++ b/process/factory.py
> >@@ -6383,16 +6383,25 @@ class MozillaTestFactory(MozillaBuildFac
> >             ))
> > 
> >         def get_exedir(build):
> >             return os.path.dirname(build.getProperty('exepath'))
> >         self.addStep(SetBuildProperty(
> >          property_name="exedir",
> >          value=get_exedir,
> >         ))
> >+        def get_absolute_exepath(build):
> >+            return os.path.join(build.getProperty('basedir'),
> >+                                'build',
> >+                                build.getProperty('exepath'),
> >+                                )
> 
> If this code is executed on the master (which I believe it is), the
> os.path.join will return a string based on the master's os instead of the
> slave's os.  Msys should let this work but string joining would avoid the
> subtleties of where the function is executed.

If by string joining meaning verbosely using '/'s, that is now what is universally done in this patch.

> >+        self.addStep(SetBuildProperty(
> >+            property_name="absolute_exepath",
> >+            value=get_absolute_exepath
> >+            )) 
> 
> If this property is the basedir property and exepath joined in a cosistent
> manner (i.e. always unix path joining as above), couldn't we use
> WithProperties("%(basedir)s/%(exepath)s")?

I didn't realize that build properties were particularly expensive.  Currently, I'm using exactly one, that being the absolute path of the executable wrt mozmill.  Since it differs by platform (no '-bin' suffix on linux currently, though its in the works) a property seemed the easiest way to do it.  I removed this code and put it in directly where I needed it.

> >+class MozmillFactory(MozillaTestFactory):
> >+    """factory to run Mozmill tests"""
> >+
> >+    def __init__(self, platform, env=None, productName='firefox', **kwargs):

> >+        # environment
> >+        if env is None:
> >+            env = MozillaEnvironments['%s-unittest' % platform].copy()
> >+        env['MOZ_CRASHREPORTER_NO_REPORT'] = '1'
> >+        if 'NO_EM_RESTART' in env:
> >+            del self.env['NO_EM_RESTART']
I killed this section.  ABICT, I don't need it.

> >+        # intialize parent class
> >+        MozillaTestFactory.__init__(self, platform,
> >+                                    productName,
> >+                                    downloadTests=True,
> >+                                    downloadSymbols=False,
> >+                                    **kwargs)
> >+
> >+        # set platform-dependent python variables
> >+        if self.platform == "win32":
> >+            self.addStep(SetBuildProperty(
> >+             property_name="pythonpath",
> >+             value="Scripts\\python",
> >+             name="set_pythonpath",
> >+            ))
> >+            self.addStep(SetBuildProperty(
> >+             property_name="mozmillpath",
> >+             value="Scripts\\mozmill-script.py",
> >+             name="set_mozmillpath",
> >+                ))
> >+            self.addStep(SetBuildProperty(
> >+             property_name="mozmillrestart",
> >+             value="Scripts\\mozmill-restart-script.py",
> >+             name="set_mozmillrestart_path"
> >+            ))
> >+
> >+        else:
> >+            self.addStep(SetBuildProperty(
> >+             property_name="pythonpath",
> >+             value="bin/python",
> >+             name="set_pythonpath",
> >+            ))
> >+            self.addStep(SetBuildProperty(
> >+             property_name="mozmillpath",
> >+             value="bin/mozmill",
> >+             name="set_mozmillpath",
> >+            ))
> >+            self.addStep(SetBuildProperty(
> >+                property_name="mozmillrestart",
> >+                value="bin/mozmill-restart",
> >+                name="set_mozmillrestart_path",
> >+                ))
> 
> If these are hardcoded, non-runtime dependent strings, why not use factory
> arguments and factory logic instead of properties?

Again, I wasn't aware these were particularly expensive.  It is now done that way.

> >+        # set absolute path of the mozmill environment's root
> >+        self.addStep(SetProperty(
> >+            name='set_mozmillenv',
> >+            command=['pwd'],
> >+            property='mozmillenv',
> >+            workdir='build/mozmill',
> >+            ))
> 
> Relative paths don't work?

They do. Fixed.

> >+
> >+        # install mozmill into its virtualenv
> >+        self.addStep(ShellCommand(
> >+            name='install mozmill',
> >+            command=['python',
> >+                     os.path.join('mozmill', 'installmozmill.py')],'
> 
> This is definitely master side os.path.join, which is going to do it based on
> the master's os instead of the slave's os, can we use string joining to make
> this subtle issue more noticeable?

Yes, I think (see above).  If so, done.

> 
> >+        # run mozmill tests
> >+        self.addStep(unittest_steps.MozmillStep(
> >+            name="run_mozmill",
> >+            mozmill=WithProperties('%(mozmillpath)s'),
> >+            tests_dir='tests/firefox',
> >+            env={'PYTHONHOME': WithProperties('%(mozmillenv)s')},
> >+            workdir=WithProperties('%(mozmillenv)s'),
> >+            timeout=10*60,
> >+            flunkOnFailure=True
> >+            ))
> >+        self.addStep(unittest_steps.MozmillStep(
> >+            name="run_mozmill_restart",
> >+            mozmill=WithProperties('%(mozmillrestart)s'),
> >+            tests_dir='tests/firefox/restartTests',
> >+            env={'PYTHONHOME': WithProperties('%(mozmillenv)s')},
> >+            workdir=WithProperties('%(mozmillenv)s'),
> >+            timeout=5*60,
> >+            flunkOnFailure=True
> >+            ))
> >+
> 
> This logic should really be in the self.addSetupSteps() and
> self.addRunTestSteps().  As it is right now, the reboot steps will occur before
> any of the setup or testing occurs because the steps are added after
> self.addTearDownSteps() is called.

I've actually put this into addSetupSteps and addRunTestsSteps separately.  Let me know if you think my division is inappropriate.
(In reply to comment #138)
> (In reply to comment #134)
> > It would be WithProperties("%(basedir)s/build/%(exepath)s"), but in theory. 
> > Not sure why that would be preferable.
> 
> The properties are tracked per run and add overhead to the overloaded test
> masters.  Not sure if it is significant.

Ah, didn't realize that.  Thanks.

> > > >+        # set platform-dependent python variables
> > > >+        if self.platform == "win32":
> > > >+            self.addStep(SetBuildProperty(
> > > >+             property_name="pythonpath",
> > > >+             value="Scripts\\python",
> > > >+             name="set_pythonpath",
> > > >+            ))
> > > >+            self.addStep(SetBuildProperty(
> > > >+             property_name="mozmillpath",
> > > >+             value="Scripts\\mozmill-script.py",
> > > >+             name="set_mozmillpath",
> > > >+                ))
> > > >+            self.addStep(SetBuildProperty(
> > > >+             property_name="mozmillrestart",
> > > >+             value="Scripts\\mozmill-restart-script.py",
> > > >+             name="set_mozmillrestart_path"
> > > >+            ))
> > > >+
> > > >+        else:
> > > >+            self.addStep(SetBuildProperty(
> > > >+             property_name="pythonpath",
> > > >+             value="bin/python",
> > > >+             name="set_pythonpath",
> > > >+            ))
> > > >+            self.addStep(SetBuildProperty(
> > > >+             property_name="mozmillpath",
> > > >+             value="bin/mozmill",
> > > >+             name="set_mozmillpath",
> > > >+            ))
> > > >+            self.addStep(SetBuildProperty(
> > > >+                property_name="mozmillrestart",
> > > >+                value="bin/mozmill-restart",
> > > >+                name="set_mozmillrestart_path",
> > > >+                ))
> > > 
> > > If these are hardcoded, non-runtime dependent strings, why not use factory
> > > arguments and factory logic instead of properties?
> > 
> > I'm not sure what is meant here.  Care to elaborate your suggestion?
> 
> something like:
> 
> if 'win' in self.platform:
>   self.pythonpath = 'Scripts\\python'
> else:
>   self.pythonpath = 'bin/python'
> 
> to set it the value in the factory, then you can consume it in MozmillStep with
> something like: 
> 
> class MozmillStep(ShellCommandReportTimeout):
>   def __init__(self, mozmill, tests_dir, pythonpath, **kwargs):
>     <snip>
>     self.addFactoryArguments(mozmill=mozmill, tests_dir=tests_dir,
>                              pythonpath=pythonpath)
>     self.command = [pythonpath, mozmill, '--showall',
>     <snip>
> 
> Alternately, you could pass the platform into the step and make those decisions
> in the step.

I did this the former way.

> > 
> > > >+        # set absolute path of the mozmill environment's root
> > > >+        self.addStep(SetProperty(
> > > >+            name='set_mozmillenv',
> > > >+            command=['pwd'],
> > > >+            property='mozmillenv',
> > > >+            workdir='build/mozmill',
> > > >+            ))
> > > 
> > > Relative paths don't work?
> > 
> > To override PYTHONHOME?  I doubt it, but could be wrong:
> > 
> > http://k0s.org/mozilla/hg/buildbotcustom-patches/file/3f5f085160b5/bug-516984-2#l209
> 
> wfm,
> jhford-wifi:test-for-mozmill jhford$ export
> PYTHONHOME=~/test-for-mozmill/test-env
> jhford-wifi:test-for-mozmill jhford$ python
> Python 2.6.1 (r261:67515, Feb 11 2010, 00:51:29) 
> [GCC 4.2.1 (Apple Inc. build 5646)] on darwin
> Type "help", "copyright", "credits" or "license" for more information.
> >>> import buildbotcustom
> >>> buildbotcustom.__file__
> '/Users/jhford/test-for-mozmill/test-env/lib/python2.6/site-packages/buildbotcustom/__init__.pyc'
> >>>

Ah, this does work.  For some reason, I thought PYTHONHOME had to be absolute.  Also, could use the -E flag (see http://docs.python.org/using/cmdline.html) if we're not dependent on other python variables (not sure) or unset PYTHONHOME if they're on virtualenv now (also not sure).

> (In reply to comment #135)
> > >>diff --git a/misc.py b/misc.py
> > >This bit of code is in the generateBranchObjects section.  This function is
> > >only run for builder masters.  I am not sure if there is a dependency of
> > >running on a machine capable of running builds, but there is a general movement
> > >towards having tests run only on the test pool.  To do this, you'd want to
> > >modify generateTalosBranchObjects
> > 
> > Okay, I'll try to make this modification if that is what is desired.  I'm a bit
> > perplexed looking at the code though, but I'll continue to look through it.
> 
> Yes, this is complicated code.  If you have a working factory and step I can
> take a look at fitting it into misc.py

I have it up and working except moving the misc.py code into generateTalosBranchObjects.  I'll try to decipher it, this may be time-consuming.
Comment on attachment 473287 [details] [diff] [review]
mozmill + buildbot patch

>+        # install mozmill into its virtualenv
>+        self.addStep(ShellCommand(
>+            name='install mozmill',
>+            command=['python',
>+                     os.path.join('mozmill', 'installmozmill.py')],
>+            flunkOnFailure=True,
>+            haltOnFailure=True,
>+            ))

In comment 141, you mentioned that
>If by string joining meaning verbosely using '/'s, that is now what is
>universally done in this patch.

Is your plan to change this for the final patch?

>+        # point to firefox, not firefox-bin, on linux and macosx
>+        # see https://bugzilla.mozilla.org/show_bug.cgi?id=575863
>+        if (self.platform.startswith('win')
>+            or self.platform.startswith('macosx')):

This comment and the code don't seem to jibe.  The code removes -bin for linux but leaves it there for mac.  Which is correct?

That bug doesn't have a clear explanation of whether the intent is to use firefox-bin or just make it available as an option.  Could you clarify your comment to mention whether replacing '-bin' with '' is a temporary thing until that bug is fixed or the long term plan?

When I tried this out on my macbook using (rev 36f5cf6b2d42)

>mozmill/bin/python mozmill/bin/mozmill --showall -b /Users/jhford/test-
>for-mozmill/trying-out-mozmill/Minefield.app/Contents/MacOS/firefox -t >mozmill/tests/firefox/

and I got 

>Exception: Binary path does not exist /Users/jhford/test-for-mozmill/trying-
>out-mozmill/Minefield.app/Contents/MacOS/firefox/Contents/MacOS/firefox-bin

When I ran with -b Minefield.app, it launched Minefield but died out after a bit of time with:

>Exception: Sorry, cannot connect to jsbridge extension

Have you tested this on OS X?

It looks like exepath (even an absolute version) is not what you need here.  Maybe instead you can set static values using something like:

if self.platform.startswith('win'):
  self.binary = '../%s/%s.exe' % (self.productName, self.productName)
elif self.platform.startswith('linux'):
  self.binary = '../%s/%s' % (self.productName, self.productName)
elif self.platform.startswith('macosx'):
  self.binary = '../%s.app' % self.brand_name 

The problem here is that you'd have to know brand_name at configuration time.  If you would prefer to find the App Bundle at runtime to avoid having to set the brand name statically you could find the bundle by using a FindFile step and set self.binary to be a WithProperties("%(appbundle)s").  

Looking at

>+        self.addStep(unittest_steps.MozmillStep(
>+            name="run_mozmill",
<snip>
>+        self.addStep(unittest_steps.MozmillStep(
>+            name="run_mozmill_restart",

I wonder what the approximate amount of time that the two sets take to complete?  If they are both particularly long it might be worthwhile to split them into two separate builders.  To do that we would have to be able to create factories that have either plain tests or restart tests.
Attempting to test on OS X with minefield.  It looks like there is an error concerning setting the revision step on minefield:

python /Users/mnandigama/buildbot/testor/slave/full/tools/buildfarm/utils/printbuildrev.py firefox
 in dir /Users/mnandigama/buildbot/testor/slave/full/build (timeout 1200 secs)
 watching logfiles {}
 argv: ['python', '/Users/mnandigama/buildbot/testor/slave/full/tools/buildfarm/utils/printbuildrev.py', 'firefox']
 environment:
  Apple_PubSub_Socket_Render=/tmp/launch-uszPN2/Render
  COMMAND_MODE=unix2003
  CVS_RSH=ssh
  DISPLAY=/tmp/launch-s2z14i/:0
  HISTCONTROL=ignoreboth
  HOME=/Users/mnandigama
  LANG=en_US.UTF-8
  LOGNAME=mnandigama
  MANPATH=/usr/share/man:/usr/local/share/man:/usr/X11/man:/usr/local/git/share/man
  OLDPWD=/Users/mnandigama/buildbot
  PATH=/Users/mnandigama/buildbot/bin:/usr/local/mysql/bin:/home/mnandigama/Komodo-IDE-5/bin:/tools/buildbot/bin:/tools/python/bin:/opt/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin:/usr/X11/bin:/usr/local/git/bin
  PS1=(buildbot)${debian_chroot:+($debian_chroot)}\[\033[01;32m\]\u@\h\[\033[00m\]:\[\033[01;34m\]\w\[\033[00m\]\$ 
  PWD=/Users/mnandigama/buildbot/testor/slave/full/build
  PYTHONPATH=/Users/mnandigama/buildbot/buildbotcustom:/tools/buildbot/lib/python2.5/site-packages:/tools/twisted/lib/python2.5/site-packages/:/tools/twisted-core/lib/python2.5/site-packages:/tools/zope-interface/lib/python2.5/site-packages:
  SECURITYSESSIONID=836650
  SHELL=/bin/bash
  SHLVL=1
  SSH_AUTH_SOCK=/tmp/launch-mPUGPP/Listeners
  TERM=xterm-color
  TERM_PROGRAM=Apple_Terminal
  TERM_PROGRAM_VERSION=240.2
  TMPDIR=/var/folders/8I/8Iw3DsvLGMOUSOPGilVffU+++TM/-Tmp-/
  USER=mnandigama
  VIRTUAL_ENV=/Users/mnandigama/buildbot
  _=./restart_buildbot.py
  __CF_USER_TEXT_ENCODING=0x1F6:0:0
 closing stdin
 using PTY: False
Traceback (most recent call last):
  File "/Users/mnandigama/buildbot/testor/slave/full/tools/buildfarm/utils/printbuildrev.py", line 14, in <module>
    buildid = appini.get('App', 'BuildID')
  File "/tools/python-2.5.2/lib/python2.5/ConfigParser.py", line 511, in get
    raise NoSectionError(section)
ConfigParser.NoSectionError: No section: 'App'
program finished with exit code 1
elapsedTime=0.106518

This really means "the file can't be found as the directory is called Minefield.app and not firefox"
Yes, that is what it does mean.  Not sure how this is happening, but I think this is a problem with your environment as this code works in production.

see 
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1284142303.1284142843.8884.gz&fulltext=1

for:

python /Users/cltbld/talos-slave/mozilla-central_leopard_test-reftest/tools/buildfarm/utils/printbuildrev.py ./Minefield.app/Contents/MacOS
 in dir /Users/cltbld/talos-slave/mozilla-central_leopard_test-reftest/build (timeout 1200 secs)
<snip>
buildid: 20100910103003
TinderboxPrint:<a href=http://hg.mozilla.org/mozilla-central/rev/d8203cb80c95 title="Built from revision d8203cb80c95">rev:d8203cb80c95</a>

Are you passing in the correct value into your factory constructor?  It seems like you are getting linux behaviour.  Try using platform='macosx' as a kwarg.
(In reply to comment #145)
<snip/>
> Are you passing in the correct value into your factory constructor?  It seems
> like you are getting linux behaviour.  Try using platform='macosx' as a kwarg.

Yeah, sorry.  I thought platform was set to 'macosx' in my setup, but i must have accidently entered linux when I wiped and recreated my environment.  False alarm.
(In reply to comment #143)

There are three distinct issues (string paths, binary-finding, and multiple factories) here, so I'll break this into three replies.  First, lets take string paths:

> Comment on attachment 473287 [details] [diff] [review]
> mozmill + buildbot patch
> 
> >+        # install mozmill into its virtualenv
> >+        self.addStep(ShellCommand(
> >+            name='install mozmill',
> >+            command=['python',
> >+                     os.path.join('mozmill', 'installmozmill.py')],
> >+            flunkOnFailure=True,
> >+            haltOnFailure=True,
> >+            ))
> 
> In comment 141, you mentioned that
> >If by string joining meaning verbosely using '/'s, that is now what is
> >universally done in this patch.

> Is your plan to change this for the final patch?

I'm not sure I understand the question.  I changed the joiners to '/'s because I thought this was desired, so, yes, per that I would intend to use them for the final patch.  Is this not what is meant?
(In reply to comment #147)
> > Is your plan to change this for the final patch?
> 
> I'm not sure I understand the question.  I changed the joiners to '/'s because
> I thought this was desired, so, yes, per that I would intend to use them for
> the final patch.  Is this not what is meant?

It is desired that they be using string joins.  The most recent patch that you uploaded included an os.path.join after you had said that you had changed all of them to string joins.
(In reply to comment #143)
> Comment on attachment 473287 [details] [diff] [review]
<snip/>
> >+        # point to firefox, not firefox-bin, on linux and macosx
> >+        # see https://bugzilla.mozilla.org/show_bug.cgi?id=575863
> >+        if (self.platform.startswith('win')
> >+            or self.platform.startswith('macosx')):
> 
> This comment and the code don't seem to jibe.  The code removes -bin for linux
> but leaves it there for mac.  Which is correct?

The code is correct;  I've updated the comment in my patch locally, but see below.

> That bug doesn't have a clear explanation of whether the intent is to use
> firefox-bin or just make it available as an option.  Could you clarify your
> comment to mention whether replacing '-bin' with '' is a temporary thing until
> that bug is fixed or the long term plan?

It is meant to be a temporary thing.  In fact, the bug is to be hotfixed today and uploaded to pypi and then to be put into mozilla-central.  So this will be in the tree soon which will eliminate the need for this code.
 
> When I tried this out on my macbook using (rev 36f5cf6b2d42)
> 
> >mozmill/bin/python mozmill/bin/mozmill --showall -b /Users/jhford/test-
> >for-mozmill/trying-out-mozmill/Minefield.app/Contents/MacOS/firefox -t >mozmill/tests/firefox/
> 
> and I got 
> 
> >Exception: Binary path does not exist /Users/jhford/test-for-mozmill/trying-
> >out-mozmill/Minefield.app/Contents/MacOS/firefox/Contents/MacOS/firefox-bin
> 
> When I ran with -b Minefield.app, it launched Minefield but died out after a
> bit of time with:
> 
> >Exception: Sorry, cannot connect to jsbridge extension
> 
> Have you tested this on OS X?

Yes, I get the same result.  This is a known failure: https://bugzilla.mozilla.org/show_bug.cgi?id=583142  Likewise, this is hotfixed and will be in the upcoming pypi mozrunner release.  http://pypi.python.org/pypi/mozrunner 

> It looks like exepath (even an absolute version) is not what you need here. 
> Maybe instead you can set static values using something like:
> 
> if self.platform.startswith('win'):
>   self.binary = '../%s/%s.exe' % (self.productName, self.productName)
> elif self.platform.startswith('linux'):
>   self.binary = '../%s/%s' % (self.productName, self.productName)
> elif self.platform.startswith('macosx'):
>   self.binary = '../%s.app' % self.brand_name 
> 
> The problem here is that you'd have to know brand_name at configuration time. 
> If you would prefer to find the App Bundle at runtime to avoid having to set
> the brand name statically you could find the bundle by using a FindFile step
> and set self.binary to be a WithProperties("%(appbundle)s").  

Again, both of these issues will be fixed soon, first in the mozrunner package and then in mozilla-central.  Following this, it shouldn't be necessary to special-case binary path treatment for mozmill, which IMHO is preferable.
OS: Linux → All
Hardware: x86 → All
That's really good to hear!
(In reply to comment #143)
> Comment on attachment 473287 [details] [diff] [review]
<snip/>
> Looking at
> 
> >+        self.addStep(unittest_steps.MozmillStep(
> >+            name="run_mozmill",
> <snip>
> >+        self.addStep(unittest_steps.MozmillStep(
> >+            name="run_mozmill_restart",
> 
> I wonder what the approximate amount of time that the two sets take to
> complete?  

For the full suite of tests, it is currently 6m49s for the regular tests and 3m45s for the restart tests.  Only a subset of these tests will initially be landed on m-c so the runs will initially be even shorter.

> If they are both particularly long it might be worthwhile to split
> them into two separate builders.  To do that we would have to be able to create
> factories that have either plain tests or restart tests.

So I see three options for this:

1. Keep things as they are.  The easiest option, if this isn't going to be a blocker.

2. Have a __init__ argument for the factory that tells which test suite to run.

3. Have a subclass for the mozmill-restart tests whose only difference is to run the restart tests instead of the normal mozmill tests.

I'd prefer 1., since it is easiest and we can do course correction if the ~10min (or less) run time will be a concern.  Any objections?
Attached patch mozmill + buildbot patch (obsolete) — Splinter Review
removes the remaining os.path.join in favor of joining with /
Attachment #473287 - Attachment is obsolete: true
(In reply to comment #148)
> (In reply to comment #147)
> > > Is your plan to change this for the final patch?
> > 
> > I'm not sure I understand the question.  I changed the joiners to '/'s because
> > I thought this was desired, so, yes, per that I would intend to use them for
> > the final patch.  Is this not what is meant?
> 
> It is desired that they be using string joins.  The most recent patch that you
> uploaded included an os.path.join after you had said that you had changed all
> of them to string joins.

Sorry, I missed that one.  Fixed in the latest version of the patch [posted].
At ~10 minutes, I think we are going to be fine to leave things as they are.  When we get more tests going and it gets longer than that, we could pass a list of test suites to the factory.
(In reply to comment #154)
> At ~10 minutes, I think we are going to be fine to leave things as they are. 
> When we get more tests going and it gets longer than that, we could pass a list
> of test suites to the factory.

Worksforme :)
So if we're not changing the factory invocation, then going from the existing code in generateBranchObjects to adding the factory in generateTalosBranchObjects should not be blocked by the state of the factory, as what is required is how to invoke this as is currently done in the patch in generateBranchObjects.

Were you going to work on this, :jhford?  Any ETA or additional things you need for this task?
I have filed bug 595984 to track the misc.py changes required to get the factories/builders/schedulers.
Component: Release Engineering → Mozmill
Product: mozilla.org → Testing
QA Contact: release → mozmill
See Also: → 595984
Version: other → Trunk
(In reply to comment #156)
> Any ETA or additional things you need
> for this task?

I don't think there is anything additional that I need.  Not sure when I am going to be able to get to this as I am currently working on getting a bunch of new hardware into production.  I will look into this for timing information at the first opportunity that I get.
(In reply to comment #156)
> So if we're not changing the factory invocation, then going from the existing
> code in generateBranchObjects to adding the factory in
> generateTalosBranchObjects should not be blocked by the state of the factory,
> as what is required is how to invoke this as is currently done in the patch in
> generateBranchObjects.
> 
> Were you going to work on this, :jhford?  Any ETA or additional things you need
> for this task?

On further investigation into how buildbot works, it doesn't look like generateTalosBranchObjects is the appropriate place to roll this in.  See the discussion on https://bugzilla.mozilla.org/show_bug.cgi?id=595984#c1
No longer depends on: 519540
Depends on: 519540, 585086
No longer depends on: 519540
this version of the patch appends running mozmill as three steps (one installation, two running) steps in UnittestPackagedBuildFactory so that it can be invoked in the usual way: buildbot-configs -> generateTalosBranchObjects -> generateTestBuilder -> UnittestPackagedBuildFactory -> MozillaPackagedMozmillTests

It has been tested on linux;  it will not correctly invoke unitl bug 585086 is resolved (jsbridge is not compatible with minefield according to version number; mozmill can't use firefox-bin on linux; additional path appending on Mac), but the general structure is sound and it will work once bug 585086 is landed.
Attachment #475672 - Flags: review?(jhford)
Comment on attachment 475672 [details] [diff] [review]
mozmill in UnittestPackagedBuildFactory

>@@ -6492,17 +6492,17 @@ class UnittestPackagedBuildFactory(Mozil
>                  thisChunk=None, chunkByDir=None, **kwargs):
>         platform = platform.split('-')[0]
>         self.test_suites = test_suites
>         self.totalChunks = totalChunks
>         self.thisChunk = thisChunk
>         self.chunkByDir = chunkByDir
>         self.env = MozillaEnvironments['%s-unittest' % platform].copy()
>         self.env['MINIDUMP_STACKWALK'] = getPlatformMinidumpPath(platform)
>-        self.env.update(env)
>+        self.env.update(env or {})

I am not sure if this is the right bug to change this behaviour in.  If you'd like to make the current behaviour more obvious, lets assert env.

>+            elif suite == 'mozmill':
>+
>+                # install mozmill into its virtualenv
>+                self.addStep(ShellCommand(
>+                    name='install mozmill',
>+                    command=['python',
>+                             'mozmill/installmozmill.py'],
>+                    flunkOnFailure=True,
>+                    haltOnFailure=True,
>+                    ))
>+
>+                env = {'PYTHONHOME': '.'}

Is this so that the slave's environment doesn't break mozmill?  Can we instead use python -E in the MozmillStep?

>+
>+                # run the mozmill tests
>+                self.addStep(unittest_steps.MozillaPackagedMozmillTests(
>+                    name="run_mozmill",
>+                    tests_dir='tests/firefox',
>+                    binary=WithProperties('%(basedir)s/build/%(exepath)s'),
>+                    platform=self.platform,
>+                    env=env,
>+                    workdir='build/mozmill',
>+                    timeout=10*60,
>+                    flunkOnFailure=True
>+                    ))
>+                self.addStep(unittest_steps.MozillaPackagedMozmillTests(
>+                    name="run_mozmill_restart",
>+                    tests_dir='tests/firefox/restartTests',
>+                    binary=WithProperties('%(basedir)s/build/%(exepath)s'),                    platform=self.platform,

Assuming that this is a typo but please fix for the next patch

>+                    env=env,
>+                    workdir='build/mozmill',
>+                    timeout=5*60,
>+                    flunkOnFailure=True
>+                    ))

I don't think it is critical, but it would be nice if the workdir parameter used the default of 'build'.  This would be consistent with how we have the other tests set up.  We could do this by passing in a mozmill subdirectory and changing paths as appropriate in the step.  We could avoid using absolute paths.  Not going to r- on this though.

As you mentioned today, the restart tests need to use mozmill-restart instead of mozmill.

>+def summarizeMozmillLog(name, log):
>+    log = log.getText()
>+    m = re.search("^Passed: (\d+) :: Failed (\d+) :: Skipped (\d+)", log, re.M)
>+    if not m:
>+        return "TinderboxPrint: %s<br/>%s\n" % (name, summaryText(0, 0))
>+    else:
>+        return "TinderboxPrint: %s<br/>%s\n" % (name, summaryText(
>+            int(m.group(1)),
>+            int(m.group(2)),
>+            ))
>+    

This is not valid for the current output of the harness.  The TinderboxPrint should be similar to:

TinderboxPrint: harness-name pass_count/<em class="testfail">fail_count</em>/skip_count

You might find http://hg.mozilla.org/build/buildbotcustom/file/tip/steps/unittest.py#l43 useful

>+class MozillaPackagedMozmillTests(ShellCommandReportTimeout):
>+    warnOnFailure=True
>+    warnOnWarnings=True
>+
>+    # XXX the log parsing will need to be redone when Mozmill 1.5
>+    # lands in mozilla-central
>+    # see https://bugzilla.mozilla.org/show_bug.cgi?id=585086
>+    # Mozmill 1.5 conforms close to standard Mozilla logging conventions
>+
>+    def __init__(self, tests_dir, binary, platform, restart=False, **kwargs):
>+        """
>+        run mozmill tests;
>+        should be run from the mozmill virtualenv directory
>+        - tests_dir : path to the tests to run
>+        - binary : path to the binary to be used
>+        - platform : system platform
>+        - restart : whether to run the restart tests
>+        """
>+        
>+        self.super_class = ShellCommandReportTimeout
>+        ShellCommandReportTimeout.__init__(self, **kwargs)
>+
>+        self.addFactoryArguments(tests_dir=tests_dir,
>+                                 binary=binary,
>+                                 platform=platform,
>+                                 restart=restart)

If it is realistic that someone would ever use a script other than 'mozmill' or 'mozmill-restart', could we instead pass in a mozmill_script_name variable?  If it defaulted to 'mozmill', we could override for the restart tests

>+
>+        # set platform-dependent python scripts
>+        # relative to the created virtualenv
>+        if platform == "win32":
>+            python = r'Scripts\python'
>+            if restart:
>+                mozmill = r'Scripts\mozmill-restart-script.py'
>+            else:
>+                mozmill = r'Scripts\mozmill-script.py'
>+        else:
>+            python = 'bin/python'
>+            if restart:
>+                mozmill = 'bin/mozmill-restart'
>+            else:
>+                mozmill = 'bin/mozmill'
>+
>+            
>+        self.command = [
>+            python,
>+            mozmill,
>+            '--showall',
>+            '-b', binary,
>+            '-t', tests_dir,
>+            ]
>+            
>+    def createSummary(self, log):
>+        self.addCompleteLog('summary', summarizeMozmillLog(self.name, log))
>+
>+    def evaluateCommand(self, cmd):
>+        superResult = self.super_class.evaluateCommand(self, cmd)
>+        if superResult != SUCCESS:
>+            return superResult
>+        
>+        # If we find "Test-Failed", then return WARNINGS
>+        if "Test-Failed" in cmd.logs['stdio'].getText():
>+            return WARNINGS

You mention moving towards standard logging output.  Lots of other frameworks print out TEST-UNEXPECTED-PASS and TEST-UNEXPECTED-FAIL.  Maybe we can add a check for TEST-UNEXPECTED

This patch isn't ready for production, but it is looking good.  I will run it through staging again when the patches have landed on mozilla-central.
Attachment #475672 - Flags: review?(jhford) → review-
(In reply to comment #161)
> Comment on attachment 475672 [details] [diff] [review]
> mozmill in UnittestPackagedBuildFactory
> 
> >@@ -6492,17 +6492,17 @@ class UnittestPackagedBuildFactory(Mozil
> >                  thisChunk=None, chunkByDir=None, **kwargs):
> >         platform = platform.split('-')[0]
> >         self.test_suites = test_suites
> >         self.totalChunks = totalChunks
> >         self.thisChunk = thisChunk
> >         self.chunkByDir = chunkByDir
> >         self.env = MozillaEnvironments['%s-unittest' % platform].copy()
> >         self.env['MINIDUMP_STACKWALK'] = getPlatformMinidumpPath(platform)
> >-        self.env.update(env)
> >+        self.env.update(env or {})
> 
> I am not sure if this is the right bug to change this behaviour in.  If you'd
> like to make the current behaviour more obvious, lets assert env.

Should I file this as a separate bug then?  Passing env=None and then doing a dictionary update that will fail on the default value sure seems like undesired behaviour.  It is not important for this bug, though it does make testing more difficult

> 
> >+            elif suite == 'mozmill':
> >+
> >+                # install mozmill into its virtualenv
> >+                self.addStep(ShellCommand(
> >+                    name='install mozmill',
> >+                    command=['python',
> >+                             'mozmill/installmozmill.py'],
> >+                    flunkOnFailure=True,
> >+                    haltOnFailure=True,
> >+                    ))
> >+
> >+                env = {'PYTHONHOME': '.'}
> 
> Is this so that the slave's environment doesn't break mozmill?  Can we instead
> use python -E in the MozmillStep?

The only reason I didn't use python -E is I wasn't sure if any of the other python variables were defined or depended on on the slaves.  If they aren't, we can certainly use python -E.  Are they?

I'll address the other issues tomorrow.
(In reply to comment #162)
> (In reply to comment #161)
> > Comment on attachment 475672 [details] [diff] [review] [details]
> > mozmill in UnittestPackagedBuildFactory
> > 
> > >@@ -6492,17 +6492,17 @@ class UnittestPackagedBuildFactory(Mozil
> > >                  thisChunk=None, chunkByDir=None, **kwargs):
> > >         platform = platform.split('-')[0]
> > >         self.test_suites = test_suites
> > >         self.totalChunks = totalChunks
> > >         self.thisChunk = thisChunk
> > >         self.chunkByDir = chunkByDir
> > >         self.env = MozillaEnvironments['%s-unittest' % platform].copy()
> > >         self.env['MINIDUMP_STACKWALK'] = getPlatformMinidumpPath(platform)
> > >-        self.env.update(env)
> > >+        self.env.update(env or {})
> > 
> > I am not sure if this is the right bug to change this behaviour in.  If you'd
> > like to make the current behaviour more obvious, lets assert env.
> 
> Should I file this as a separate bug then?  Passing env=None and then doing a
> dictionary update that will fail on the default value sure seems like undesired
> behaviour.  It is not important for this bug, though it does make testing more
> difficult

Instead of changing this behaviour, lets make it an explicit |assert env, 'UnittestPackagedBuildFactory needs an environment'|.  This requires no extra testing.

> > Is this so that the slave's environment doesn't break mozmill?  Can we instead
> > use python -E in the MozmillStep?
> 
> The only reason I didn't use python -E is I wasn't sure if any of the other
> python variables were defined or depended on on the slaves.  If they aren't, we
> can certainly use python -E.  Are they?
> 
> I'll address the other issues tomorrow.

ShellCommands work at the low level by running an os.exec* function.  This shouldn't have any impact on the running buildbot.  If mozmill works with python -E, lets use it and remove a potential source of future errors :)

It seems like there were two blocking issues for this landing.  For the next round of testing, I'd like to do it in our staging environment on all the platforms we want to deploy to.  For this, can we make sure that there is a build available for windows, linux, linux64, mac 32bit and mac64bit.  It doesn't matter to me if they are mozilla-central tinderbox/nightly/try builds or one-off testing builds like the one Clint did today.
(In reply to comment #163)
> (In reply to comment #162)
> > (In reply to comment #161)
> > > Comment on attachment 475672 [details] [diff] [review] [details] [details]
> > > mozmill in UnittestPackagedBuildFactory
> > > 
> > > >@@ -6492,17 +6492,17 @@ class UnittestPackagedBuildFactory(Mozil
> > > >                  thisChunk=None, chunkByDir=None, **kwargs):
> > > >         platform = platform.split('-')[0]
> > > >         self.test_suites = test_suites
> > > >         self.totalChunks = totalChunks
> > > >         self.thisChunk = thisChunk
> > > >         self.chunkByDir = chunkByDir
> > > >         self.env = MozillaEnvironments['%s-unittest' % platform].copy()
> > > >         self.env['MINIDUMP_STACKWALK'] = getPlatformMinidumpPath(platform)
> > > >-        self.env.update(env)
> > > >+        self.env.update(env or {})
> > > 
> > > I am not sure if this is the right bug to change this behaviour in.  If you'd
> > > like to make the current behaviour more obvious, lets assert env.
> > 
> > Should I file this as a separate bug then?  Passing env=None and then doing a
> > dictionary update that will fail on the default value sure seems like undesired
> > behaviour.  It is not important for this bug, though it does make testing more
> > difficult
> 
> Instead of changing this behaviour, lets make it an explicit |assert env,
> 'UnittestPackagedBuildFactory needs an environment'|.  This requires no extra
> testing.

As you point out, this really has nothing to do with this bug.  Maybe we should just leave as is and ticket separately if desired.

> 
> > > Is this so that the slave's environment doesn't break mozmill?  Can we instead
> > > use python -E in the MozmillStep?
> > 
> > The only reason I didn't use python -E is I wasn't sure if any of the other
> > python variables were defined or depended on on the slaves.  If they aren't, we
> > can certainly use python -E.  Are they?
> > 
> > I'll address the other issues tomorrow.
> 
> ShellCommands work at the low level by running an os.exec* function.  This
> shouldn't have any impact on the running buildbot.  If mozmill works with
> python -E, lets use it and remove a potential source of future errors :)

Yeah, mozmill should work fine with it.  Will do.
 
> It seems like there were two blocking issues for this landing.  For the next
> round of testing, I'd like to do it in our staging environment on all the
> platforms we want to deploy to.  For this, can we make sure that there is a
> build available for windows, linux, linux64, mac 32bit and mac64bit.  It
> doesn't matter to me if they are mozilla-central tinderbox/nightly/try builds
> or one-off testing builds like the one Clint did today.
Attached patch mozmill + buildbot patch (obsolete) — Splinter Review
udpated patch from review
Attachment #474795 - Attachment is obsolete: true
Attachment #475672 - Attachment is obsolete: true
Attachment #476361 - Flags: review?(jhford)
> Assuming that this is a typo but please fix for the next patch

It was a typo, and it is fixed
 
> >+                    env=env,
> >+                    workdir='build/mozmill',
> >+                    timeout=5*60,
> >+                    flunkOnFailure=True
> >+                    ))
> 
> I don't think it is critical, but it would be nice if the workdir parameter
> used the default of 'build'.  This would be consistent with how we have the
> other tests set up.  We could do this by passing in a mozmill subdirectory and
> changing paths as appropriate in the step.  We could avoid using absolute
> paths.  Not going to r- on this though.

I can do this, but would prefer not to.  To me it makes a lot more sense having the mozmill step assume it is in the directory of its (to be) virtualenv than otherwise.  I realize this is more illusion of separation of concerns than actual separation of concerns.

> As you mentioned today, the restart tests need to use mozmill-restart instead
> of mozmill.
> 
> >+def summarizeMozmillLog(name, log):
> >+    log = log.getText()
> >+    m = re.search("^Passed: (\d+) :: Failed (\d+) :: Skipped (\d+)", log, re.M)
> >+    if not m:
> >+        return "TinderboxPrint: %s<br/>%s\n" % (name, summaryText(0, 0))
> >+    else:
> >+        return "TinderboxPrint: %s<br/>%s\n" % (name, summaryText(
> >+            int(m.group(1)),
> >+            int(m.group(2)),
> >+            ))
> >+    
> 
> This is not valid for the current output of the harness.  The TinderboxPrint
> should be similar to:
> 
> TinderboxPrint: harness-name pass_count/<em
> class="testfail">fail_count</em>/skip_count
> 
> You might find
> http://hg.mozilla.org/build/buildbotcustom/file/tip/steps/unittest.py#l43
> useful

Should be fixed (I hope).

> >+class MozillaPackagedMozmillTests(ShellCommandReportTimeout):
> >+    warnOnFailure=True
> >+    warnOnWarnings=True
> >+
> >+    # XXX the log parsing will need to be redone when Mozmill 1.5
> >+    # lands in mozilla-central
> >+    # see https://bugzilla.mozilla.org/show_bug.cgi?id=585086
> >+    # Mozmill 1.5 conforms close to standard Mozilla logging conventions
> >+
> >+    def __init__(self, tests_dir, binary, platform, restart=False, **kwargs):
> >+        """
> >+        run mozmill tests;
> >+        should be run from the mozmill virtualenv directory
> >+        - tests_dir : path to the tests to run
> >+        - binary : path to the binary to be used
> >+        - platform : system platform
> >+        - restart : whether to run the restart tests
> >+        """
> >+        
> >+        self.super_class = ShellCommandReportTimeout
> >+        ShellCommandReportTimeout.__init__(self, **kwargs)
> >+
> >+        self.addFactoryArguments(tests_dir=tests_dir,
> >+                                 binary=binary,
> >+                                 platform=platform,
> >+                                 restart=restart)
> 
> If it is realistic that someone would ever use a script other than 'mozmill' or
> 'mozmill-restart', could we instead pass in a mozmill_script_name variable?  If
> it defaulted to 'mozmill', we could override for the restart tests

The medium-long term plans are to replace `mozmill-restart` with `mozmill --restart` (and likewise `mozmill --thunderbird` for comm-central).  So I'm not sure if this is a direction we want to go.  We'll have to update buildbotcustom when this change lives on m-c, but that probably won't be until early 2011.

> >+
> >+        # set platform-dependent python scripts
> >+        # relative to the created virtualenv
> >+        if platform == "win32":
> >+            python = r'Scripts\python'
> >+            if restart:
> >+                mozmill = r'Scripts\mozmill-restart-script.py'
> >+            else:
> >+                mozmill = r'Scripts\mozmill-script.py'
> >+        else:
> >+            python = 'bin/python'
> >+            if restart:
> >+                mozmill = 'bin/mozmill-restart'
> >+            else:
> >+                mozmill = 'bin/mozmill'
> >+
> >+            
> >+        self.command = [
> >+            python,
> >+            mozmill,
> >+            '--showall',
> >+            '-b', binary,
> >+            '-t', tests_dir,
> >+            ]
> >+            
> >+    def createSummary(self, log):
> >+        self.addCompleteLog('summary', summarizeMozmillLog(self.name, log))
> >+
> >+    def evaluateCommand(self, cmd):
> >+        superResult = self.super_class.evaluateCommand(self, cmd)
> >+        if superResult != SUCCESS:
> >+            return superResult
> >+        
> >+        # If we find "Test-Failed", then return WARNINGS
> >+        if "Test-Failed" in cmd.logs['stdio'].getText():
> >+            return WARNINGS
> 
> You mention moving towards standard logging output.  Lots of other frameworks
> print out TEST-UNEXPECTED-PASS and TEST-UNEXPECTED-FAIL.  Maybe we can add a
> check for TEST-UNEXPECTED

Again, hopefully should be fixed.
 
> This patch isn't ready for production, but it is looking good.  I will run it
> through staging again when the patches have landed on mozilla-central.

New one attached.  Let me know if there are any additional concerns, please.
(In reply to comment #166)
> > Assuming that this is a typo but please fix for the next patch
> 
> It was a typo, and it is fixed
> 
> > >+                    env=env,
> > >+                    workdir='build/mozmill',
> > >+                    timeout=5*60,
> > >+                    flunkOnFailure=True
> > >+                    ))
> > 
> > I don't think it is critical, but it would be nice if the workdir parameter
> > used the default of 'build'.  This would be consistent with how we have the
> > other tests set up.  We could do this by passing in a mozmill subdirectory and
> > changing paths as appropriate in the step.  We could avoid using absolute
> > paths.  Not going to r- on this though.
> 
> I can do this, but would prefer not to.  To me it makes a lot more sense having
> the mozmill step assume it is in the directory of its (to be) virtualenv than
> otherwise.  I realize this is more illusion of separation of concerns than
> actual separation of concerns.

Sure, that makes sense

> > As you mentioned today, the restart tests need to use mozmill-restart instead
> > of mozmill.
> > 
> > >+def summarizeMozmillLog(name, log):
> > >+    log = log.getText()
> > >+    m = re.search("^Passed: (\d+) :: Failed (\d+) :: Skipped (\d+)", log, re.M)
> > >+    if not m:
> > >+        return "TinderboxPrint: %s<br/>%s\n" % (name, summaryText(0, 0))
> > >+    else:
> > >+        return "TinderboxPrint: %s<br/>%s\n" % (name, summaryText(
> > >+            int(m.group(1)),
> > >+            int(m.group(2)),
> > >+            ))
> > >+    
> > 
> > This is not valid for the current output of the harness.  The TinderboxPrint
> > should be similar to:
> > 
> > TinderboxPrint: harness-name pass_count/<em
> > class="testfail">fail_count</em>/skip_count
> > 
> > You might find
> > http://hg.mozilla.org/build/buildbotcustom/file/tip/steps/unittest.py#l43
> > useful
> 
> Should be fixed (I hope).

It does look like it has been fixed.

> > >+class MozillaPackagedMozmillTests(ShellCommandReportTimeout):
> > >+    warnOnFailure=True
> > >+    warnOnWarnings=True
> > >+
> > >+    # XXX the log parsing will need to be redone when Mozmill 1.5
> > >+    # lands in mozilla-central
> > >+    # see https://bugzilla.mozilla.org/show_bug.cgi?id=585086
> > >+    # Mozmill 1.5 conforms close to standard Mozilla logging conventions
> > >+
> > >+    def __init__(self, tests_dir, binary, platform, restart=False, **kwargs):
> > >+        """
> > >+        run mozmill tests;
> > >+        should be run from the mozmill virtualenv directory
> > >+        - tests_dir : path to the tests to run
> > >+        - binary : path to the binary to be used
> > >+        - platform : system platform
> > >+        - restart : whether to run the restart tests
> > >+        """
> > >+        
> > >+        self.super_class = ShellCommandReportTimeout
> > >+        ShellCommandReportTimeout.__init__(self, **kwargs)
> > >+
> > >+        self.addFactoryArguments(tests_dir=tests_dir,
> > >+                                 binary=binary,
> > >+                                 platform=platform,
> > >+                                 restart=restart)
> > 
> > If it is realistic that someone would ever use a script other than 'mozmill' or
> > 'mozmill-restart', could we instead pass in a mozmill_script_name variable?  If
> > it defaulted to 'mozmill', we could override for the restart tests
> 
> The medium-long term plans are to replace `mozmill-restart` with `mozmill
> --restart` (and likewise `mozmill --thunderbird` for comm-central).  So I'm not
> sure if this is a direction we want to go.  We'll have to update buildbotcustom
> when this change lives on m-c, but that probably won't be until early 2011.

That would make sense.  Lets leave it

> > >+
> > >+        # set platform-dependent python scripts
> > >+        # relative to the created virtualenv
> > >+        if platform == "win32":
> > >+            python = r'Scripts\python'
> > >+            if restart:
> > >+                mozmill = r'Scripts\mozmill-restart-script.py'
> > >+            else:
> > >+                mozmill = r'Scripts\mozmill-script.py'
> > >+        else:
> > >+            python = 'bin/python'
> > >+            if restart:
> > >+                mozmill = 'bin/mozmill-restart'
> > >+            else:
> > >+                mozmill = 'bin/mozmill'
> > >+
> > >+            
> > >+        self.command = [
> > >+            python,
> > >+            mozmill,
> > >+            '--showall',
> > >+            '-b', binary,
> > >+            '-t', tests_dir,
> > >+            ]
> > >+            
> > >+    def createSummary(self, log):
> > >+        self.addCompleteLog('summary', summarizeMozmillLog(self.name, log))
> > >+
> > >+    def evaluateCommand(self, cmd):
> > >+        superResult = self.super_class.evaluateCommand(self, cmd)
> > >+        if superResult != SUCCESS:
> > >+            return superResult
> > >+        
> > >+        # If we find "Test-Failed", then return WARNINGS
> > >+        if "Test-Failed" in cmd.logs['stdio'].getText():
> > >+            return WARNINGS
> > 
> > You mention moving towards standard logging output.  Lots of other frameworks
> > print out TEST-UNEXPECTED-PASS and TEST-UNEXPECTED-FAIL.  Maybe we can add a
> > check for TEST-UNEXPECTED
> 
> Again, hopefully should be fixed.
> 
> > This patch isn't ready for production, but it is looking good.  I will run it
> > through staging again when the patches have landed on mozilla-central.
> 
> New one attached.  Let me know if there are any additional concerns, please.

I think that my concerns are addressed by this patch, providing mozmill code issues are solved before landing this patch.  Are the fixes for the Mac app bundle going to be landing on m-c soon?

I am going to hold testing off until I have builds that are going to work with this patch available for testing.
There is currently no restart tests, so the second command does not work but will as soon as a restart test is checked in (tested on linux)
(In reply to comment #168)
> There is currently no restart tests, so the second command does not work but
> will as soon as a restart test is checked in (tested on linux)

Is there a restart test that is planned to be included before this is expected to land in production?  If not, lets leave the second command out of the patch.
(In reply to comment #169)
> (In reply to comment #168)
> > There is currently no restart tests, so the second command does not work but
> > will as soon as a restart test is checked in (tested on linux)
> 
> Is there a restart test that is planned to be included before this is expected
> to land in production?  If not, lets leave the second command out of the patch.

We do not plan to have any restart tests for now. The initial testruns should only include the normal mozmill tests which have already been checked in on m-c. We will follow-up with restart tests once the normal tests are stable and restarts are working fine on the try server.
Comment on attachment 476361 [details] [diff] [review]
mozmill + buildbot patch

This is not showing the summary properly.  The build is showing as green (correctly) and one that failed a test (fedora 12 x64 debug) showed orange (correctly).

Both instances print 

TinderboxPrint: run_mozmill<br/><em class="testfail">T-FAIL</em>

as their summary.

>+def summarizeMozmillLog(name, log):
>+    log = log.getText()
>+    m = re.search("^INFO Passed: (\d+).*\sINFO Failed: (\d+)\sINFO Skipped: (\d+)", log, re.M)
>+    if not m:
>+        return 'TinderboxPrint: %s<br/>%s\n' % (name, summaryText(0, 0))
>+    else:
>+        return 'TinderboxPrint: %s %d/<em class="testfail">%d</em>/%d\n' % (name, 
>+                                                                            int(m.group(1)),
>+                                                                            int(m.group(2)),
>+                                                                            int(m.group(3)))
>+ 

INFO | Step Pass: {"function": "Controller.type()"}
INFO | Step Pass: {"function": "Controller.type()"}
INFO | Step Pass: {"function": "controller.waitForEval()"}
INFO | Step Pass: {"function": "controller.assertJS('subject.isSameFavicon == true')"}
TEST-PASS | /home/cltbld/talos-slave/mozilla-central_fedora-debug_test-mozmill-all/build/mozmill/tests/firefox/testAwesomeBar/testFaviconInAutocomplete.js | testFaviconInAutoComplete
DEBUG | mozmill.endTest | {"passes": [{"function": "controller.waitFor()"}, {"function": "Controller.waitForElement()"}, {"function": "Controller.click()"}, {"function": "Controller.keypress()"}, {"function": "controller.waitForEval()"}, {"function": "Controller.type()"}, {"function": "Controller.keypress()"}, {"function": "controller.waitFor()"}, {"function": "controller.waitForPageLoad()"}, {"function": "Controller.keypress()"}, {"function": "controller.waitForEval()"}, {"function": "Controller.keypress()"}, {"function": "controller.waitForEval()"}, {"function": "Controller.type()"}, {"function": "Controller.type()"}, {"function": "Controller.type()"}, {"function": "Controller.type()"}, {"function": "Controller.type()"}, {"function": "Controller.type()"}, {"function": "Controller.type()"}, {"function": "controller.waitForEval()"}, {"function": "controller.assertJS('subject.isSameFavicon == true')"}], "fails": [], "name": "testFaviconInAutoComplete", "filename": "/home/cltbld/talos-slave/mozilla-central_fedora-debug_test-mozmill-all/build/mozmill/tests/firefox/testAwesomeBar/testFaviconInAutocomplete.js", "failed": 0, "passed": 22}
DEBUG | mozmill.persist
DEBUG | mozmill.endRunner | true
INFO Passed: 28
INFO Failed: 0
INFO Skipped: 0
--DOCSHELL 0x9a77ec8 == 5
--DOCSHELL 0x99b92d8 == 4
--DOCSHELL 0x9110348 == 3
WARNING: An event was posted to a thread that will never run it (rejected): file /builds/slave/mozilla-central-linux-debug/build/xpcom/threads/nsThread.cpp, line 371
WARNING: unable to post continuation event: file /builds/slave/mozilla-central-linux-debug/build/xpcom/io/nsStreamUtils.cpp, line 475
WARNING: An event was posted to a thread that will never run it (rejected): file /builds/slave/mozilla-central-linux-debug/build/xpcom/threads/nsThread.cpp, line 371
WARNING: unable to post continuation event: file /builds/slave/mozilla-central-linux-debug/build/xpcom/io/nsStreamUtils.cpp, line 475
Comment on attachment 476361 [details] [diff] [review]
mozmill + buildbot patch

As discussed

"^INFO Passed: (\d+).*\sINFO Failed: (\d+)\sINFO Skipped: (\d+)"

should really be 

"^INFO Passed: (\d+).*\sINFO Failed: (\d+).*\sINFO Skipped: (\d+)"

I can do that as a part of landing the patch.

I will add this to the next bug.
Attachment #476361 - Flags: review?(jhford) → review+
add the mozmill-all test suite
Attachment #476361 - Attachment is obsolete: true
Attachment #477196 - Flags: review?(aki)
Comment on attachment 476361 [details] [diff] [review]
mozmill + buildbot patch

whoops
Attachment #476361 - Attachment is obsolete: false
Attachment #477196 - Flags: review?(aki) → review+
Further testing has shown that tests aren't working on windows because of the mozmill installation.

The current plan is for this to be fixed an landed before the downtime on Thursday.
there is an error for windows because windows uses different paths for everything....not sure why, but hopefully this fixes
Attachment #477279 - Flags: review?(ctalbert)
(In reply to comment #175)
> Further testing has shown that tests aren't working on windows because of the
> mozmill installation.
> 
> The current plan is for this to be fixed an landed before the downtime on
> Thursday.

The patch in https://bug516984.bugzilla.mozilla.org/attachment.cgi?id=477279 should fix, I think
Attachment #477279 - Flags: review?(ctalbert) → review+
This adds a restart test to our test list.  Tested the restart test on windows 7 with both debug and release.  Henrik and Geo tested it on linux and mac as well.
Attachment #477307 - Flags: review?(hskupin)
Attachment #477307 - Flags: review?(hskupin) → review+
From the tools staging env, it looks like there is a windows path issue.

The windows builder gets

Exception: Binary path does not exist /c/talos-slave/mozilla-central_win7_test-mozmill-all/build/firefox/firefox.exe

even though this is the correct path that it should be extracted to:

C:\Windows\system32\cmd.exe /c unzip -o firefox-4.0b7pre.en-US.win32.zip
 in dir c:\talos-slave\mozilla-central_win7_test-mozmill-all\build 
...
  inflating: firefox/firefox.exe

Trying to use a relative path, I get

C:\Windows\system32\cmd.exe /c Scripts\python -E Scripts\mozmill-restart-script.py --showall -b ../firefox/firefox.exe -t tests/firefox/restartTests
 in dir c:\talos-slave\mozilla-central_win7_test-mozmill-all\build/mozmill
...
Traceback (most recent call last):
  File "Scripts\mozmill-restart-script.py", line 8, in <module>
    load_entry_point('mozmill==1.5.0', 'console_scripts', 'mozmill-restart')()
  File "c:\talos-slave\mozilla-central_win7_test-mozmill-all\build\mozmill\Lib\site-packages\mozmill\__init__.py", line 816, in restart_cli
    RestartCLI().run()
  File "c:\talos-slave\mozilla-central_win7_test-mozmill-all\build\mozmill\Lib\site-packages\mozmill\__init__.py", line 765, in run
    self.mozmill.run_tests(self.options.test)
  File "c:\talos-slave\mozilla-central_win7_test-mozmill-all\build\mozmill\Lib\site-packages\mozmill\__init__.py", line 667, in run_tests
    self.run_dir(d, sleeptime)
  File "c:\talos-slave\mozilla-central_win7_test-mozmill-all\build\mozmill\Lib\site-packages\mozmill\__init__.py", line 612, in run_dir
    frame = self.start_runner()
  File "c:\talos-slave\mozilla-central_win7_test-mozmill-all\build\mozmill\Lib\site-packages\mozmill\__init__.py", line 573, in start_runner
    self.runner.start()
  File "c:\talos-slave\mozilla-central_win7_test-mozmill-all\build\mozmill\Lib\site-packages\mozrunner\__init__.py", line 446, in start
    self.process_handler = run_command(self.command+self.cmdargs, self.env, **self.kp_kwargs)
  File "c:\talos-slave\mozilla-central_win7_test-mozmill-all\build\mozmill\Lib\site-packages\mozrunner\__init__.py", line 89, in run_command
    return killableprocess.Popen(cmd, env=env, **killable_kwargs)
  File "C:\mozilla-build\Python25\lib\subprocess.py", line 593, in __init__
    errread, errwrite)
  File "c:\talos-slave\mozilla-central_win7_test-mozmill-all\build\mozmill\Lib\site-packages\mozrunner\killableprocess.py", line 149, in _execute_child
    winprocess.AssignProcessToJobObject(self._job, hp)
  File "c:\talos-slave\mozilla-central_win7_test-mozmill-all\build\mozmill\Lib\site-packages\mozrunner\winprocess.py", line 49, in ErrCheckBool
    raise WinError()
WindowsError: [Error 13] Access is denied.

I am unsure if this is the same error in different form.  If the latter is a path (e.g. missing binary, file not found), then Mozmill should raise a better error message.

I'm also unsure why this is the case for Mozmill and not the other test harnesses.  Paths in buildbot all seem to be '/' joined (vs. '\' joined):

http://hg.mozilla.org/build/buildbotcustom/file/a70b38b40088/steps/unittest.py#l453

Maybe using bash -c will help us here?

I will try that, and failing so will setup a windows VM for debugging locally.
> I'm also unsure why this is the case for Mozmill and not the other test
> harnesses.  Paths in buildbot all seem to be '/' joined (vs. '\' joined):
> 
> http://hg.mozilla.org/build/buildbotcustom/file/a70b38b40088/steps/unittest.py#l453
> 
> Maybe using bash -c will help us here?
> 

Nope, no luck that way.  I'll setup a windows VM locally and try to fix
(In reply to comment #181)
> > I'm also unsure why this is the case for Mozmill and not the other test
> > harnesses.  Paths in buildbot all seem to be '/' joined (vs. '\' joined):
> > 
> > http://hg.mozilla.org/build/buildbotcustom/file/a70b38b40088/steps/unittest.py#l453
> > 
> > Maybe using bash -c will help us here?
> > 
> 
> Nope, no luck that way.  I'll setup a windows VM locally and try to fix

To further clarify, the paths are not the issue (or if they are, there is a great deal of subtlty concerning them).  In order to be able to generate OS-specific path delimeters from the '/'s universally used in buildbotcustom, I wrote some new classes, `Path` and `WithPathProperties` and made `MozillaPackagedMozmillTests` consume them:

"""
mport posixpath
import ntpath
class Path(object):
    def __init__(self, platform):
        self.platform = platform

    def __call__(self, *path):
        if self.platform.startswith('win'):
            path = [i.replace('/', '\\') for i in path]
            return ntpath.join(*path)
        return posixpath.join(*path)

class WithPathProperties(WithProperties):
    def __init__(self, fmtstring, platform, *args):
        self.path = Path(platform)
        WithProperties.__init__(self, fmtstring, *args)

    def render(self, pmap):
        return self.path(WithProperties.render(self, pmap))

class MozillaPackagedMozmillTests(ShellCommandReportTimeout):
    warnOnFailure=True
    warnOnWarnings=True

    def __init__(self, tests_dir, binary, platform, restart=False, **kwargs):
        """
        run mozmill tests;
        should be run from the mozmill virtualenv directory
        - tests_dir : path to the tests to run
        - binary : path to the binary to be used
        - platform : system platform
        - restart : whether to run the restart tests
        """

        self.super_class = ShellCommandReportTimeout
        ShellCommandReportTimeout.__init__(self, **kwargs)

        self.addFactoryArguments(tests_dir=tests_dir,
                                 binary=binary,
                                 platform=platform,
                                 restart=restart)

        path = Path(platform)

        # set platform-dependent python scripts
        # relative to the created virtualenv
        if platform == "win32":
            python = r'mozmill/Scripts/python'
            if restart:
                mozmill = r'mozmill/Scripts/mozmill-restart-script.py'
            else:
                mozmill = r'mozmill/Scripts/mozmill-script.py'
        else:
            python = 'mozmill/bin/python'
            if restart:
                mozmill = 'mozmill/bin/mozmill-restart'
            else:
                mozmill = 'mozmill/bin/mozmill'

        # command to run
        script = [
            path(python),
            '-E',
            path(mozmill),
            '--showall',
            '-b', WithPathProperties(binary, platform),
            '-t', path(tests_dir),
            ]
        self.command = script
"""

This correctly reverses the slashes; however, the mysterious "Access denied" persists:

"""
C:\Windows\system32\cmd.exe /c mozmill\Scripts\python -E mozmill\Scripts\mozmill-script.py --showall -b firefox\firefox.exe -t mozmill\tests\firefox
 in dir c:\talos-slave\mozilla-central_win7_test-mozmill-all\build (timeout 600 secs)
 watching logfiles {}
 argv: ['C:\\Windows\\system32\\cmd.exe', '/c', 'mozmill\\Scripts\\python', '-E', 'mozmill\\Scripts\\mozmill-script.py', '--showall', '-b', 'firefox\\firefox.exe', '-t', 'mozmill\\tests\\firefox']
 environment:
  ALLUSERSPROFILE=C:\ProgramData
  APPDATA=C:\Users\cltbld\AppData\Roaming
  COMMONPROGRAMFILES=C:\Program Files\Common Files
  COMPUTERNAME=TOOLS-R3-W7-001
  COMSPEC=C:\Windows\system32\cmd.exe
  FP_NO_HOST_CHECK=NO
  HOMEDRIVE=C:
  HOMEPATH=\Users\cltbld
  LOCALAPPDATA=C:\Users\cltbld\AppData\Local
  LOGONSERVER=\\TOOLS-R3-W7-001
  MOZ_CRASHREPORTER_NO_REPORT=1
  MOZ_NO_REMOTE=1
  NO_EM_RESTART=1
  NUMBER_OF_PROCESSORS=2
  OS=Windows_NT
  PATH=C:\mozilla-build;C:\mozilla-build\msys\bin;C:\mozilla-build\msys\local\bin;C:\mozilla-build\Python25;C:\mozilla-build\Python25\Scripts;C:\mozilla-build\hg;C:\mozilla-build\7zip;C:\mozilla-build\upx203w;C:\WINDOWS\System32;C:\WINDOWS;
  PATHEXT=.COM;.EXE;.BAT;.CMD;.VBS;.VBE;.JS;.JSE;.WSF;.WSH;.MSC
  PROCESSOR_ARCHITECTURE=x86
  PROCESSOR_IDENTIFIER=x86 Family 6 Model 23 Stepping 10, GenuineIntel
  PROCESSOR_LEVEL=6
  PROCESSOR_REVISION=170a
  PROGRAMDATA=C:\ProgramData
  PROGRAMFILES=C:\Program Files
  PROMPT=$P$G
  PSMODULEPATH=C:\Windows\system32\WindowsPowerShell\v1.0\Modules\
  PUBLIC=C:\Users\Public
  SYSTEMDRIVE=C:
  SYSTEMROOT=C:\Windows
  TEMP=C:\Users\cltbld\AppData\Local\Temp
  TMP=C:\Users\cltbld\AppData\Local\Temp
  USERDOMAIN=TOOLS-R3-W7-001
  USERNAME=cltbld
  USERPROFILE=C:\Users\cltbld
  WINDIR=C:\Windows
  XPCOM_DEBUG_BREAK=warn
 closing stdin
 using PTY: False
Traceback (most recent call last):
  File "mozmill\Scripts\mozmill-script.py", line 8, in <module>
    load_entry_point('mozmill==1.5.0', 'console_scripts', 'mozmill')()
  File "c:\talos-slave\mozilla-central_win7_test-mozmill-all\build\mozmill\Lib\site-packages\mozmill\__init__.py", line 810, in cli
    CLI().run()
  File "c:\talos-slave\mozilla-central_win7_test-mozmill-all\build\mozmill\Lib\site-packages\mozmill\__init__.py", line 758, in run
    self.mozmill.start(runner=runner, profile=runner.profile)
  File "c:\talos-slave\mozilla-central_win7_test-mozmill-all\build\mozmill\Lib\site-packages\mozmill\__init__.py", line 242, in start
    self.runner.start()
  File "c:\talos-slave\mozilla-central_win7_test-mozmill-all\build\mozmill\Lib\site-packages\mozrunner\__init__.py", line 446, in start
    self.process_handler = run_command(self.command+self.cmdargs, self.env, **self.kp_kwargs)
  File "c:\talos-slave\mozilla-central_win7_test-mozmill-all\build\mozmill\Lib\site-packages\mozrunner\__init__.py", line 89, in run_command
    return killableprocess.Popen(cmd, env=env, **killable_kwargs)
  File "C:\mozilla-build\Python25\lib\subprocess.py", line 593, in __init__
    errread, errwrite)
  File "c:\talos-slave\mozilla-central_win7_test-mozmill-all\build\mozmill\Lib\site-packages\mozrunner\killableprocess.py", line 149, in _execute_child
    winprocess.AssignProcessToJobObject(self._job, hp)
  File "c:\talos-slave\mozilla-central_win7_test-mozmill-all\build\mozmill\Lib\site-packages\mozrunner\winprocess.py", line 49, in ErrCheckBool
    raise WinError()
WindowsError: [Error 13] Access is denied.

command timed out: 600 seconds without output
program finished with exit code 1
elapsedTime=600.633000
"""

Again, I'm not sure why this fails but I'll work on reproducing it interactively.

As an aside, it may (or may not) make sense to put WithPathProperties (probably renamed to the more semantic PathWithProperties) inside of buildbotcustom.  I'm not sure if this solves anything, and have heard such conflicting stories both about how different versions of windows handle reversed slashes as well as how our test harnesses handle reversed slashes that I'm not sure if this is actually a potential issue with our system or not.  If it is, something like WithPathProperties should be incorporated.
Shelling into the windows slave on the tools-staging environment and running the identical command from the log in https://bugzilla.mozilla.org/show_bug.cgi?id=516984#c182 , I do not get the error.  mozmill runs correctly with 27 passing tests and one failing test.

Note that my command does not include the initial `C:\Windows\system32\cmd.exe /c`.  I'll add this next and see what happens
(In reply to comment #183)
> Shelling into the windows slave on the tools-staging environment and running
> the identical command from the log in
> https://bugzilla.mozilla.org/show_bug.cgi?id=516984#c182 , I do not get the
> error.  mozmill runs correctly with 27 passing tests and one failing test.
> 
> Note that my command does not include the initial `C:\Windows\system32\cmd.exe
> /c`.  I'll add this next and see what happens

Nope!  This works too and I even get 28 passing tests (not sure if one is intermittent on windows or not...works consistently on linux).

So there is some subtle difference between running these from the prompt and running these via buildbot.  I have no idea what.   I also have no idea how to debug any further
Instead, can you teach the harness how to understand unix paths on windows and use a relative path for the -b argument to mozmill?

posixpath and ntpath both have split and join which could be used to decompose the unix paths and recreate them as windows paths.
(In reply to comment #185)
> Instead, can you teach the harness how to understand unix paths on windows and
> use a relative path for the -b argument to mozmill?
> 
> posixpath and ntpath both have split and join which could be used to decompose
> the unix paths and recreate them as windows paths.

So, a few issues here.

Firstly, what makes you suspect its a path issue?  As documented above, I've tried commands with pure windows paths, commands with pure unix paths, commands with absolute paths, commands with only relative paths, commands from the mozmill directory, and commands from the build directory. I've also tried this in various combinations.  The WindowsError: Access denied may mean its a path issue....or not.  I'm not sure.  

I can run the exact same command that buildbot sends on the same slave from the command prompt, either using cmd.exe \c <command> (as buildbot does, via ShellCommandReportTimeout) or just invoking the body of the command (mozmill\Scripts\python -E ..., see log).  I use identical slashes for buildbot and....the tests run fine.  Again, see above.  So there is some difference between how buildbot is invoking the command and how invoking the command from the prompt works.  From the point of view of debugging, this difference is what is critical to understand.  If I can reproduce this locally, I can reduce turnaround time from altering code to testing a change to be a few seconds.  Currently, the reproductions steps are:

[0) edit code]
1) make package-tests
2) upload package tests to a url
3) run sendchanges.py with that url and the url of a build and symbols
4) wait for build process to complete

So its at least 20 min turnaround to test a single change.  If I could repro locally, then at least my guess+check would be faster ;)

Also, does it make more sense for the harness to detect paths sent for the other operating systems or for buildbot to send paths appropriate to the operating system?  In my opinion the latter, hence WithPathProperties from 182 (and the Path class).  Again, since I have fixed the paths buildbot-side which show up as expected in the logs, I don't think this is a path issue (though I'm not sure where the failure is, so I wouldn't want to rule it out).  I'd hate to modify the harness both as a one-off where the path logic can be done on the buildbot side and also without confidence that this is where the failure is.

As such, I'm going to throw a bunch of output into mozrunner, where the failure occurs, and test with 30 min turnaround times.  Not efficient, but hopefully informative.
(In reply to comment #186)
> (In reply to comment #185)
> > Instead, can you teach the harness how to understand unix paths on windows and
> > use a relative path for the -b argument to mozmill?
> > 
> > posixpath and ntpath both have split and join which could be used to decompose
> > the unix paths and recreate them as windows paths.
> 
> So, a few issues here.
> 
> Firstly, what makes you suspect its a path issue?  As documented above, I've
> tried commands with pure windows paths, commands with pure unix paths, commands
> with absolute paths, commands with only relative paths, commands from the
> mozmill directory, and commands from the build directory. I've also tried this
> in various combinations.  The WindowsError: Access denied may mean its a path
> issue....or not.  I'm not sure.  

I was under the impression after our conversation that it was a path issue.  That comment was submitted from a stale form before you had made your comments above.

> I can run the exact same command that buildbot sends on the same slave from the
> command prompt, either using cmd.exe \c <command> 

isn't that /c?

The relevant code is

http://hg.mozilla.org/build/buildbot/file/5e4ed40eafd2/slave/buildslave/commands/base.py#l424

and 

http://hg.mozilla.org/build/buildbot/file/5e4ed40eafd2/slave/buildslave/commands/base.py#l524

> (as buildbot does, via
> ShellCommandReportTimeout) or just invoking the body of the command
> (mozmill\Scripts\python -E ..., see log).  I use identical slashes for buildbot
> and....the tests run fine.  Again, see above.  So there is some difference
> between how buildbot is invoking the command and how invoking the command from
> the prompt works.  From the point of view of debugging, this difference is what
> is critical to understand.  If I can reproduce this locally, I can reduce
> turnaround time from altering code to testing a change to be a few seconds. 
> Currently, the reproductions steps are:
> 
> [0) edit code]
> 1) make package-tests
> 2) upload package tests to a url
> 3) run sendchanges.py with that url and the url of a build and symbols
> 4) wait for build process to complete
> 
> So its at least 20 min turnaround to test a single change.  If I could repro
> locally, then at least my guess+check would be faster ;)

I am sure it would be :)

> Also, does it make more sense for the harness to detect paths sent for the
> other operating systems or for buildbot to send paths appropriate to the
> operating system?  In my opinion the latter, hence WithPathProperties from 182
> (and the Path class).  Again, since I have fixed the paths buildbot-side which
> show up as expected in the logs, I don't think this is a path issue (though I'm
> not sure where the failure is, so I wouldn't want to rule it out).  I'd hate to
> modify the harness both as a one-off where the path logic can be done on the
> buildbot side and also without confidence that this is where the failure is.

If using unix style paths is an assumed feature of the build system, then I'd think that including it in the harness makes most sense.  Is there a plan to use |make mozmill| in the objdir at some point?  Is it going to special case windows?

Do other suites translate paths internally?

I would prefer being consistent with the other suites.  I haven't hacked on those harnesses so I can't comment to this.

> As such, I'm going to throw a bunch of output into mozrunner, where the failure
> occurs, and test with 30 min turnaround times.  Not efficient, but hopefully
> informative.

Sounds like a useful debugging step.  Be mindful of how you are dumping your information.  If you are making harness-specific changes, then you should be able to print to stdout.  Would this information be generally useful?

If you are hoping to print things out in buildbot land, it could show up in the master twistd.log or the slaves twistd.log depending on whether the code runs on the remote object or the master objects.

Unless we are able to get this in a working state, I am inclined to deploy this to linux and mac and leave windows off the plate.  Does this sound ok to you?
(In reply to comment #187)
> (In reply to comment #186)
> > (In reply to comment #185)
> > > Instead, can you teach the harness how to understand unix paths on windows and
> > > use a relative path for the -b argument to mozmill?
> > > 
> > > posixpath and ntpath both have split and join which could be used to decompose
> > > the unix paths and recreate them as windows paths.
> > 
> > So, a few issues here.
> > 
> > Firstly, what makes you suspect its a path issue?  As documented above, I've
> > tried commands with pure windows paths, commands with pure unix paths, commands
> > with absolute paths, commands with only relative paths, commands from the
> > mozmill directory, and commands from the build directory. I've also tried this
> > in various combinations.  The WindowsError: Access denied may mean its a path
> > issue....or not.  I'm not sure.  
> 
> I was under the impression after our conversation that it was a path issue. 
> That comment was submitted from a stale form before you had made your comments
> above.
> 
> > I can run the exact same command that buildbot sends on the same slave from the
> > command prompt, either using cmd.exe \c <command> 
> 
> isn't that /c?

Yeah, sorry, /c

> 
> The relevant code is
> 
> http://hg.mozilla.org/build/buildbot/file/5e4ed40eafd2/slave/buildslave/commands/base.py#l424
> 
> and 
> 
> http://hg.mozilla.org/build/buildbot/file/5e4ed40eafd2/slave/buildslave/commands/base.py#l524
> 
> > (as buildbot does, via
> > ShellCommandReportTimeout) or just invoking the body of the command
> > (mozmill\Scripts\python -E ..., see log).  I use identical slashes for buildbot
> > and....the tests run fine.  Again, see above.  So there is some difference
> > between how buildbot is invoking the command and how invoking the command from
> > the prompt works.  From the point of view of debugging, this difference is what
> > is critical to understand.  If I can reproduce this locally, I can reduce
> > turnaround time from altering code to testing a change to be a few seconds. 
> > Currently, the reproductions steps are:
> > 
> > [0) edit code]
> > 1) make package-tests
> > 2) upload package tests to a url
> > 3) run sendchanges.py with that url and the url of a build and symbols
> > 4) wait for build process to complete
> > 
> > So its at least 20 min turnaround to test a single change.  If I could repro
> > locally, then at least my guess+check would be faster ;)
> 
> I am sure it would be :)
> 
> > Also, does it make more sense for the harness to detect paths sent for the
> > other operating systems or for buildbot to send paths appropriate to the
> > operating system?  In my opinion the latter, hence WithPathProperties from 182
> > (and the Path class).  Again, since I have fixed the paths buildbot-side which
> > show up as expected in the logs, I don't think this is a path issue (though I'm
> > not sure where the failure is, so I wouldn't want to rule it out).  I'd hate to
> > modify the harness both as a one-off where the path logic can be done on the
> > buildbot side and also without confidence that this is where the failure is.
> 
> If using unix style paths is an assumed feature of the build system, then I'd
> think that including it in the harness makes most sense.  Is there a plan to
> use |make mozmill| in the objdir at some point?  Is it going to special case
> windows?
> 
> Do other suites translate paths internally?
> 
> I would prefer being consistent with the other suites.  I haven't hacked on
> those harnesses so I can't comment to this.

I haven't seen code in the other harnesses that does this.  I haven't done a thorough audit, and could have therefore missed something, but ctalbert seems convinced that this is not the case.

> > As such, I'm going to throw a bunch of output into mozrunner, where the failure
> > occurs, and test with 30 min turnaround times.  Not efficient, but hopefully
> > informative.
> 
> Sounds like a useful debugging step.  Be mindful of how you are dumping your
> information.  If you are making harness-specific changes, then you should be
> able to print to stdout.  Would this information be generally useful?

I am modifying mozrunner in-tree, building, putting on people.m.o/~jhammel/builds and sending changes.  I am sending output to stdout so that it is displayed in the build step output in buildbot.  No, this information is not generally useful.  It is only useful for debugging this one issue.

> If you are hoping to print things out in buildbot land, it could show up in the
> master twistd.log or the slaves twistd.log depending on whether the code runs
> on the remote object or the master objects.
> 
> Unless we are able to get this in a working state, I am inclined to deploy this
> to linux and mac and leave windows off the plate.  Does this sound ok to you?

I'll have to defer to ctalbert for this decision.
Still trying to track down the bug.  Thus far, my debugging output hasn't shown anything unexpected.  Could it be because firefox (as invoked via mozrunner) is attempting to open a jsbridge port (24242)?  I am not sure what, exactly, access is denied to.  My initial assumption about paths was based on conversations with you and clint.  While my tests haven't refuted this assumption (as no one really knows what the problem is), the replacement of '/'s with '\'s imply it is at least not in that class of path issues where slash direction matters.
(In reply to comment #188)
<snip/>
> If using unix style paths is an assumed feature of the build system, then I'd
> think that including it in the harness makes most sense.  Is there a plan to
> use |make mozmill| in the objdir at some point?  

Yes, though there hasn't been much urgency to do it: https://bugzilla.mozilla.org/show_bug.cgi?id=568642

> Is it going to special case windows?

I would assume not.
Just an update on what the problem is on windows:  pretty much literally what the message says; that is, for reasons beyond my windows expertise, when running via buildbot (on the slave), mozrunner cannot bind a created process to a job object.  It fails here:

http://mxr.mozilla.org/mozilla-central/source/testing/mozmill/mozrunner-2.5.1/mozrunner/killableprocess.py#149

Why this works interactively but not via buildbot is beyond me.  Are there any security or environmental issues that would result in access denied on windows for this operation running via a slave vs running interactively?  I'm not knowledgeable enough about either how buildbot works or how windows security works to really have much insight here.
Depends on: 600321
(In reply to comment #191)
> Just an update on what the problem is on windows:  pretty much literally what
> the message says; that is, for reasons beyond my windows expertise, when
> running via buildbot (on the slave), mozrunner cannot bind a created process to
> a job object.  It fails here:
> 
> http://mxr.mozilla.org/mozilla-central/source/testing/mozmill/mozrunner-2.5.1/mozrunner/killableprocess.py#149
> 
> Why this works interactively but not via buildbot is beyond me.  Are there any
> security or environmental issues that would result in access denied on windows
> for this operation running via a slave vs running interactively?  I'm not
> knowledgeable enough about either how buildbot works or how windows security
> works to really have much insight here.

The problem is now diagnosed, though the solution has yet to be agreed upon and implemented.

The problem occurs when you are in a process that is already part of a windows job object that tries AssignProcessToJobObject (see the mxr link to mozrunner above).

We are in a job object already because we patch twisted (which is what launches processes on the buildslaves) to put *those* processes in a job object: 

http://people.mozilla.org/~anodelman/killprocess/_dumbwin32proc.py

As best I can tell, this patching of twisted isn't documented anywhere, so it was by fortune, good grace, and happening to talk about the issue with :anodelman that this could be tracked down short of auditing the slave-side code.

Undoing the patch causes the tests to be run correctly on windows;  not only that, but they all pass.  Who knew?

Obviously this is not a production solution.  Working on it...
(In reply to comment #192)
> As best I can tell, this patching of twisted isn't documented anywhere, so it
> was by fortune, good grace, and happening to talk about the issue with
> :anodelman that this could be tracked down short of auditing the slave-side
> code.

https://wiki.mozilla.org/ReferencePlatforms/Win32#Downgrade_twisted_and_apply_process_group_patch

and 

http://hg.mozilla.org/build/twisted/rev/510653655551

> Undoing the patch causes the tests to be run correctly on windows;  not only
> that, but they all pass.  Who knew?
> 
> Obviously this is not a production solution.  Working on it...

In the case the next downtime happens before this issue is fixed in mozilla-central, should we land macosx and linux configs, leaving win32 for a follow up?
Depends on: 600736
(In reply to comment #193)
> In the case the next downtime happens before this issue is fixed in
> mozilla-central, should we land macosx and linux configs, leaving win32 for a
> follow up?
I think we'll be ready on all three platforms by next thursday.  When is the next downtime?
I am not sure when the downtime will be.  If the fix is likely to be ready for next thursday, I'd be inclined to wait until it is fixed on windows to land all platforms in production simultaneously.

Is this going to require a change to the twisted installation?

Because it is hard to schedule downtime these days, I am going to run this through production once more closer to the downtime to double check that nothing has broken while waiting.
We aren't going to include this in the next downtime.

From #qa:

[10:24:41] <jhford> ctalbert: hey, looks like there is a downtime happening on monday
[10:24:57] <jhford> not sure what to do re: mozmill
[10:25:05] <ctalbert> jhford: yeah, I'm not sure mozmill will be ready by monday
[10:25:58] <ctalbert> We are nearly complete with the changes for windows, but we found a test that only fails on buildbot on mac.  So we need to investigate that before going live, and I don't think it makes much sense to go live with only linux and then have you do all that work again in the next downtime.
[10:26:03] <ctalbert> jhford: ^
[10:26:17] * jhford agrees and thanks you :)
(In reply to comment #196)
> [10:25:58] <ctalbert> We are nearly complete with the changes for windows, but
> we found a test that only fails on buildbot on mac.  So we need to investigate
> that before going live, and I don't think it makes much sense to go live with
> only linux and then have you do all that work again in the next downtime.

Clint, which test are you talking about? Would be interested to know.
(In reply to comment #197)
> (In reply to comment #196)
> > [10:25:58] <ctalbert> We are nearly complete with the changes for windows, but
> > we found a test that only fails on buildbot on mac.  So we need to investigate
> > that before going live, and I don't think it makes much sense to go live with
> > only linux and then have you do all that work again in the next downtime.
> 
> Clint, which test are you talking about? Would be interested to know.
The private browsing tests fail every time we run them on mac in buildbot.  We haven't had time to track down why yet.  We also haven't tried running them on the mac slave itself without going through buildbot, that's the next step.  I can verify that they run fine on my mac locally, so it is either something due to slave configuration (I hope) or to the way buildbot works on mac (aka another issue like the one we've been fighting on windows)
(In reply to comment #198)
> The private browsing tests fail every time we run them on mac in buildbot.  We
> haven't had time to track down why yet.  We also haven't tried running them on

Once you have a change please let us know about the exact line of the failure and the exception message itself.
(In reply to comment #199)
> (In reply to comment #198)
> > The private browsing tests fail every time we run them on mac in buildbot.  We
> > haven't had time to track down why yet.  We also haven't tried running them on
> 
> Once you have a change please let us know about the exact line of the failure
> and the exception message itself.

The failures may be seen at http://tools-staging-master.mv.mozilla.com:8010/builders/Rev3%20MacOSX%20Snow%20Leopard%2010.6.2%20mozilla-central%20opt%20test%20mozmill-all/builds/18/steps/run_mozmill/logs/stdio on the MV VPN.  Pasting here for convenience:

mozmill/bin/python -E mozmill/bin/mozmill --showall -b ./Minefield.app/Contents/MacOS/firefox-bin -t mozmill/tests/firefox
 in dir /Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build (timeout 600 secs)
 watching logfiles {}
 argv: ['mozmill/bin/python', '-E', 'mozmill/bin/mozmill', '--showall', '-b', './Minefield.app/Contents/MacOS/firefox-bin', '-t', 'mozmill/tests/firefox']
 environment:
  Apple_PubSub_Socket_Render=/tmp/launch-8CAQPP/Render
  DISPLAY=/tmp/launch-U5YWAV/:0
  HOME=/Users/cltbld
  LOGNAME=cltbld
  PATH=/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin:/usr/X11/bin
  PWD=/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build
  PYTHONPATH=/Library/Python/2.5/site-packages
  SHELL=/bin/bash
  SSH_AUTH_SOCK=/tmp/launch-oXWTHe/Listeners
  TMPDIR=/var/folders/H5/H5TD8hgwEqKq9hgKlayjWU+++TM/-Tmp-/
  USER=cltbld
  VERSIONER_PYTHON_PREFER_32_BIT=no
  VERSIONER_PYTHON_VERSION=2.6
 closing stdin
 using PTY: True
2010-09-30 16:40:34.935 firefox-bin[20488:903] invalid pixel format
2010-09-30 16:40:34.938 firefox-bin[20488:903] invalid context
2010-09-30 16:40:34.938 firefox-bin[20488:903] invalid pixel format
2010-09-30 16:40:34.939 firefox-bin[20488:903] invalid context
DEBUG | mozmill.startRunner | true
DEBUG | mozmill.setState | "currentState"
DEBUG | mozmill.setModule | "currentModule"
DEBUG | mozmill.setState | "currentState"
TEST-START | /Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testAwesomeBar/testFaviconInAutocomplete.js | setupModule
DEBUG | mozmill.setTest | {"name": "setupModule", "filename": "/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testAwesomeBar/testFaviconInAutocomplete.js"}
TEST-PASS | /Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testAwesomeBar/testFaviconInAutocomplete.js | setupModule
DEBUG | mozmill.endTest | {"passes": [], "fails": [], "name": "setupModule", "filename": "/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testAwesomeBar/testFaviconInAutocomplete.js", "failed": 0, "passed": 0}
DEBUG | mozmill.setState | "currentState"
TEST-START | /Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testAwesomeBar/testFaviconInAutocomplete.js | testFaviconInAutoComplete
DEBUG | mozmill.setTest | {"name": "testFaviconInAutoComplete", "filename": "/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testAwesomeBar/testFaviconInAutocomplete.js"}
2010-09-30 16:40:43.284 firefox-bin[20488:903] invalid pixel format
2010-09-30 16:40:43.284 firefox-bin[20488:903] invalid context
2010-09-30 16:40:43.285 firefox-bin[20488:903] invalid pixel format
2010-09-30 16:40:43.285 firefox-bin[20488:903] invalid context
INFO | Step Pass: {"function": "controller.waitFor()"}
INFO | Step Pass: {"function": "Controller.waitForElement()"}
INFO | Step Pass: {"function": "Controller.click()"}
INFO | Step Pass: {"function": "controller.waitForEval()"}
INFO | Step Pass: {"function": "controller.waitForEval()"}
INFO | Step Pass: {"function": "controller.waitFor()"}
INFO | Step Pass: {"function": "Controller.waitForElement()"}
INFO | Step Pass: {"function": "Controller.select()"}
INFO | Step Pass: {"function": "Controller.keypress()"}
INFO | Step Pass: {"function": "Controller.keypress()"}
INFO | Step Pass: {"function": "controller.waitForEval()"}
INFO | Step Pass: {"function": "Controller.type()"}
INFO | Step Pass: {"function": "Controller.keypress()"}
INFO | Step Pass: {"function": "controller.waitFor()"}
INFO | Step Pass: {"function": "controller.waitForPageLoad()"}
INFO | Step Pass: {"function": "Controller.keypress()"}
INFO | Step Pass: {"function": "controller.waitForEval()"}
INFO | Step Pass: {"function": "Controller.keypress()"}
INFO | Step Pass: {"function": "controller.waitForEval()"}
INFO | Step Pass: {"function": "Controller.type()"}
2010-09-30 16:40:51.264 firefox-bin[20488:903] invalid pixel format
2010-09-30 16:40:51.264 firefox-bin[20488:903] invalid context
2010-09-30 16:40:51.265 firefox-bin[20488:903] invalid pixel format
2010-09-30 16:40:51.266 firefox-bin[20488:903] invalid context
INFO | Step Pass: {"function": "Controller.type()"}
INFO | Step Pass: {"function": "Controller.type()"}
INFO | Step Pass: {"function": "Controller.type()"}
INFO | Step Pass: {"function": "Controller.type()"}
INFO | Step Pass: {"function": "Controller.type()"}
INFO | Step Pass: {"function": "Controller.type()"}
INFO | Step Pass: {"function": "controller.waitForEval()"}
INFO | Step Pass: {"function": "controller.assertJS('subject.isSameFavicon == true')"}
TEST-PASS | /Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testAwesomeBar/testFaviconInAutocomplete.js | testFaviconInAutoComplete
DEBUG | mozmill.endTest | {"passes": [{"function": "controller.waitFor()"}, {"function": "Controller.waitForElement()"}, {"function": "Controller.click()"}, {"function": "controller.waitForEval()"}, {"function": "controller.waitForEval()"}, {"function": "controller.waitFor()"}, {"function": "Controller.waitForElement()"}, {"function": "Controller.select()"}, {"function": "Controller.keypress()"}, {"function": "Controller.keypress()"}, {"function": "controller.waitForEval()"}, {"function": "Controller.type()"}, {"function": "Controller.keypress()"}, {"function": "controller.waitFor()"}, {"function": "controller.waitForPageLoad()"}, {"function": "Controller.keypress()"}, {"function": "controller.waitForEval()"}, {"function": "Controller.keypress()"}, {"function": "controller.waitForEval()"}, {"function": "Controller.type()"}, {"function": "Controller.type()"}, {"function": "Controller.type()"}, {"function": "Controller.type()"}, {"function": "Controller.type()"}, {"function": "Controller.type()"}, {"function": "Controller.type()"}, {"function": "controller.waitForEval()"}, {"function": "controller.assertJS('subject.isSameFavicon == true')"}], "fails": [], "name": "testFaviconInAutoComplete", "filename": "/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testAwesomeBar/testFaviconInAutocomplete.js", "failed": 0, "passed": 28}
DEBUG | mozmill.setState | "currentState"
DEBUG | mozmill.setModule | "currentModule"
DEBUG | mozmill.setState | "currentState"
TEST-START | /Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testAwesomeBar/testGoButton.js | setupModule
DEBUG | mozmill.setTest | {"name": "setupModule", "filename": "/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testAwesomeBar/testGoButton.js"}
TEST-PASS | /Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testAwesomeBar/testGoButton.js | setupModule
DEBUG | mozmill.endTest | {"passes": [], "fails": [], "name": "setupModule", "filename": "/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testAwesomeBar/testGoButton.js", "failed": 0, "passed": 0}
DEBUG | mozmill.setState | "currentState"
TEST-START | /Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testAwesomeBar/testGoButton.js | testGoButtonOnTypeOnly
DEBUG | mozmill.setTest | {"name": "testGoButtonOnTypeOnly", "filename": "/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testAwesomeBar/testGoButton.js"}
INFO | Step Pass: {"function": "Controller.open()"}
INFO | Step Pass: {"function": "controller.waitFor()"}
INFO | Step Pass: {"function": "controller.waitForPageLoad()"}
INFO | Step Pass: {"function": "controller.assertJS('subject.visible == subject.expectedVisibility')"}
INFO | Step Pass: {"function": "Controller.keypress()"}
INFO | Step Pass: {"function": "controller.waitForEval()"}
INFO | Step Pass: {"function": "Controller.type()"}
INFO | Step Pass: {"function": "controller.assertJS('subject.visible == subject.expectedVisibility')"}
INFO | Step Pass: {"function": "Controller.keypress()"}
INFO | Step Pass: {"function": "controller.waitForEval()"}
INFO | Step Pass: {"function": "Controller.keypress()"}
INFO | Step Pass: {"function": "controller.waitForEval()"}
INFO | Step Pass: {"function": "Controller.keypress()"}
INFO | Step Pass: {"function": "controller.assertJS('subject.visible == subject.expectedVisibility')"}
TEST-PASS | /Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testAwesomeBar/testGoButton.js | testGoButtonOnTypeOnly
DEBUG | mozmill.endTest | {"passes": [{"function": "Controller.open()"}, {"function": "controller.waitFor()"}, {"function": "controller.waitForPageLoad()"}, {"function": "controller.assertJS('subject.visible == subject.expectedVisibility')"}, {"function": "Controller.keypress()"}, {"function": "controller.waitForEval()"}, {"function": "Controller.type()"}, {"function": "controller.assertJS('subject.visible == subject.expectedVisibility')"}, {"function": "Controller.keypress()"}, {"function": "controller.waitForEval()"}, {"function": "Controller.keypress()"}, {"function": "controller.waitForEval()"}, {"function": "Controller.keypress()"}, {"function": "controller.assertJS('subject.visible == subject.expectedVisibility')"}], "fails": [], "name": "testGoButtonOnTypeOnly", "filename": "/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testAwesomeBar/testGoButton.js", "failed": 0, "passed": 14}
DEBUG | mozmill.setState | "currentState"
TEST-START | /Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testAwesomeBar/testGoButton.js | testClickLocationBarAndGo
DEBUG | mozmill.setTest | {"name": "testClickLocationBarAndGo", "filename": "/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testAwesomeBar/testGoButton.js"}
INFO | Step Pass: {"function": "Controller.open()"}
INFO | Step Pass: {"function": "controller.waitFor()"}
INFO | Step Pass: {"function": "controller.waitForPageLoad()"}
INFO | Step Pass: {"function": "Controller.keypress()"}
INFO | Step Pass: {"function": "controller.waitForEval()"}
INFO | Step Pass: {"function": "Controller.type()"}
INFO | Step Pass: {"function": "Controller.click()"}
INFO | Step Pass: {"function": "controller.waitForPageLoad()"}
INFO | Step Pass: {"function": "Controller.assertNode()"}
INFO | Step Pass: {"function": "controller.assertJS('subject.visible == subject.expectedVisibility')"}
INFO | Step Pass: {"function": "Controller.assertValue()"}
TEST-PASS | /Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testAwesomeBar/testGoButton.js | testClickLocationBarAndGo
DEBUG | mozmill.endTest | {"passes": [{"function": "Controller.open()"}, {"function": "controller.waitFor()"}, {"function": "controller.waitForPageLoad()"}, {"function": "Controller.keypress()"}, {"function": "controller.waitForEval()"}, {"function": "Controller.type()"}, {"function": "Controller.click()"}, {"function": "controller.waitForPageLoad()"}, {"function": "Controller.assertNode()"}, {"function": "controller.assertJS('subject.visible == subject.expectedVisibility')"}, {"function": "Controller.assertValue()"}], "fails": [], "name": "testClickLocationBarAndGo", "filename": "/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testAwesomeBar/testGoButton.js", "failed": 0, "passed": 11}
DEBUG | mozmill.setState | "currentState"
DEBUG | mozmill.setModule | "currentModule"
DEBUG | mozmill.setState | "currentState"
TEST-START | /Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPreferences/testSetToCurrentPage.js | setupModule
DEBUG | mozmill.setTest | {"name": "setupModule", "filename": "/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPreferences/testSetToCurrentPage.js"}
INFO | Step Pass: {"function": "controller.waitForEval()"}
INFO | Step Pass: {"function": "Controller.open()"}
INFO | Step Pass: {"function": "controller.waitFor()"}
INFO | Step Pass: {"function": "controller.waitForPageLoad()"}
TEST-PASS | /Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPreferences/testSetToCurrentPage.js | setupModule
DEBUG | mozmill.endTest | {"passes": [{"function": "controller.waitForEval()"}, {"function": "Controller.open()"}, {"function": "controller.waitFor()"}, {"function": "controller.waitForPageLoad()"}], "fails": [], "name": "setupModule", "filename": "/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPreferences/testSetToCurrentPage.js", "failed": 0, "passed": 4}
DEBUG | mozmill.setState | "currentState"
TEST-START | /Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPreferences/testSetToCurrentPage.js | testSetHomePage
DEBUG | mozmill.setTest | {"name": "testSetHomePage", "filename": "/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPreferences/testSetToCurrentPage.js"}
INFO | Step Pass: {"function": "Controller.open()"}
INFO | Step Pass: {"function": "controller.waitFor()"}
INFO | Step Pass: {"function": "controller.waitForPageLoad()"}
INFO | Step Pass: {"function": "Controller.assertNode()"}
2010-09-30 16:40:58.167 firefox-bin[20488:903] invalid pixel format
2010-09-30 16:40:58.167 firefox-bin[20488:903] invalid context
2010-09-30 16:40:58.170 firefox-bin[20488:903] invalid pixel format
2010-09-30 16:40:58.171 firefox-bin[20488:903] invalid context
INFO | Step Pass: {"function": "controller.waitFor()"}
INFO | Step Pass: {"function": "Controller.waitForElement()"}
INFO | Step Pass: {"function": "Controller.click()"}
INFO | Step Pass: {"function": "controller.waitForEval()"}
INFO | Step Pass: {"function": "controller.waitForEval()"}
INFO | Step Pass: {"function": "Controller.click()"}
INFO | Step Pass: {"function": "Controller.keypress()"}
TEST-PASS | /Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPreferences/testSetToCurrentPage.js | testSetHomePage
DEBUG | mozmill.endTest | {"passes": [{"function": "Controller.open()"}, {"function": "controller.waitFor()"}, {"function": "controller.waitForPageLoad()"}, {"function": "Controller.assertNode()"}, {"function": "controller.waitFor()"}, {"function": "Controller.waitForElement()"}, {"function": "Controller.click()"}, {"function": "controller.waitForEval()"}, {"function": "controller.waitForEval()"}, {"function": "Controller.click()"}, {"function": "Controller.keypress()"}], "fails": [], "name": "testSetHomePage", "filename": "/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPreferences/testSetToCurrentPage.js", "failed": 0, "passed": 11}
DEBUG | mozmill.setState | "currentState"
TEST-START | /Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPreferences/testSetToCurrentPage.js | testHomeButton
DEBUG | mozmill.setTest | {"name": "testHomeButton", "filename": "/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPreferences/testSetToCurrentPage.js"}
INFO | Step Pass: {"function": "Controller.open()"}
INFO | Step Pass: {"function": "controller.waitFor()"}
INFO | Step Pass: {"function": "controller.waitForPageLoad()"}
INFO | Step Pass: {"function": "Controller.click()"}
INFO | Step Pass: {"function": "controller.waitForPageLoad()"}
INFO | Step Pass: {"function": "Controller.assertValue()"}
TEST-PASS | /Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPreferences/testSetToCurrentPage.js | testHomeButton
DEBUG | mozmill.endTest | {"passes": [{"function": "Controller.open()"}, {"function": "controller.waitFor()"}, {"function": "controller.waitForPageLoad()"}, {"function": "Controller.click()"}, {"function": "controller.waitForPageLoad()"}, {"function": "Controller.assertValue()"}], "fails": [], "name": "testHomeButton", "filename": "/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPreferences/testSetToCurrentPage.js", "failed": 0, "passed": 6}
DEBUG | mozmill.setState | "currentState"
TEST-START | /Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPreferences/testSetToCurrentPage.js | teardownModule
DEBUG | mozmill.setTest | {"name": "teardownModule", "filename": "/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPreferences/testSetToCurrentPage.js"}
TEST-PASS | /Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPreferences/testSetToCurrentPage.js | teardownModule
DEBUG | mozmill.endTest | {"passes": [], "fails": [], "name": "teardownModule", "filename": "/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPreferences/testSetToCurrentPage.js", "failed": 0, "passed": 0}
DEBUG | mozmill.setState | "currentState"
DEBUG | mozmill.setModule | "currentModule"
DEBUG | mozmill.setState | "currentState"
TEST-START | /Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPrivateBrowsing/testCloseWindow.js | setupModule
DEBUG | mozmill.setTest | {"name": "setupModule", "filename": "/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPrivateBrowsing/testCloseWindow.js"}
INFO | Step Pass: {"function": "Controller.open()"}
INFO | Step Pass: {"function": "controller.waitFor()"}
INFO | Step Pass: {"function": "controller.waitForPageLoad()"}
TEST-PASS | /Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPrivateBrowsing/testCloseWindow.js | setupModule
DEBUG | mozmill.endTest | {"passes": [{"function": "Controller.open()"}, {"function": "controller.waitFor()"}, {"function": "controller.waitForPageLoad()"}], "fails": [], "name": "setupModule", "filename": "/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPrivateBrowsing/testCloseWindow.js", "failed": 0, "passed": 3}
DEBUG | mozmill.setState | "currentState"
TEST-START | /Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPrivateBrowsing/testCloseWindow.js | testCloseWindow
DEBUG | mozmill.setTest | {"name": "testCloseWindow", "filename": "/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPrivateBrowsing/testCloseWindow.js"}
INFO | Step Pass: {"function": "Controller.open()"}
INFO | Step Pass: {"function": "Controller.open()"}
INFO | Step Pass: {"function": "controller.waitFor()"}
INFO | Step Pass: {"function": "Controller.waitForElement()"}
INFO | Step Pass: {"function": "controller.waitFor()"}
INFO | Step Pass: {"function": "Controller.waitForElement()"}
INFO | Step Pass: {"function": "controller.waitForEval()"}
INFO | Step Pass: {"function": "controller.waitForEval()"}
INFO | Step Pass: {"function": "Controller.keypress()"}
INFO | Step Pass: {"function": "controller.waitForEval()"}
INFO | Step Pass: {"function": "controller.waitForEval()"}
2010-09-30 16:41:01.695 firefox-bin[20488:903] invalid pixel format
2010-09-30 16:41:01.695 firefox-bin[20488:903] invalid context
2010-09-30 16:41:01.696 firefox-bin[20488:903] invalid pixel format
2010-09-30 16:41:01.696 firefox-bin[20488:903] invalid context
TEST-PASS | /Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPrivateBrowsing/testCloseWindow.js | testCloseWindow
DEBUG | mozmill.endTest | {"passes": [{"function": "Controller.open()"}, {"function": "Controller.open()"}, {"function": "controller.waitFor()"}, {"function": "Controller.waitForElement()"}, {"function": "controller.waitFor()"}, {"function": "Controller.waitForElement()"}, {"function": "controller.waitForEval()"}, {"function": "controller.waitForEval()"}, {"function": "Controller.keypress()"}, {"function": "controller.waitForEval()"}, {"function": "controller.waitForEval()"}], "fails": [], "name": "testCloseWindow", "filename": "/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPrivateBrowsing/testCloseWindow.js", "failed": 0, "passed": 11}
DEBUG | mozmill.setState | "currentState"
TEST-START | /Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPrivateBrowsing/testCloseWindow.js | teardownModule
DEBUG | mozmill.setTest | {"name": "teardownModule", "filename": "/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPrivateBrowsing/testCloseWindow.js"}
TEST-PASS | /Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPrivateBrowsing/testCloseWindow.js | teardownModule
DEBUG | mozmill.endTest | {"passes": [], "fails": [], "name": "teardownModule", "filename": "/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPrivateBrowsing/testCloseWindow.js", "failed": 0, "passed": 0}
DEBUG | mozmill.setState | "currentState"
DEBUG | mozmill.setModule | "currentModule"
DEBUG | mozmill.setState | "currentState"
TEST-START | /Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPrivateBrowsing/testStartStopPBMode.js | setupModule
DEBUG | mozmill.setTest | {"name": "setupModule", "filename": "/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPrivateBrowsing/testStartStopPBMode.js"}
INFO | Step Pass: {"function": "controller.waitForEval()"}
INFO | Step Pass: {"function": "controller.waitForEval()"}
INFO | Step Pass: {"function": "Controller.open()"}
INFO | Step Pass: {"function": "controller.waitFor()"}
INFO | Step Pass: {"function": "controller.waitForPageLoad()"}
TEST-PASS | /Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPrivateBrowsing/testStartStopPBMode.js | setupModule
DEBUG | mozmill.endTest | {"passes": [{"function": "controller.waitForEval()"}, {"function": "controller.waitForEval()"}, {"function": "Controller.open()"}, {"function": "controller.waitFor()"}, {"function": "controller.waitForPageLoad()"}], "fails": [], "name": "setupModule", "filename": "/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPrivateBrowsing/testStartStopPBMode.js", "failed": 0, "passed": 5}
DEBUG | mozmill.setState | "currentState"
TEST-START | /Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPrivateBrowsing/testStartStopPBMode.js | testEnablePrivateBrowsingMode
DEBUG | mozmill.setTest | {"name": "testEnablePrivateBrowsingMode", "filename": "/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPrivateBrowsing/testStartStopPBMode.js"}
INFO | Step Pass: {"function": "Controller.open()"}
INFO | Step Pass: {"function": "Controller.open()"}
INFO | Step Pass: {"function": "controller.waitFor()"}
INFO | Step Pass: {"function": "Controller.waitForElement()"}
INFO | Step Pass: {"function": "controller.waitFor()"}
INFO | Step Pass: {"function": "Controller.waitForElement()"}
INFO | Step Pass: {"function": "controller.waitFor()"}
INFO | Step Pass: {"function": "Controller.waitForElement()"}
INFO | Step Pass: {"function": "Controller.click()"}
INFO | Step Pass: {"function": "Controller.click()"}
INFO | Step Pass: {"function": "controller.waitForEval()"}
INFO | Step Pass: {"function": "controller.waitForEval()"}
INFO | Step Pass: {"function": "controller.assertJS('subject.isOnlyOneTab == true')"}
INFO | Step Pass: {"function": "controller.assertJS('subject.hasTitleModifier == true')"}
INFO | Step Pass: {"function": "controller.waitFor()"}
INFO | Step Pass: {"function": "Controller.waitForElement()"}
INFO | Step Pass: {"function": "Controller.assertText()"}
INFO | Step Pass: {"function": "Controller.assertText()"}
TEST-PASS | /Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPrivateBrowsing/testStartStopPBMode.js | testEnablePrivateBrowsingMode
DEBUG | mozmill.endTest | {"passes": [{"function": "Controller.open()"}, {"function": "Controller.open()"}, {"function": "controller.waitFor()"}, {"function": "Controller.waitForElement()"}, {"function": "controller.waitFor()"}, {"function": "Controller.waitForElement()"}, {"function": "controller.waitFor()"}, {"function": "Controller.waitForElement()"}, {"function": "Controller.click()"}, {"function": "Controller.click()"}, {"function": "controller.waitForEval()"}, {"function": "controller.waitForEval()"}, {"function": "controller.assertJS('subject.isOnlyOneTab == true')"}, {"function": "controller.assertJS('subject.hasTitleModifier == true')"}, {"function": "controller.waitFor()"}, {"function": "Controller.waitForElement()"}, {"function": "Controller.assertText()"}, {"function": "Controller.assertText()"}], "fails": [], "name": "testEnablePrivateBrowsingMode", "filename": "/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPrivateBrowsing/testStartStopPBMode.js", "failed": 0, "passed": 18}
DEBUG | mozmill.setState | "currentState"
TEST-START | /Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPrivateBrowsing/testStartStopPBMode.js | testStopPrivateBrowsingMode
DEBUG | mozmill.setTest | {"name": "testStopPrivateBrowsingMode", "filename": "/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPrivateBrowsing/testStartStopPBMode.js"}
INFO | Step Pass: {"function": "controller.waitForEval()"}
INFO | Step Pass: {"function": "controller.waitForEval()"}
ERROR | Test Failure: {"exception": {"message": "this.controller.window.gBrowser is undefined", "lineNumber": 1015, "stack": "()@resource://mozmill/modules/controller.js:1015\n()@resource://mozmill/modules/frame.js -> file:///Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPrivateBrowsing/testStartStopPBMode.js:123\n((function () {pb.enabled = true;pb.stop();controller.assertJS(\"subject.allTabsRestored == true\", {allTabsRestored: controller.tabs.length == LOCAL_TEST_PAGES.length + 1});for (var i = 0; i < LOCAL_TEST_PAGES.length; i++) {var elem = new elementslib.ID(controller.tabs.getTab(i), LOCAL_TEST_PAGES[i].id);controller.waitForElement(elem, TIMEOUT);}controller.assertJS(\"subject.noTitleModifier == true\", {noTitleModifier: controller.window.document.title.indexOf(modifier) == -1});}))@resource://mozmill/modules/frame.js:514\n([object Object])@resource://mozmill/modules/frame.js:582\n([object Object])@resource://mozmill/modules/frame.js:625\n(\"/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox\")@resource://mozmill/modules/frame.js:455\n(\"/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox\")@resource://mozmill/modules/frame.js:631\n((function (dir, invokedFromIDE) {var runner = new Runner(new Collector, invokedFromIDE);runner.runTestDirectory(dir);runner.end();return true;}),[object Array])@resource://jsbridge/modules/server.js:164\n(\"2385bfb6-ccec-11df-be03-34159e186224\",(function (dir, invokedFromIDE) {var runner = new Runner(new Collector, invokedFromIDE);runner.runTestDirectory(dir);runner.end();return true;}),[object Array])@resource://jsbridge/modules/server.js:168\n@resource://jsbridge/modules/server.js:249\n", "fileName": "resource://mozmill/modules/controller.js"}}
TEST-UNEXPECTED-FAIL | /Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPrivateBrowsing/testStartStopPBMode.js | testStopPrivateBrowsingMode
DEBUG | mozmill.endTest | {"passes": [{"function": "controller.waitForEval()"}, {"function": "controller.waitForEval()"}], "fails": [{"exception": {"message": "this.controller.window.gBrowser is undefined", "lineNumber": 1015, "stack": "()@resource://mozmill/modules/controller.js:1015\n()@resource://mozmill/modules/frame.js -> file:///Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPrivateBrowsing/testStartStopPBMode.js:123\n((function () {pb.enabled = true;pb.stop();controller.assertJS(\"subject.allTabsRestored == true\", {allTabsRestored: controller.tabs.length == LOCAL_TEST_PAGES.length + 1});for (var i = 0; i < LOCAL_TEST_PAGES.length; i++) {var elem = new elementslib.ID(controller.tabs.getTab(i), LOCAL_TEST_PAGES[i].id);controller.waitForElement(elem, TIMEOUT);}controller.assertJS(\"subject.noTitleModifier == true\", {noTitleModifier: controller.window.document.title.indexOf(modifier) == -1});}))@resource://mozmill/modules/frame.js:514\n([object Object])@resource://mozmill/modules/frame.js:582\n([object Object])@resource://mozmill/modules/frame.js:625\n(\"/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox\")@resource://mozmill/modules/frame.js:455\n(\"/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox\")@resource://mozmill/modules/frame.js:631\n((function (dir, invokedFromIDE) {var runner = new Runner(new Collector, invokedFromIDE);runner.runTestDirectory(dir);runner.end();return true;}),[object Array])@resource://jsbridge/modules/server.js:164\n(\"2385bfb6-ccec-11df-be03-34159e186224\",(function (dir, invokedFromIDE) {var runner = new Runner(new Collector, invokedFromIDE);runner.runTestDirectory(dir);runner.end();return true;}),[object Array])@resource://jsbridge/modules/server.js:168\n@resource://jsbridge/modules/server.js:249\n", "fileName": "resource://mozmill/modules/controller.js"}}], "name": "testStopPrivateBrowsingMode", "filename": "/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPrivateBrowsing/testStartStopPBMode.js", "failed": 1, "passed": 2}
DEBUG | mozmill.setState | "currentState"
TEST-START | /Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPrivateBrowsing/testStartStopPBMode.js | testKeyboardShortcut
DEBUG | mozmill.setTest | {"name": "testKeyboardShortcut", "filename": "/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPrivateBrowsing/testStartStopPBMode.js"}
ERROR | Test Failure: {"exception": {"message": "Synthesizing key event failed. \n[Exception... \"Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIDOMWindowUtils.sendKeyEvent]\"  nsresult: \"0x80004005 (NS_ERROR_FAILURE)\"  location: \"JS frame :: resource://mozmill/stdlib/EventUtils.js :: synthesizeKey :: line 342\"  data: no]", "lineNumber": 136, "stack": "([object ChromeWindow],\"keypress\",\"P\",[object Object])@resource://mozmill/modules/events.js:136\n(null,\"P\",[object Object])@resource://mozmill/modules/controller.js:170\nprivateBrowsing_start(true)@resource://mozmill/modules/frame.js -> file:///Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/shared-modules/testPrivateBrowsingAPI.js:191\n()@resource://mozmill/modules/frame.js -> file:///Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPrivateBrowsing/testStartStopPBMode.js:145\n((function () {pb.enabled = false;pb.showPrompt = true;pb.start(true);pb.stop(true);}))@resource://mozmill/modules/frame.js:514\n([object Object])@resource://mozmill/modules/frame.js:582\n([object Object])@resource://mozmill/modules/frame.js:625\n(\"/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox\")@resource://mozmill/modules/frame.js:455\n(\"/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox\")@resource://mozmill/modules/frame.js:631\n((function (dir, invokedFromIDE) {var runner = new Runner(new Collector, invokedFromIDE);runner.runTestDirectory(dir);runner.end();return true;}),[object Array])@resource://jsbridge/modules/server.js:164\n(\"2385bfb6-ccec-11df-be03-34159e186224\",(function (dir, invokedFromIDE) {var runner = new Runner(new Collector, invokedFromIDE);runner.runTestDirectory(dir);runner.end();return true;}),[object Array])@resource://jsbridge/modules/server.js:168\n@resource://jsbridge/modules/server.js:249\n", "fileName": "resource://mozmill/modules/events.js"}}
TEST-UNEXPECTED-FAIL | /Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPrivateBrowsing/testStartStopPBMode.js | testKeyboardShortcut
DEBUG | mozmill.endTest | {"passes": [], "fails": [{"exception": {"message": "Synthesizing key event failed. \n[Exception... \"Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIDOMWindowUtils.sendKeyEvent]\"  nsresult: \"0x80004005 (NS_ERROR_FAILURE)\"  location: \"JS frame :: resource://mozmill/stdlib/EventUtils.js :: synthesizeKey :: line 342\"  data: no]", "lineNumber": 136, "stack": "([object ChromeWindow],\"keypress\",\"P\",[object Object])@resource://mozmill/modules/events.js:136\n(null,\"P\",[object Object])@resource://mozmill/modules/controller.js:170\nprivateBrowsing_start(true)@resource://mozmill/modules/frame.js -> file:///Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/shared-modules/testPrivateBrowsingAPI.js:191\n()@resource://mozmill/modules/frame.js -> file:///Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPrivateBrowsing/testStartStopPBMode.js:145\n((function () {pb.enabled = false;pb.showPrompt = true;pb.start(true);pb.stop(true);}))@resource://mozmill/modules/frame.js:514\n([object Object])@resource://mozmill/modules/frame.js:582\n([object Object])@resource://mozmill/modules/frame.js:625\n(\"/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox\")@resource://mozmill/modules/frame.js:455\n(\"/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox\")@resource://mozmill/modules/frame.js:631\n((function (dir, invokedFromIDE) {var runner = new Runner(new Collector, invokedFromIDE);runner.runTestDirectory(dir);runner.end();return true;}),[object Array])@resource://jsbridge/modules/server.js:164\n(\"2385bfb6-ccec-11df-be03-34159e186224\",(function (dir, invokedFromIDE) {var runner = new Runner(new Collector, invokedFromIDE);runner.runTestDirectory(dir);runner.end();return true;}),[object Array])@resource://jsbridge/modules/server.js:168\n@resource://jsbridge/modules/server.js:249\n", "fileName": "resource://mozmill/modules/events.js"}}], "name": "testKeyboardShortcut", "filename": "/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPrivateBrowsing/testStartStopPBMode.js", "failed": 1, "passed": 0}
DEBUG | mozmill.setState | "currentState"
TEST-START | /Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPrivateBrowsing/testStartStopPBMode.js | teardownModule
DEBUG | mozmill.setTest | {"name": "teardownModule", "filename": "/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPrivateBrowsing/testStartStopPBMode.js"}
TEST-PASS | /Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPrivateBrowsing/testStartStopPBMode.js | teardownModule
DEBUG | mozmill.endTest | {"passes": [], "fails": [], "name": "teardownModule", "filename": "/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPrivateBrowsing/testStartStopPBMode.js", "failed": 0, "passed": 0}
DEBUG | mozmill.setState | "currentState"
DEBUG | mozmill.setModule | "currentModule"
DEBUG | mozmill.setState | "currentState"
TEST-START | /Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPrivateBrowsing/testTabRestoration.js | setupModule
DEBUG | mozmill.setTest | {"name": "setupModule", "filename": "/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPrivateBrowsing/testTabRestoration.js"}
INFO | Step Pass: {"function": "Controller.open()"}
2010-09-30 16:41:04.853 firefox-bin[20488:903] invalid pixel format
2010-09-30 16:41:04.853 firefox-bin[20488:903] invalid context
2010-09-30 16:41:04.854 firefox-bin[20488:903] invalid pixel format
2010-09-30 16:41:04.854 firefox-bin[20488:903] invalid context
INFO | Step Pass: {"function": "controller.waitFor()"}
INFO | Step Pass: {"function": "controller.waitForPageLoad()"}
TEST-PASS | /Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPrivateBrowsing/testTabRestoration.js | setupModule
DEBUG | mozmill.endTest | {"passes": [{"function": "Controller.open()"}, {"function": "controller.waitFor()"}, {"function": "controller.waitForPageLoad()"}], "fails": [], "name": "setupModule", "filename": "/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPrivateBrowsing/testTabRestoration.js", "failed": 0, "passed": 3}
DEBUG | mozmill.setState | "currentState"
TEST-START | /Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPrivateBrowsing/testTabRestoration.js | testTabRestoration
DEBUG | mozmill.setTest | {"name": "testTabRestoration", "filename": "/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPrivateBrowsing/testTabRestoration.js"}
INFO | Step Pass: {"function": "Controller.open()"}
INFO | Step Pass: {"function": "Controller.open()"}
INFO | Step Pass: {"function": "controller.waitFor()"}
INFO | Step Pass: {"function": "Controller.waitForElement()"}
INFO | Step Pass: {"function": "controller.waitFor()"}
INFO | Step Pass: {"function": "Controller.waitForElement()"}
INFO | Step Pass: {"function": "controller.waitForEval()"}
INFO | Step Pass: {"function": "controller.waitForEval()"}
INFO | Step Pass: {"function": "controller.waitForEval()"}
INFO | Step Pass: {"function": "controller.waitForEval()"}
INFO | Step Pass: {"function": "controller.assertJS('subject.tabCountActual == subject.tabCountExpected')"}
INFO | Step Pass: {"function": "controller.waitFor()"}
INFO | Step Pass: {"function": "Controller.waitForElement()"}
INFO | Step Pass: {"function": "controller.waitFor()"}
INFO | Step Pass: {"function": "Controller.waitForElement()"}
TEST-PASS | /Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPrivateBrowsing/testTabRestoration.js | testTabRestoration
DEBUG | mozmill.endTest | {"passes": [{"function": "Controller.open()"}, {"function": "Controller.open()"}, {"function": "controller.waitFor()"}, {"function": "Controller.waitForElement()"}, {"function": "controller.waitFor()"}, {"function": "Controller.waitForElement()"}, {"function": "controller.waitForEval()"}, {"function": "controller.waitForEval()"}, {"function": "controller.waitForEval()"}, {"function": "controller.waitForEval()"}, {"function": "controller.assertJS('subject.tabCountActual == subject.tabCountExpected')"}, {"function": "controller.waitFor()"}, {"function": "Controller.waitForElement()"}, {"function": "controller.waitFor()"}, {"function": "Controller.waitForElement()"}], "fails": [], "name": "testTabRestoration", "filename": "/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPrivateBrowsing/testTabRestoration.js", "failed": 0, "passed": 15}
DEBUG | mozmill.setState | "currentState"
TEST-START | /Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPrivateBrowsing/testTabRestoration.js | teardownModule
DEBUG | mozmill.setTest | {"name": "teardownModule", "filename": "/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPrivateBrowsing/testTabRestoration.js"}
TEST-PASS | /Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPrivateBrowsing/testTabRestoration.js | teardownModule
DEBUG | mozmill.endTest | {"passes": [], "fails": [], "name": "teardownModule", "filename": "/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testPrivateBrowsing/testTabRestoration.js", "failed": 0, "passed": 0}
DEBUG | mozmill.setState | "currentState"
DEBUG | mozmill.setModule | "currentModule"
DEBUG | mozmill.setState | "currentState"
TEST-START | /Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testSessionStore/testUndoTabFromContextMenu.js | setupModule
DEBUG | mozmill.setTest | {"name": "setupModule", "filename": "/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testSessionStore/testUndoTabFromContextMenu.js"}
INFO | Step Pass: {"function": "controller.waitForEval()"}
INFO | Step Pass: {"function": "controller.waitForEval()"}
INFO | Step Pass: {"function": "Controller.open()"}
INFO | Step Pass: {"function": "controller.waitFor()"}
INFO | Step Pass: {"function": "controller.waitForPageLoad()"}
TEST-PASS | /Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testSessionStore/testUndoTabFromContextMenu.js | setupModule
DEBUG | mozmill.endTest | {"passes": [{"function": "controller.waitForEval()"}, {"function": "controller.waitForEval()"}, {"function": "Controller.open()"}, {"function": "controller.waitFor()"}, {"function": "controller.waitForPageLoad()"}], "fails": [], "name": "setupModule", "filename": "/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testSessionStore/testUndoTabFromContextMenu.js", "failed": 0, "passed": 5}
DEBUG | mozmill.setState | "currentState"
TEST-START | /Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testSessionStore/testUndoTabFromContextMenu.js | testUndoTabFromContextMenu
DEBUG | mozmill.setTest | {"name": "testUndoTabFromContextMenu", "filename": "/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testSessionStore/testUndoTabFromContextMenu.js"}
INFO | Step Pass: {"function": "Controller.rightClick()"}
DEBUG | mozmill.log | {"function": "controller.assertProperty() - DEPRECATED", "message": "assertProperty(el, attrib, val) is deprecated. Use assertJSProperty(el, attrib, val) or assertDOMProperty(el, attrib, val) instead"}
INFO | Step Pass: {"function": "Controller.assertJSProperty(\"ID: context_undoCloseTab\") : true"}
INFO | Step Pass: {"function": "Controller.keypress()"}
INFO | Step Pass: {"function": "controller.assertJS('subject.closedTabCount == 0')"}
INFO | Step Pass: {"function": "Controller.open()"}
INFO | Step Pass: {"function": "controller.waitFor()"}
INFO | Step Pass: {"function": "controller.waitForPageLoad()"}
INFO | Step Pass: {"function": "controller.waitForEval()"}
INFO | Step Pass: {"function": "Controller.open()"}
INFO | Step Pass: {"function": "controller.waitFor()"}
INFO | Step Pass: {"function": "controller.waitForPageLoad()"}
INFO | Step Pass: {"function": "controller.waitForEval()"}
INFO | Step Pass: {"function": "Controller.open()"}
INFO | Step Pass: {"function": "controller.waitFor()"}
INFO | Step Pass: {"function": "controller.waitForPageLoad()"}
INFO | Step Pass: {"function": "controller.waitForEval()"}
INFO | Step Pass: {"function": "Controller.click()"}
INFO | Step Pass: {"function": "controller.waitForEval()"}
INFO | Step Pass: {"function": "Controller.assertText()"}
INFO | Step Pass: {"function": "controller.assertJS('subject.closedTabCount == 1')"}
INFO | Step Pass: {"function": "Controller.rightClick()"}
DEBUG | mozmill.log | {"function": "controller.assertProperty() - DEPRECATED", "message": "assertProperty(el, attrib, val) is deprecated. Use assertJSProperty(el, attrib, val) or assertDOMProperty(el, attrib, val) instead"}
INFO | Step Pass: {"function": "Controller.assertJSProperty(\"ID: context_undoCloseTab\") : false"}
INFO | Step Pass: {"function": "controller.waitFor()"}
INFO | Step Pass: {"function": "controller.waitForPageLoad()"}
INFO | Step Pass: {"function": "Controller.keypress()"}
INFO | Step Pass: {"function": "Controller.assertText()"}
INFO | Step Pass: {"function": "controller.assertJS('subject.closedTabCount == 0')"}
INFO | Step Pass: {"function": "Controller.rightClick()"}
DEBUG | mozmill.log | {"function": "controller.assertProperty() - DEPRECATED", "message": "assertProperty(el, attrib, val) is deprecated. Use assertJSProperty(el, attrib, val) or assertDOMProperty(el, attrib, val) instead"}
INFO | Step Pass: {"function": "Controller.assertJSProperty(\"ID: context_undoCloseTab\") : true"}
INFO | Step Pass: {"function": "Controller.keypress()"}
TEST-PASS | /Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testSessionStore/testUndoTabFromContextMenu.js | testUndoTabFromContextMenu
DEBUG | mozmill.endTest | {"passes": [{"function": "Controller.rightClick()"}, {"function": "Controller.assertJSProperty(\"ID: context_undoCloseTab\") : true"}, {"function": "Controller.keypress()"}, {"function": "controller.assertJS('subject.closedTabCount == 0')"}, {"function": "Controller.open()"}, {"function": "controller.waitFor()"}, {"function": "controller.waitForPageLoad()"}, {"function": "controller.waitForEval()"}, {"function": "Controller.open()"}, {"function": "controller.waitFor()"}, {"function": "controller.waitForPageLoad()"}, {"function": "controller.waitForEval()"}, {"function": "Controller.open()"}, {"function": "controller.waitFor()"}, {"function": "controller.waitForPageLoad()"}, {"function": "controller.waitForEval()"}, {"function": "Controller.click()"}, {"function": "controller.waitForEval()"}, {"function": "Controller.assertText()"}, {"function": "controller.assertJS('subject.closedTabCount == 1')"}, {"function": "Controller.rightClick()"}, {"function": "Controller.assertJSProperty(\"ID: context_undoCloseTab\") : false"}, {"function": "controller.waitFor()"}, {"function": "controller.waitForPageLoad()"}, {"function": "Controller.keypress()"}, {"function": "Controller.assertText()"}, {"function": "controller.assertJS('subject.closedTabCount == 0')"}, {"function": "Controller.rightClick()"}, {"function": "Controller.assertJSProperty(\"ID: context_undoCloseTab\") : true"}, {"function": "Controller.keypress()"}], "fails": [], "name": "testUndoTabFromContextMenu", "filename": "/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testSessionStore/testUndoTabFromContextMenu.js", "failed": 0, "passed": 30}
DEBUG | mozmill.setState | "currentState"
TEST-START | /Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testSessionStore/testUndoTabFromContextMenu.js | teardownModule
DEBUG | mozmill.setTest | {"name": "teardownModule", "filename": "/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testSessionStore/testUndoTabFromContextMenu.js"}
INFO | Step Pass: {"function": "Controller.keypress()"}
INFO | Step Pass: {"function": "controller.waitForEval()"}
INFO | Step Pass: {"function": "controller.waitForEval()"}
INFO | Step Pass: {"function": "controller.waitForEval()"}
INFO | Step Pass: {"function": "Controller.open()"}
INFO | Step Pass: {"function": "controller.waitFor()"}
INFO | Step Pass: {"function": "controller.waitForPageLoad()"}
TEST-PASS | /Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testSessionStore/testUndoTabFromContextMenu.js | teardownModule
DEBUG | mozmill.endTest | {"passes": [{"function": "Controller.keypress()"}, {"function": "controller.waitForEval()"}, {"function": "controller.waitForEval()"}, {"function": "controller.waitForEval()"}, {"function": "Controller.open()"}, {"function": "controller.waitFor()"}, {"function": "controller.waitForPageLoad()"}], "fails": [], "name": "teardownModule", "filename": "/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testSessionStore/testUndoTabFromContextMenu.js", "failed": 0, "passed": 7}
DEBUG | mozmill.setState | "currentState"
DEBUG | mozmill.setModule | "currentModule"
DEBUG | mozmill.setState | "currentState"
TEST-START | /Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testTechnicalTools/testAccessPageInfoDialog.js | setupModule
DEBUG | mozmill.setTest | {"name": "setupModule", "filename": "/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testTechnicalTools/testAccessPageInfoDialog.js"}
TEST-PASS | /Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testTechnicalTools/testAccessPageInfoDialog.js | setupModule
DEBUG | mozmill.endTest | {"passes": [], "fails": [], "name": "setupModule", "filename": "/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testTechnicalTools/testAccessPageInfoDialog.js", "failed": 0, "passed": 0}
DEBUG | mozmill.setState | "currentState"
TEST-START | /Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testTechnicalTools/testAccessPageInfoDialog.js | testAccessPageInfo
DEBUG | mozmill.setTest | {"name": "testAccessPageInfo", "filename": "/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testTechnicalTools/testAccessPageInfoDialog.js"}
INFO | Step Pass: {"function": "Controller.open()"}
INFO | Step Pass: {"function": "controller.waitFor()"}
INFO | Step Pass: {"function": "controller.waitForPageLoad()"}
DEBUG | mozmill.log | {"function": "rightclick - Deprecation Warning", "message": "Controller.rightclick should be renamed to Controller.rightClick"}
INFO | Step Pass: {"function": "Controller.rightClick()"}
2010-09-30 16:41:15.700 firefox-bin[20488:903] invalid pixel format
2010-09-30 16:41:15.701 firefox-bin[20488:903] invalid context
2010-09-30 16:41:15.701 firefox-bin[20488:903] invalid pixel format
2010-09-30 16:41:15.701 firefox-bin[20488:903] invalid context
INFO | Step Pass: {"function": "Controller.click()"}
INFO | Step Pass: {"function": "controller.waitFor()"}
INFO | Step Pass: {"function": "Controller.waitForElement()"}
INFO | Step Pass: {"function": "Controller.click()"}
INFO | Step Pass: {"function": "controller.waitFor()"}
INFO | Step Pass: {"function": "Controller.waitForElement()"}
INFO | Step Pass: {"function": "Controller.click()"}
INFO | Step Pass: {"function": "controller.waitFor()"}
INFO | Step Pass: {"function": "Controller.waitForElement()"}
INFO | Step Pass: {"function": "Controller.click()"}
INFO | Step Pass: {"function": "controller.waitFor()"}
INFO | Step Pass: {"function": "Controller.waitForElement()"}
INFO | Step Pass: {"function": "Controller.click()"}
INFO | Step Pass: {"function": "controller.waitFor()"}
INFO | Step Pass: {"function": "Controller.waitForElement()"}
INFO | Step Pass: {"function": "Controller.keypress()"}
TEST-PASS | /Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testTechnicalTools/testAccessPageInfoDialog.js | testAccessPageInfo
DEBUG | mozmill.endTest | {"passes": [{"function": "Controller.open()"}, {"function": "controller.waitFor()"}, {"function": "controller.waitForPageLoad()"}, {"function": "Controller.rightClick()"}, {"function": "Controller.click()"}, {"function": "controller.waitFor()"}, {"function": "Controller.waitForElement()"}, {"function": "Controller.click()"}, {"function": "controller.waitFor()"}, {"function": "Controller.waitForElement()"}, {"function": "Controller.click()"}, {"function": "controller.waitFor()"}, {"function": "Controller.waitForElement()"}, {"function": "Controller.click()"}, {"function": "controller.waitFor()"}, {"function": "Controller.waitForElement()"}, {"function": "Controller.click()"}, {"function": "controller.waitFor()"}, {"function": "Controller.waitForElement()"}, {"function": "Controller.keypress()"}], "fails": [], "name": "testAccessPageInfo", "filename": "/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testTechnicalTools/testAccessPageInfoDialog.js", "failed": 0, "passed": 20}
DEBUG | mozmill.setState | "currentState"
TEST-START | /Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testTechnicalTools/testAccessPageInfoDialog.js | teardownModule
DEBUG | mozmill.setTest | {"name": "teardownModule", "filename": "/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testTechnicalTools/testAccessPageInfoDialog.js"}
INFO | Step Pass: {"function": "Controller.keypress()"}
TEST-PASS | /Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testTechnicalTools/testAccessPageInfoDialog.js | teardownModule
DEBUG | mozmill.endTest | {"passes": [{"function": "Controller.keypress()"}], "fails": [], "name": "teardownModule", "filename": "/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testTechnicalTools/testAccessPageInfoDialog.js", "failed": 0, "passed": 1}
DEBUG | mozmill.setModule | "currentModule"
DEBUG | mozmill.setState | "currentState"
TEST-START | /Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testToolbar/testBackForwardButtons.js | setupModule
DEBUG | mozmill.setTest | {"name": "setupModule", "filename": "/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testToolbar/testBackForwardButtons.js"}
TEST-PASS | /Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testToolbar/testBackForwardButtons.js | setupModule
DEBUG | mozmill.endTest | {"passes": [], "fails": [], "name": "setupModule", "filename": "/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testToolbar/testBackForwardButtons.js", "failed": 0, "passed": 0}
DEBUG | mozmill.setState | "currentState"
TEST-START | /Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testToolbar/testBackForwardButtons.js | testBackAndForward
DEBUG | mozmill.setTest | {"name": "testBackAndForward", "filename": "/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testToolbar/testBackForwardButtons.js"}
INFO | Step Pass: {"function": "Controller.open()"}
INFO | Step Pass: {"function": "controller.waitFor()"}
INFO | Step Pass: {"function": "controller.waitForPageLoad()"}
INFO | Step Pass: {"function": "controller.waitFor()"}
INFO | Step Pass: {"function": "Controller.waitForElement()"}
INFO | Step Pass: {"function": "Controller.open()"}
INFO | Step Pass: {"function": "controller.waitFor()"}
INFO | Step Pass: {"function": "controller.waitForPageLoad()"}
INFO | Step Pass: {"function": "controller.waitFor()"}
INFO | Step Pass: {"function": "Controller.waitForElement()"}
INFO | Step Pass: {"function": "Controller.open()"}
INFO | Step Pass: {"function": "controller.waitFor()"}
INFO | Step Pass: {"function": "controller.waitForPageLoad()"}
INFO | Step Pass: {"function": "controller.waitFor()"}
INFO | Step Pass: {"function": "Controller.waitForElement()"}
INFO | Step Pass: {"function": "Controller.goBack()"}
INFO | Step Pass: {"function": "controller.waitFor()"}
INFO | Step Pass: {"function": "controller.waitForPageLoad()"}
INFO | Step Pass: {"function": "controller.waitFor()"}
INFO | Step Pass: {"function": "Controller.waitForElement()"}
INFO | Step Pass: {"function": "Controller.goBack()"}
INFO | Step Pass: {"function": "controller.waitFor()"}
INFO | Step Pass: {"function": "controller.waitForPageLoad()"}
INFO | Step Pass: {"function": "controller.waitFor()"}
INFO | Step Pass: {"function": "Controller.waitForElement()"}
INFO | Step Pass: {"function": "Controller.goForward()"}
INFO | Step Pass: {"function": "controller.waitFor()"}
INFO | Step Pass: {"function": "controller.waitForPageLoad()"}
INFO | Step Pass: {"function": "controller.waitFor()"}
INFO | Step Pass: {"function": "Controller.waitForElement()"}
INFO | Step Pass: {"function": "Controller.goForward()"}
INFO | Step Pass: {"function": "controller.waitFor()"}
INFO | Step Pass: {"function": "controller.waitForPageLoad()"}
INFO | Step Pass: {"function": "controller.waitFor()"}
INFO | Step Pass: {"function": "Controller.waitForElement()"}
TEST-PASS | /Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testToolbar/testBackForwardButtons.js | testBackAndForward
DEBUG | mozmill.endTest | {"passes": [{"function": "Controller.open()"}, {"function": "controller.waitFor()"}, {"function": "controller.waitForPageLoad()"}, {"function": "controller.waitFor()"}, {"function": "Controller.waitForElement()"}, {"function": "Controller.open()"}, {"function": "controller.waitFor()"}, {"function": "controller.waitForPageLoad()"}, {"function": "controller.waitFor()"}, {"function": "Controller.waitForElement()"}, {"function": "Controller.open()"}, {"function": "controller.waitFor()"}, {"function": "controller.waitForPageLoad()"}, {"function": "controller.waitFor()"}, {"function": "Controller.waitForElement()"}, {"function": "Controller.goBack()"}, {"function": "controller.waitFor()"}, {"function": "controller.waitForPageLoad()"}, {"function": "controller.waitFor()"}, {"function": "Controller.waitForElement()"}, {"function": "Controller.goBack()"}, {"function": "controller.waitFor()"}, {"function": "controller.waitForPageLoad()"}, {"function": "controller.waitFor()"}, {"function": "Controller.waitForElement()"}, {"function": "Controller.goForward()"}, {"function": "controller.waitFor()"}, {"function": "controller.waitForPageLoad()"}, {"function": "controller.waitFor()"}, {"function": "Controller.waitForElement()"}, {"function": "Controller.goForward()"}, {"function": "controller.waitFor()"}, {"function": "controller.waitForPageLoad()"}, {"function": "controller.waitFor()"}, {"function": "Controller.waitForElement()"}], "fails": [], "name": "testBackAndForward", "filename": "/Users/cltbld/talos-slave/mozilla-central_snowleopard_test-mozmill-all/build/mozmill/tests/firefox/testToolbar/testBackForwardButtons.js", "failed": 0, "passed": 35}
DEBUG | mozmill.persist
DEBUG | mozmill.endRunner | true
INFO Passed: 26
INFO Failed: 2
INFO Skipped: 0
program finished with exit code 1
elapsedTime=57.068468
Attached patch mozmill + buildbot patch (obsolete) — Splinter Review
unbitrotted patch, as of today.  Add a selective unpacking step.  Carrying forward r+ from attachment 476361 [details] [diff] [review] (:jhford)
Attachment #476361 - Attachment is obsolete: true
Attachment #481376 - Flags: review+
Provided it works in testing, looks good.  Lets fix the typo

6666 	                 testtype='mozmill',
6667 	                 haltOnFailure=True,
6668 	                 name='unpack mochitest tests',

before landing though :)
(In reply to comment #202)
> Provided it works in testing, looks good.  Lets fix the typo
> 
> 6666                      testtype='mozmill',
> 6667                      haltOnFailure=True,
> 6668                      name='unpack mochitest tests',
> 
> before landing though :)

Should the others be fixed too? e.g. http://hg.mozilla.org/build/buildbotcustom/file/bd701a44b9c0/process/factory.py#l6617  Or should I file a separate bug for that?

Works locally, waiting to test on automation staging
Depends on: 605978
Attached patch buildbot configs example patch (obsolete) — Splinter Review
Attached patch mozmill buildbotcustom patch (obsolete) — Splinter Review
unbitrot patch; carrying :jhford r+ forward
Attachment #481376 - Attachment is obsolete: true
Attachment #485298 - Flags: review+
results from staging on Releng systems.  All mozilla-central builds:

10.5.8 debug all:24/1/0 restart:2/0/0 
10.5.8 opt   all:25/0/0 restart:2/0/0
10.6.2 debug all:24/1/0 restart:2/0/0 
10.6.2 opt   all:25/0/0 restart:2/0/0
f12-64 debug all:24/1/0 restart:2/0/0
f12-64 opt   all:24/1/0 restart:2/0/0
f12-32 opt   all:25/0/0 restart:2/0/0
f12-32 debug all:24/1/0 restart:2/0/0

The windows slave that I am using is in a bad state, so I don't have results for it.  I will get this fixed asap to get real results

Notes:

-Fedora 12 shows 'T-FAIL' instead of the pass/fail/known even if tests pass
-All failures are mozmill/tests/firefox/testPrivateBrowsing/testStartStopPBMode.js | testStopPrivateBrowsingMode

This looks like great progress, but not quite ready to land.









[1]TEST-UNEXPECTED-FAIL | /Users/cltbld/talos-slave/mozilla-central_leopard-o-debug_test-mozmill-all/build/mozmill/tests/firefox/testPrivateBrowsing/testStartStopPBMode.js | testStopPrivateBrowsingMode
[2]TEST-UNEXPECTED-FAIL | /Users/cltbld/talos-slave/mozilla-central_snowleopard-debug_test-mozmill-all/build/mozmill/tests/firefox/testPrivateBrowsing/testStartStopPBMode.js | testStopPrivateBrowsingMode
[3]TEST-UNEXPECTED-FAIL | /home/cltbld/talos-slave/mozilla-central_fedora64_test-mozmill-all/build/mozmill/tests/firefox/testPrivateBrowsing/testStartStopPBMode.js | testStopPrivateBrowsingMode
[4]TEST-UNEXPECTED-FAIL | /home/cltbld/talos-slave/mozilla-central_fedora64-debug_test-mozmill-all/build/mozmill/tests/firefox/testPrivateBrowsing/testStartStopPBMode.js | testStopPrivateBrowsingMode
win7 opt all:25/0/0 restart:2/0/0 

I noticed the same T-FAIL behaviour for the summary even though it passed
this patch may fix testStartStopPBMode.js: https://bugzilla.mozilla.org/show_bug.cgi?id=607000
(In reply to comment #208)
> this patch may fix testStartStopPBMode.js:
> https://bugzilla.mozilla.org/show_bug.cgi?id=607000

In detail it is the part of the following attachment:
https://bug607000.bugzilla.mozilla.org/attachment.cgi?id=486058

It's not fully tested yet, so I would say lets remove the test for now.
Attached patch mozmill buildbotcustom patch (obsolete) — Splinter Review
use a line-based parser instead of a regex based parser for summarizeMozmill.  seems to work in staging.  carrying over review , r=jhford
Attachment #485298 - Attachment is obsolete: true
Attachment #487587 - Flags: review+
Attached patch mozmill buildbotcustom patch (obsolete) — Splinter Review
modify to use summaryText and give back WARNINGS instead of FAILURE if superResult is FAILURE
Attachment #487587 - Attachment is obsolete: true
Attachment #487767 - Flags: review+
carrying over r+ , r=jhford
Attachment #487767 - Attachment is obsolete: true
Attachment #488386 - Flags: review+
uploaded the wrong patch; carrying over r+ r=jhford
Attachment #488386 - Attachment is obsolete: true
Attachment #488388 - Flags: review+
run mozmill unit tests.
Attachment #485187 - Attachment is obsolete: true
Attachment #488883 - Flags: review?(armenzg)
Attachment #488883 - Flags: review?(armenzg) → review+
Fixed.  Filed a follow up (bug610413) for making sure that this works for the try server.
No longer blocks: 610413
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
This patch addresses the T-FAIL issues being experienced.  These issues were fixed in a previous version of the patch but showed up again.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment on attachment 489065 [details] [diff] [review]
follow up to address indentation

We're going to need a mini postmortem on this to figure out how this regressed.
Attachment #489065 - Flags: review?(aki) → review+
Flags: needs-reconfig+
landed, waiting to see passing tests not have T-FAIL
this is working now.
Status: REOPENED → RESOLVED
Closed: 14 years ago14 years ago
Resolution: --- → FIXED
Depends on: 610579
Wow, this looks absolutely awesome! It's incredible to see that all those tedious efforts from the last months made it finally on buildbot. You can't imagine how lucky I am at the moment.

Great job for everyone involved!

But one question, why don't we run Mozmill on WINNT 5.2 mozilla-central debug? Is there any specific reason?
(In reply to comment #221)
> Wow, this looks absolutely awesome! It's incredible to see that all those
> tedious efforts from the last months made it finally on buildbot. You can't
> imagine how lucky I am at the moment.
> 
> Great job for everyone involved!
> 
> But one question, why don't we run Mozmill on WINNT 5.2 mozilla-central debug?
> Is there any specific reason?

Because we are only running Mozmill on the mac minis and we aren't currenly doing XP debug tests on those machines.  This will be fixed by bug 562459.
(In reply to comment #217)
> Comment on attachment 489065 [details] [diff] [review]
> follow up to address indentation
> 
> We're going to need a mini postmortem on this to figure out how this regressed.

This is probably mostly my fault as I put up the patch with the bad indentation (https://bug516984.bugzilla.mozilla.org/attachment.cgi?id=487767).  Why this was pushed to buildbotcustom instead of my subsequent patch with correct indentation (https://bug516984.bugzilla.mozilla.org/attachment.cgi?id=488388) I do not know.  Possibly a lack of communication, again possibly my fault.

How the incorrect indentation occured was due to a copy+paste.  I'm still a bit confused by the details.  I believe that I copied from a bitrotted patch and pasted to unbitrot.  Ironically, the way that I did this was to preserve correct indentation levels.  I don't know how a tab got in there.  Probably PEBCAK on my part.

I'm not sure how to fix this in the future, except being more careful.  Since python tries to be intelligent when tabs + spaces are mixed, I guess we hit an ambiguous case.  On all platforms the python did not give an error.  On my laptop and IIRC tools-staging, I did not get the T-FAIL result and things behaved as expected.  In my editor, the code displayed with the proper indentation levels.

I would not be opposed to setting up e.g. tabnanny or using python -t (or -tt) in whatever staging procedure is used.  Short of that, I'm not really sure how much of a safeguard we can put up.

There are numerous other things that could have made this better and otherwise tending towards presenting these errors.  I won't outline them here, but am available to talk about them in a more appropriate forum.
Depends on: 616383
Assignee: jhammel → nobody
Product: Testing → Testing Graveyard
You need to log in before you can comment on or make changes to this bug.