688667 - refactor automation.py into mozprofile and load via virtualenv

Status: RESOLVED FIXED

(Testing :: General, defect)

Description

[Jeff Hammel]

The vast majority of automation.py duplicates functionality of mozprocess and mozprofile. These should be installed in a virtualenv and mozautomation should make use of their functionality once it is no longer interpolated.

Comment 1

[Jeff Hammel]

Most of the functionality in automation.py is reproduced, often in better form, in mozprocess (https://github.com/mozautomation/mozmill/tree/master/mozprocess) and mozprofile (https://github.com/mozautomation/mozmill/tree/master/mozprofile). These should be mirrored to e.g. build/python in m-c (see bug 688686) and installed into the $OBJDIR vitualenv (see bug 661908).

Comment 3

[Andrew Halberstadt [:ahal]]

Is there any reason we want to keep automation.py around at all? What if each of the test harnesses (mochitest, reftest, etc) just consumed mozbase directly. If we find a need for a common integration point, we can modify moztest to support this. Perhaps one day we'd be able to remove  all the *automation.py files from the tree completely.

Comment 4

[Jeff Hammel]

My general opinion is "whatever is less work". My general opinion for refactors, including this one, is that it is generally both safer and easier to swap out a piece at a time vs chuck and run.

Comment 5

[Andrew Halberstadt [:ahal]]

Ok, sounds like we are on the same page. That's what I was planning to do, just that once all pieces have been swapped out it'll be possible to stop copying the automation files to the tests.zip

Comment 6

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

There is still a lot of functionality in automation.py which doesn't exist in mozbase/*.  Can we ensure that we have pulled all the functionality out?

Comment 7

[Andrew Halberstadt [:ahal]]

Yep. I'm going to start with B2G since it's at the bottom of the inheritance tree so shouldn't cause problems for desktop or fennec. For each "thing" that automation.py does I'll file a bug to make the B2G automation use the mozbase version of that "thing" (and if that "thing doesn't exist in mozbase, I'll file a bug to add that "thing" first). Finally when there are no "things" left and B2G is completely switched over, mozbase should hopefully be at around 70-90% parity with automation.py and then switching fennec and desktop will go a lot faster.

Comment 8

[Jeff Hammel]

This may be obseleted by evolving plans; current discussion focuses around refactoring e.g. mochitest on mozbase by removing its usage of automation.py.in and replacing said functionality with mozbase modules.

Note that in either case some of one, some of the other approach is not ruled out if it is a clear stride forward for expediency.

In both cases the end goal is the same:
- porting of all desired functionality of automation.py.in to mozbase or other appropriate long-term home
- removal of automation.py.in in its entirety

Comment 9

[Andrew Halberstadt [:ahal]]

Ah, I forgot about this bug. This is basically what I did in bug 835930. Almost all of the profile management bits are now in mozprofile 0.6 and being used in production by the b2g mochitests. I say almost because mozprofile still doesn't manage userChrome.css and userContent.css (though I'm not sure that it needs to yet).

The mozprocess side is part of my Q2 goals.

Comment 10

[cmtalbert]

Where are we on this, ahal?

Comment 11

[Andrew Halberstadt [:ahal]]

I'd say 97% of the profile management stuff in automation.py can be handled by mozprofile right now.

I'm working on the mozprocess stuff for B2G mochitests in bug 865349. I have a patch that works locally but is blocked on some build module configuration issues (i.e getting the values that are interpolated in automation.py.in). Once this is finished there will still be more work required for desktop/fennec tests, but I'm hoping there will be a good foundation to start from.

Comment 12

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

Attachment: use mozprofile for mochitests (1.0)

this patch looks good initially on try server.  I have a modification in the permissions.py inside of mozprofile to adjust a port in order to get websocket tests to work.  I haven't tested the webapps stuff that I modified, I assume that works.

What this patch needs is testing on all platforms as well as cleaning up of automation.py install extensions stuff (i.e. remove code!)

Comment 13

[Jeff Hammel]

Joel, I assume the permissions.py fix we want anyway?  Ticketed separately for my sanity in bug 875527

Comment 14

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

yes we NEED that permissions fix, and then it won't need to be hacked on my patch for m-c!

Comment 15

[Jeff Hammel]

(In reply to Joel Maher (:jmaher) from comment #12)
> Created attachment 753366 [details] [diff] [review]
> use mozprofile for mochitests (1.0)
> 
> this patch looks good initially on try server.  I have a modification in the
> permissions.py inside of mozprofile to adjust a port in order to get
> websocket tests to work.  I haven't tested the webapps stuff that I
> modified, I assume that works.
> 
> What this patch needs is testing on all platforms as well as cleaning up of
> automation.py install extensions stuff (i.e. remove code!)

Unbitrotting this now

Comment 16

[Jeff Hammel]

Attachment: unbitrot

straight unbitrot of attachment 753366 [details] [diff] [review]; untested

Comment 17

[Jeff Hammel]

A sys.path mystery:

with code

"""
# --------------------------------------------------------------
# TODO: this is a hack for mozbase without virtualenv, remove with bug 849900
#
here = os.path.dirname(os.path.realpath(__file__))
mozbase = os.path.realpath(os.path.join(os.path.dirname(here), 'mozbase'))

if os.path.isdir(mozbase):
    for package in os.listdir(mozbase):
        sys.path.append(os.path.join(mozbase, package))

import mozcrash
from mozprofile import Profile, Preferences
from mozprofile.permissions import ServerLocations

# ---------------------------------------------------------------
"""

This doesn't work:

(Pdb) mozbase
'/home/jhammel/mozilla/src/obj-browser/mozbase'
(Pdb) here
'/home/jhammel/mozilla/src/mozilla-central/../obj-browser/build'
(Pdb) os.path.dirname(here)
'/home/jhammel/mozilla/src/mozilla-central/../obj-browser'
(Pdb) os.path.join(os.path.dirname(here), 'mozbase
*** SyntaxError: EOL while scanning string literal (<stdin>, line 1)
(Pdb) os.path.join(os.path.dirname(here), 'mozbase')
'/home/jhammel/mozilla/src/mozilla-central/../obj-browser/mozbase'
(Pdb) os.path.realpath(os.path.join(os.path.dirname(here), 'mozbase'))
'/home/jhammel/mozilla/src/obj-browser/mozbase'
(Pdb) os.path.isdir(mozbase)
False

However, we do get things on the path:

 u'/home/jhammel/mozilla/src/mozilla-central/testing/marionette/client',
 u'/home/jhammel/mozilla/src/mozilla-central/testing/marionette/client/marionette',
 u'/home/jhammel/mozilla/src/mozilla-central/testing/mozbase/mozcrash',
 u'/home/jhammel/mozilla/src/mozilla-central/testing/mozbase/mozdevice',
 u'/home/jhammel/mozilla/src/mozilla-central/testing/mozbase/mozfile',
 u'/home/jhammel/mozilla/src/mozilla-central/testing/mozbase/mozhttpd',
 u'/home/jhammel/mozilla/src/mozilla-central/testing/mozbase/mozlog',
 u'/home/jhammel/mozilla/src/mozilla-central/testing/mozbase/moznetwork',
 u'/home/jhammel/mozilla/src/mozilla-central/testing/mozbase/mozprocess',
 u'/home/jhammel/mozilla/src/mozilla-central/testing/mozbase/mozprofile',
 u'/home/jhammel/mozilla/src/mozilla-central/testing/mozbase/mozrunner',
 u'/home/jhammel/mozilla/src/mozilla-central/testing/mozbase/mozinfo',
 u'/home/jhammel/mozilla/src/mozilla-central/build',
 '/home/jhammel/mozilla/src/mozilla-central',
 '/home/jhammel/python',
 '/home/jhammel/mozilla/src/mozilla-central',
 '/home/jhammel/virtualenv',
 '/usr/lib/python2.7',
 '/usr/lib/python2.7/plat-i386-linux-gnu',
 '/usr/lib/python2.7/lib-tk',
...

Obviously from something else. Need a few fixes here.

Comment 18

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

is there a specific scenario where this is failing?  Is this failing on tryserver?

Comment 19

[Jeff Hammel]

(In reply to Joel Maher (:jmaher) from comment #18)
> is there a specific scenario where this is failing?  Is this failing on
> tryserver?

This is locally...that said, i'm not sure how this is supposed to work at all currently

Comment 20

[Jeff Hammel]

Will post output shortly

Comment 21

[Jeff Hammel]

Beh, nm, I found it....yet another place we maintain the same damn list as always;

http://mxr.mozilla.org/mozilla-central/source/build/mach_bootstrap.py#42

The order look familiar? :(

Comment 22

[Jeff Hammel]

This problem fixed with http://k0s.org/mozilla/hg/mozilla-central-patches/rev/e383fb24a00d

We might want to do this thing moar betterz

Comment 23

[Jeff Hammel]

Now I get

 2:38.81 TEST-UNEXPECTED-FAIL | unknown test url | uncaught exception - ReferenceError: SpecialPowers is not defined at http://mochi.test:8888/tests/SimpleTest/SimpleTest.js:736

Comment 24

[Jeff Hammel]

To contrast, the non-patched case:

<snip/>
 0:21.36 TEST-PASS | unknown test url | wrong responseText in test for ({pass:1, method:"POST", body:"hi there", headers:{'Content-Type':"text/plain"}, hops:[{server:"http://example.org"}, {server:"http://example.com", allowOrigin:"http://example.org"}]})
 0:21.37 TEST-PASS | unknown test url | should have failed in test for ({pass:0, method:"POST", body:"hi there", headers:{'Content-Type':"text/plain", 'my-header':"myValue"}, hops:[{server:"http://example.com", allowOrigin:"http://example.org", allowHeaders:"my-header"}, {server:"http://example.org", allowOrigin:"http://example.org", allowHeaders:"my-header"}]})
 0:21.37 TEST-PASS | unknown test url | wrong status in test for ({pass:0, method:"POST", body:"hi there", headers:{'Content-Type':"text/plain", 'my-header':"myValue"}, hops:[{server:"http://example.com", allowOrigin:"http://example.org", allowHeaders:"my-header"}, {server:"http://example.org", allowOrigin:"http://example.org", allowHeaders:"my-header"}]})
 0:21.37 TEST-PASS | unknown test url | wrong status text for ({pass:0, method:"POST", body:"hi there", headers:{'Content-Type':"text/plain", 'my-header':"myValue"}, hops:[{server:"http://example.com", allowOrigin:"http://example.org", allowHeaders:"my-header"}, {server:"http://example.org", allowOrigin:"http://example.org", allowHeaders:"my-header"}]})
 0:21.37 TEST-PASS | unknown test url | wrong responseXML in test for ({pass:0, method:"POST", body:"hi there", headers:{'Content-Type':"text/plain", 'my-header':"myValue"}, hops:[{server:"http://example.com", allowOrigin:"http://example.org", allowHeaders:"my-header"}, {server:"http://example.org", allowOrigin:"http://example.org", allowHeaders:"my-header"}]})
 0:21.37 TEST-PASS | unknown test url | wrong responseText in test for ({pass:0, method:"POST", body:"hi there", headers:{'Content-Type':"text/plain", 'my-header':"myValue"}, hops:[{server:"http://example.com", allowOrigin:"http://example.org", allowHeaders:"my-header"}, {server:"http://example.org", allowOrigin:"http://example.org", allowHeaders:"my-header"}]})
 0:21.37 TEST-PASS | unknown test url | wrong events in test for ({pass:0, method:"POST", body:"hi there", headers:{'Content-Type':"text/plain", 'my-header':"myValue"}, hops:[{server:"http://example.com", allowOrigin:"http://example.org", allowHeaders:"my-header"}, {server:"http://example.org", allowOrigin:"http://example.org", allowHeaders:"my-header"}]})
 0:21.37 TEST-PASS | unknown test url | wrong progressevents in test for ({pass:0, method:"POST", body:"hi there", headers:{'Content-Type':"text/plain", 'my-header':"myValue"}, hops:[{server:"http://example.com", allowOrigin:"http://example.org", allowHeaders:"my-header"}, {server:"http://example.org", allowOrigin:"http://example.org", allowHeaders:"my-header"}]})

Comment 25

[Jeff Hammel]

Not the problem but...

http://mxr.mozilla.org/mozilla-central/source/build/automation.py.in#173
  There is __all__ as a class-level variable, which AFAIK does nothing:

  """
  173   @property
  174   def __all__(self):
  175     return [
  176            "UNIXISH",
  177            "IS_WIN32",
  178            "IS_MAC",
  179            "log",
  ...
  """

  This should be deleted.  It was moved from the global scope in
  http://hg.mozilla.org/mozilla-central/diff/e636b2d6ceb1/build/automation.py.in#l1.33
  and ABICT is no longer doing anything.

Comment 26

[Jeff Hammel]

(In reply to Jeff Hammel [:jhammel] from comment #25)
> Not the problem but...
> 
> http://mxr.mozilla.org/mozilla-central/source/build/automation.py.in#173
>   There is __all__ as a class-level variable, which AFAIK does nothing:
> 
>   """
>   173   @property
>   174   def __all__(self):
>   175     return [
>   176            "UNIXISH",
>   177            "IS_WIN32",
>   178            "IS_MAC",
>   179            "log",
>   ...
>   """
> 
>   This should be deleted.  It was moved from the global scope in
>  
> http://hg.mozilla.org/mozilla-central/diff/e636b2d6ceb1/build/automation.py.
> in#l1.33
>   and ABICT is no longer doing anything.

My mistake, this is used, albeit strangely:

http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/runtests.py#43

If we're getting rid of automation.py.in , its probably fine to leave this (though I might add a comment), but if we do keep it around....should probably make this a bit more obvious.

Comment 27

[Jeff Hammel]

+    #TODO: why do we have initialProfile
+#    if initialProfile:
+#      shutil.copytree(initialProfile, profileDir)
+#    else:
+#      os.mkdir(profileDir)

Answer:  for no reason; it is unused ABICT: http://mxr.mozilla.org/mozilla-central/search?string=initializeProfile

removing

Comment 28

[Jeff Hammel]

+    #if we don't do this, the profile object is destroyed when we exit this method
+    self.profile = profile

I'm wondering if it'd be better to use `restore=False` and have the caller be responsible for its destruction. :shrug:

Comment 29

[Jeff Hammel]

So one reason this fails is that we're not loading the extensions -- big mistake.  We pretend we are, but don't raise an error due to bug 889419 . Will upload a modern patch and try again.

Comment 30

[Jeff Hammel]

Attachment: modern form of patch, so far

Comment 31

[Jeff Hammel]

(In reply to Jeff Hammel [:jhammel] from comment #30)
> Created attachment 770250 [details] [diff] [review]
> modern form of patch, so far

Wow, this might actually work!  at least the output is the same as for the non-patch case

Comment 32

[Jeff Hammel]

Attachment: final form, take 1

Comment 33

[Jeff Hammel]

pushed to try:

│mozillatry -u all https://bug688667.bugzilla.mozilla.org/attachment.cgi?id=770286 --bug 688667
try: -b do -p all -u all -t none --post-to-bugzilla Bug 688667
resolving manifests
0 files updated, 0 files merged, 0 files removed, 0 files unresolved
abort: unknown revision 'qbase'!
no patches applied
pulling from http://hg.mozilla.org/mozilla-central
searching for changes
no changes found
resolving manifests
0 files updated, 0 files merged, 0 files removed, 0 files unresolved
adding attachment.cgi?id=770286 to series file
applying attachment.cgi?id=770286
patching file build/automation.py.in
patching file build/mach_bootstrap.py
patching file testing/mochitest/runtests.py
build/automation.py.in
build/mach_bootstrap.py
testing/mochitest/runtests.py
now at: attachment.cgi?id=770286
0 A attachment.cgi?id=770286
build/automation.py.in
build/mach_bootstrap.py
testing/mochitest/runtests.py
pushing to ssh://hg.mozilla.org/try/
running ssh hg.mozilla.org 'hg -R try/ serve --stdio'
searching for changes
1 changesets found
remote: adding changesets
remote: adding manifests
remote: adding file changes
remote: added 1 changesets with 3 changes to 3 files (+1 heads)
remote: Looks like you used try syntax, going ahead with the push.
remote: If you don't get what you expected, check http://trychooser.pub.build.mozilla.org/ for help with building your trychooser request.
remote: Thanks for helping save resources, you're the best!
remote: Trying to insert into pushlog.
remote: Please do not interrupt...
remote: Inserted into the pushlog db successfully.
remote: You can view the progress of your build at the following URL:
remote:   https://tbpl.mozilla.org/?tree=Try&rev=03b5525b9620
resolving manifests
0 files updated, 0 files merged, 0 files removed, 0 files unresolved
popping attachment.cgi?id=770286
patch queue now empty

Comment 34

[Jeff Hammel]

we at least need to restore or alternately implement this function:

11:56:26     INFO - Running command: ['/builds/slave/talos-slave/test/build/venv/bin/python', '-u', '/builds/slave/talos-slave/test/build/tests/reftest/runreftest.py', '--appname=/builds/slave/talos-slave/test/build/application/FirefoxNightlyDebug.app/Contents/MacOS/firefox', '--utility-path=tests/bin', '--extra-profile-file=tests/bin/plugins', '--symbols-path=/builds/slave/talos-slave/test/build/symbols', 'tests/reftest/tests/testing/crashtest/crashtests.list'] in /builds/slave/talos-slave/test/build
11:56:26     INFO - Copy/paste: /builds/slave/talos-slave/test/build/venv/bin/python -u /builds/slave/talos-slave/test/build/tests/reftest/runreftest.py --appname=/builds/slave/talos-slave/test/build/application/FirefoxNightlyDebug.app/Contents/MacOS/firefox --utility-path=tests/bin --extra-profile-file=tests/bin/plugins --symbols-path=/builds/slave/talos-slave/test/build/symbols tests/reftest/tests/testing/crashtest/crashtests.list
11:56:26     INFO -  Traceback (most recent call last):
11:56:26     INFO -    File "/builds/slave/talos-slave/test/build/tests/reftest/runreftest.py", line 318, in <module>
11:56:26     INFO -      main()
11:56:26     INFO -    File "/builds/slave/talos-slave/test/build/tests/reftest/runreftest.py", line 315, in main
11:56:26     INFO -      sys.exit(reftest.runTests(args[0], options))
11:56:26     INFO -    File "/builds/slave/talos-slave/test/build/tests/reftest/runreftest.py", line 126, in runTests
11:56:26     INFO -      self.createReftestProfile(options, profileDir, reftestlist)
11:56:26     INFO -    File "/builds/slave/talos-slave/test/build/tests/reftest/runreftest.py", line 53, in createReftestProfile
11:56:26     INFO -      self.automation.setupPermissionsDatabase(profileDir,
11:56:26     INFO -  AttributeError: 'Automation' object has no attribute 'setupPermissionsDatabase'
11:56:26    ERROR - Return code: 1

Comment 35

[Jonathan Griffin (:jgriffin)]

This patch modifies automation.py.in for the mochitest refactor, which breaks everything else that depends on the status quo.

Perhaps we can avoid deleting anything from automation.py.in and instead simply use less and less of it in mochitest.

Jeff, Ahal, what do you think?

Comment 36

[Andrew Halberstadt [:ahal]]

(In reply to Jonathan Griffin (:jgriffin) from comment #35)
> Perhaps we can avoid deleting anything from automation.py.in and instead
> simply use less and less of it in mochitest.

So I thought that's what the plan was originally, but if Jeff has a patch that works for all harnesses that also use automation.py (which I think is just reftests and leaktests) then I don't see why we can't just take it. It'll just speed things up down the road.

Though yes, making mochitests use less automation.py a bit at a time would probably be safer.

Comment 37

[Andrew Halberstadt [:ahal]]

Jeff, also beware that if you are modifying runtests.py at all, my patch in bug 865349 will definitely bitrot it.

Comment 38

[Jeff Hammel]

(In reply to Jonathan Griffin (:jgriffin) from comment #35)
> This patch modifies automation.py.in for the mochitest refactor, which
> breaks everything else that depends on the status quo.
> 
> Perhaps we can avoid deleting anything from automation.py.in and instead
> simply use less and less of it in mochitest.
> 
> Jeff, Ahal, what do you think?

I'm inclined to:

1. only not delete functions that actively cause problems
2. mark them *heavily* with what problems they cause so that alien archaeologists can piece together the problem

Comment 39

[Jeff Hammel]

(In reply to Andrew Halberstadt [:ahal] from comment #36)
> (In reply to Jonathan Griffin (:jgriffin) from comment #35)
> > Perhaps we can avoid deleting anything from automation.py.in and instead
> > simply use less and less of it in mochitest.
> 
> So I thought that's what the plan was originally, but if Jeff has a patch
> that works for all harnesses that also use automation.py (which I think is
> just reftests and leaktests) then I don't see why we can't just take it.
> It'll just speed things up down the road.

Indeed; this was mostly about getting the patch working and AIUI not the general direction we were going for for the refactor.  That said, as you point out, killing bits at a time certainly doesn't hurt us.

> Though yes, making mochitests use less automation.py a bit at a time would
> probably be safer.

Comment 40

[Jeff Hammel]

(In reply to Andrew Halberstadt [:ahal] from comment #37)
> Jeff, also beware that if you are modifying runtests.py at all, my patch in
> bug 865349 will definitely bitrot it.

Not surprised.

Comment 41

[Jeff Hammel]

Attachment: final form, take 2

Comment 42

[Jeff Hammel]

(In reply to Jeff Hammel [:jhammel] from comment #41)
> Created attachment 770397 [details] [diff] [review]
> final form, take 2

This should (maybe) at least fix the reftest failures

Comment 43

[Jeff Hammel]

Attachment: final form, take 3

and hopefully (maybe) the leaktest failures; will try when i can (currently try is a mess)

Comment 44

[Jeff Hammel]

Attachment: final form, take 4

Comment 45

[Jonathan Griffin (:jgriffin)]

Comment on attachment 770465 [details] [diff] [review]
final form, take 4

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

lgtm, although I didn't try to figure out if this would work with leaktest or not.

Comment 46

[Jeff Hammel]

pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=e3dabd97f582

Comment 47

[Jeff Hammel]

leaktest now dies on this line:                                                 
                                                                                
Traceback (most recent call last):                                              
  File "_leaktest/leaktest.py", line 37, in <module>                            
  File "e:\builds\moz2_slave\try-w32-d-00000000000000000000\build\obj-firefox\_\
leaktest\automation.py", line 347, in initializeProfile                         
    profile_file = open(appsPath, 'r')                                          
IOError: [Errno 2] No such file or directory: 'e:\\builds\\moz2_slave\\try-w32-\
d-00000000000000000000\\build\\obj-firefox\\_leaktest\\webapps_mochitest.json'  
                                                                                
Which tries to load the DEFAULT_APPS_FILE:                                      

+_DEFAULT_APPS_FILE = os.path.join(SCRIPT_DIR, 'webapps_mochitest.json')     

ABICT, leaktest shouldn't use this at all (will make it so); but....what should?  ABICT, only mochitest calls initializeProfile at all besides leaktest, and it provides its own value.  ABICT (again), this value will never be correct for in-tree testing, so I can guess that it is either used for some hypothetical tests.zip case (?) or not at all.

In any case I intend to make leaktest not use this and....if it is for tests.zip, document this; otherwise kill this value.

Comment 48

[Jeff Hammel]

Attachment: if it don't exist, don't use it

Comment 49

[Jeff Hammel]

(In reply to Jeff Hammel [:jhammel] from comment #48)
> Created attachment 772906 [details] [diff] [review]
> if it don't exist, don't use it

pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=1c6e0028f505

I *hope* this at least fixes the leaktest failures

Comment 50

[Jeff Hammel]

> I *hope* this at least fixes the leaktest failures

Nope.  :(  The profile is not getting made correctly.  Debugging now

Comment 51

[Jeff Hammel]

Attachment: actually use the provided profile directory

Comment 52

[Jeff Hammel]

(In reply to Jeff Hammel [:jhammel] from comment #51)
> Created attachment 773698 [details] [diff] [review]
> actually use the provided profile directory

pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=f9bdd62d8576

This works for me locally with mochitest, leaktest, and reftest (noting only for earlier failure).  I do not expect this to fix the android failures (though its possible); I haven't diagnosed the problem there yet.  If diagnosing+fixing android is much more than trivial, then the plan is making android-specific copies of runtests.py and/or automation.py.in as necessary to work around the problem. Ultimately, while (AIUI) android is tier-1, it is not on mozharness  and in several other ways deprioritized wrt the other platforms in such a way to work against modularization and consolidation of next-step code (mozbase, mozbuild, mozharness, etc).

Comment 53

[Jeff Hammel]

Full Log - Android Tegra 250 try opt test mochitest-1 on 2013-07-10 18:04:22 PDT https://tbpl.mozilla.org/php/getParsedLog.php?id=25151377&tree=Try&full=1#error0

========= Started 'python mochitest/runtestsremote.py ...' warnings (results: 1, elapsed: 40 mins, 43 secs) (at 2013-07-10 18:07:41.009472) =========
python mochitest/runtestsremote.py --deviceIP 10.250.50.45 --xre-path ../hostutils/xre --utility-path ../hostutils/bin --certificate-path certs --app org.mozilla.fennec --console-level INFO --http-port 30135 --ssl-port 31135 --pidfile /builds/tegra-135/test/../runtestsremote.pid --run-only-tests android.json --symbols-path=http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-f9bdd62d8576/try-android/fennec-25.0a1.en-US.android-arm.crashreporter-symbols.zip --total-chunks 8 --this-chunk 1
 in dir /builds/tegra-135/test/build/tests (timeout 2400 secs) (maxTime 14400 secs)
 watching logfiles {}
 argv: ['python', 'mochitest/runtestsremote.py', '--deviceIP', '10.250.50.45', '--xre-path', '../hostutils/xre', '--utility-path', '../hostutils/bin', '--certificate-path', 'certs', '--app', 'org.mozilla.fennec', '--console-level', 'INFO', '--http-port', '30135', '--ssl-port', '31135', '--pidfile', '/builds/tegra-135/test/../runtestsremote.pid', '--run-only-tests', 'android.json', u'--symbols-path=http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-f9bdd62d8576/try-android/fennec-25.0a1.en-US.android-arm.crashreporter-symbols.zip', '--total-chunks', '8', '--this-chunk', '1']
 environment:
  HOME=/home/cltbld
  LOGNAME=cltbld
  MINIDUMP_SAVE_PATH=/builds/tegra-135/test/minidumps
  MINIDUMP_STACKWALK=/builds/minidump_stackwalk
  OLDPWD=/home/cltbld
  PATH=/usr/local/bin:/usr/local/bin:/usr/local/bin:/bin:/usr/bin:/usr/local/sbin:/usr/sbin:/sbin:/home/cltbld/bin
  PWD=/builds/tegra-135/test/build/tests
  PYTHONPATH=/builds/sut_tools
  SHELL=/bin/sh
  SHLVL=4
  SUT_IP=10.250.50.45
  SUT_NAME=tegra-135
  USER=cltbld
  _=/tools/buildbot/bin/python2.7
 using PTY: False
Error deleting /data/anr/traces.txt
Device info: {'uptime': ['0 days 0 hours 5 minutes 15 seconds 956 ms'], 'sutuserinfo': [], 'power': ['Power status:', ' AC power ONLINE', ' Battery charge NO BATTERY', ' Remaining charge: 0%', ' Battery Temperature: 0.0 (c)'], 'process': [['10026', '1606', 'com.svox.pico'], ['10028', '1593', 'com.android.defcontainer'], ['10007', '1213', 'com.android.inputmethod.latin'], ['1001', '1223', 'com.android.phone'], ['1000', '1020', 'system'], ['10029', '1328', 'com.android.deskclock'], ['10031', '1495', 'com.mozilla.SUTAgentAndroid'], ['10018', '1235', 'com.android.launcher'], ['10013', '1466', 'com.cooliris.media'], ['10004', '1342', 'android.process.media'], ['10009', '1446', 'com.android.quicksearchbox'], ['10002', '1458', 'com.android.music'], ['10032', '1434', 'com.mozilla.watcher'], ['10006', '1413', 'com.android.mms'], ['10010', '1398', 'com.android.providers.calendar'], ['10014', '1383', 'com.android.email'], ['10017', '1354', 'com.android.bluetooth'], ['10015', '1279', 'android.process.acore'], ['1000', '1251', 'com.android.settings']], 'screen': ['X:1024 Y:768'], 'memory': ['PA:819851264, FREE: 682569728'], 'systime': ['2013/07/10 06:07:42:071'], 'rotation': ['ROTATION:0'], 'disk': ['/data: 222035968 total, 164085760 available', '/system: 250740736 total, 150441984 available', '/mnt/sdcard: 7723384832 total, 7687905280 available'], 'os': ['harmony-eng 2.2 FRF91 20110202.102810 test-keys'], 'id': ['00:26:e8:d4:a6:6a'], 'uptimemillis': ['315977'], 'temperature': ['Temperature: unknown']}
Test root: /mnt/sdcard/tests
args: ['/builds/tegra-135/test/build/hostutils/bin/xpcshell', '-g', '/builds/tegra-135/test/build/hostutils/xre', '-v', '170', '-f', '/builds/tegra-135/test/build/hostutils/bin/components/httpd.js', '-e', "const _PROFILE_PATH = '/tmp/tmpEOQGMi';const _SERVER_PORT = '30135'; const _SERVER_ADDR = '10.250.49.163';\n                     const _TEST_PREFIX = undefined; const _DISPLAY_RESULTS = false;", '-f', './server.js']
INFO | runtests.py | Server pid: 30675
INFO | runtests.py | Running tests: start.

INFO | automation.py | Application pid: 1846

command timed out: 2400 seconds without output, attempting to kill
process killed by signal 9
program finished with exit code -1
elapsedTime=2443.323367
TinderboxPrint: mochitest-plain<br/><em class="testfail">T-FAIL</em>
TinderboxPrint: mochitest-plain<br/><em class="testfail">timeout</em>
========= Finished 'python mochitest/runtestsremote.py ...' warnings (results: 1, elapsed: 40 mins, 43 secs) (at 2013-07-10 18:48:24.731169) =========

Comment 54

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

I downloaded this build and tests.zip, ran it on my tegra and got results.  I found that I was not terminating after completing the test and closing the browser, so I CTRL+C and saw this:
^CException KeyboardInterrupt in <bound method Profile.cleanup of <mozprofile.profile.Profile object at 0x1a61c10>> ignored


Why this fails on tinderbox is beyond me.  This was a tegra run (no mozharness), and I copied/pasted the cli with minimal changes to get it running locally.  One thought is the hostutils could be causing problems?  Seems highly unlikely as that is just for the webserver and having nothing to do with the profile.

The next question is why is the profile.cleanup hanging?  Could be a fluke, could be something serious.

Comment 55

[Jeff Hammel]

One solution is not to clean up the profile.  Easy enough to test if that works

Comment 56

[Jeff Hammel]

Attachment: don't delete the profile

Comment 57

[Jeff Hammel]

(In reply to Jeff Hammel [:jhammel] from comment #56)
> Created attachment 774241 [details] [diff] [review]
> don't delete the profile

pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=02816dd5e813

Comment 58

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

after spending 15 minutes looking at this (most of the time was downloading a build/tests) I ran them locally and saw the problem.  Why I didn't see the problem the first time is concerning, but I am glad I see the problem now.

We set --http-port, and --ssl-port in the automation, mozprofile defaults to 8888 and 4443 and we are not changing those.

Comment 59

[Jeff Hammel]

Attachment: bug-688667

So our documentation sucks and is wrong;  that said, i'll follow up on that and this should work

Comment 60

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

this latest patch works for me locally with --http-port and --ssl-port.  I have pushed this to try, lets see if we are closer.

Comment 61

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

try server run is looking pretty good:
https://tbpl.mozilla.org/?tree=Try&rev=48730d0bddf3

I haven't looked into the tegra rc1+2 failures.  Pandas passed, they run mozharness, something strange is going on for sure:)

Comment 62

[Jeff Hammel]

(In reply to Joel Maher (:jmaher) from comment #61)
> try server run is looking pretty good:
> https://tbpl.mozilla.org/?tree=Try&rev=48730d0bddf3
> 
> I haven't looked into the tegra rc1+2 failures.  Pandas passed, they run
> mozharness, something strange is going on for sure:)

I had also pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=d08d8bed8fc4

Comment 63

[Jonathan Griffin (:jgriffin)]

So there seem to be two relevant failures:

M1 - something related to websockets
rc1, rc2 - something related to extensions

Comment 64

[Jeff Hammel]

Attachment: bug-688667.profile.diff

Got the M1 test_websocket failure (I hope!). I started diffing the profile and when it came to (big surprise) the network.proxy.autoconfig_url there was a small (but huge) difference; the diff attached here shows

< if (isHttp)    return 'PROXY 127.0.0.1:8888';
< if (isHttps || isWebSocket || isWebSocketSSL)    return 'PROXY 127.0.0.1:4443'
;
---
> if (isHttp) return 'PROXY 127.0.0.1:8888';
> if (isHttps) return 'PROXY 127.0.0.1:4443';
> if (isWebSocket) return 'PROXY 127.0.0.1:9988';
> if (isWebSocketSSL) return 'PROXY 127.0.0.1:4443';

Where < marks the profile where the patch is not applied and > with the patch applied.
Changing 9988 -> 4443 does indeed fix at least this particular test; now only to figure out what has put it "that way".

Comment 65

[Jeff Hammel]

Attachment: profilediff

the code i inserted to diff the profiles; this wasn't the route I planned on going, but that it's done, perhaps worth harvesting (see bug 896269 )

Comment 66

[Jeff Hammel]

(In reply to Jeff Hammel [:jhammel] from comment #64)
> Created attachment 780662 [details] [diff] [review]
> bug-688667.profile.diff
> 
> Got the M1 test_websocket failure (I hope!). I started diffing the profile
> and when it came to (big surprise) the network.proxy.autoconfig_url there
> was a small (but huge) difference; the diff attached here shows
> 
> < if (isHttp)    return 'PROXY 127.0.0.1:8888';
> < if (isHttps || isWebSocket || isWebSocketSSL)    return 'PROXY
> 127.0.0.1:4443'
> ;
> ---
> > if (isHttp) return 'PROXY 127.0.0.1:8888';
> > if (isHttps) return 'PROXY 127.0.0.1:4443';
> > if (isWebSocket) return 'PROXY 127.0.0.1:9988';
> > if (isWebSocketSSL) return 'PROXY 127.0.0.1:4443';
> 
> Where < marks the profile where the patch is not applied and > with the
> patch applied.
> Changing 9988 -> 4443 does indeed fix at least this particular test; now
> only to figure out what has put it "that way".

So what put it "that way" in such and such terms is a difference between how automation.py.in generates the proxy setting:

http://hg.mozilla.org/mozilla-central/file/a4c1961bf723/build/automation.py.in#l503
vs
https://github.com/mozilla/mozbase/blob/master/mozprofile/mozprofile/permissions.py#L350

So in one case, a websocket is matched via the SSL port....in the other, it actually gets its own port.
Boo.

IIRC, there is/was a reason for this, though I can't recall it; short term, I will likely fix up the mozprofile code that causes the failure in this case and push to try.  Longer term, we should figure out what we want to do and do it.

Comment 67

[Jeff Hammel]

Attachment: bug-688667

This uses the sslport for the websocket port....which is what, for whatever reason, we do currently.

pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=a22f54b18a51

Actually figuring out the why is probably important, but test_websocket.html failed previously locally...and now it passes.

Comment 68

[Jeff Hammel]

As :jgriffin pointed out, while the android 4.2 rc runs are green with this patch, they both actually have the same error (see, e.g. https://tbpl.mozilla.org/php/getParsedLog.php?id=25731692&tree=Try&full=1#error0 and https://tbpl.mozilla.org/php/getParsedLog.php?id=25490156&tree=Try&full=1#error0 ):

"""
13:22:25     INFO -  INFO | runtests.py | Running tests: end.
13:22:41     INFO -  Traceback (most recent call last):
13:22:41     INFO -    File "/builds/panda-0885/test/build/tests/mochitest/runtestsremote.py", line 638, in main
13:22:41     INFO -      result = mochitest.runTests(options)
13:22:41     INFO -    File "/builds/panda-0885/test/build/tests/mochitest/runtests.py", line 758, in runTests
13:22:41     INFO -      manifest = self.buildProfile(options)
13:22:41     INFO -    File "/builds/panda-0885/test/build/tests/mochitest/runtestsremote.py", line 337, in buildProfile
13:22:41     INFO -      manifest = Mochitest.buildProfile(self, options)
13:22:41     INFO -    File "/builds/panda-0885/test/build/tests/mochitest/runtests.py", line 623, in buildProfile
13:22:41     INFO -      addons=extensions)
13:22:41     INFO -    File "/builds/panda-0885/test/build/tests/mochitest/automation.py", line 367, in initializeProfile
13:22:41     INFO -      proxy=proxy)
13:22:41     INFO -    File "/builds/panda-0885/test/build/tests/mozbase/mozprofile/mozprofile/profile.py", line 87, in __init__
13:22:41     INFO -      self.addon_manager.install_addons(addons, addon_manifests)
13:22:41     INFO -    File "/builds/panda-0885/test/build/tests/mozbase/mozprofile/mozprofile/addons.py", line 52, in install_addons
13:22:41     INFO -      self.install_from_path(addon)
13:22:41     INFO -    File "/builds/panda-0885/test/build/tests/mozbase/mozprofile/mozprofile/addons.py", line 239, in install_from_path
13:22:41     INFO -      dir_util.copy_tree(addon, addon_path, preserve_symlinks=1)
13:22:41     INFO -    File "/tools/python27/lib/python2.7/distutils/dir_util.py", line 163, in copy_tree
13:22:41     INFO -      dry_run=dry_run)
13:22:41     INFO -    File "/tools/python27/lib/python2.7/distutils/file_util.py", line 148, in copy_file
13:22:41     INFO -      _copy_file_contents(src, dst)
13:22:41     INFO -    File "/tools/python27/lib/python2.7/distutils/file_util.py", line 44, in _copy_file_contents
13:22:41     INFO -      fdst = open(dst, 'wb')
13:22:41     INFO -  IOError: [Errno 2] No such file or directory: u'/tmp/tmpmiMW9B/extensions/staged/[e-mail]/chrome.manifest'
"""

Comment 69

[Jeff Hammel]

From a successful run:

11:08:29     INFO -  INFO | runtests.py | Installing extension at /builds/panda-0735/test/build/tests/mochitest/extensions/specialpowers to /tmp/tmpDPm3_H.
11:08:29     INFO -  INFO | runtests.py | Installing extension at /builds/panda-0735/test/build/tests/mochitest/extensions/workerbootstrap to /tmp/tmpDPm3_H.
11:08:29     INFO -  INFO | runtests.py | Installing extension at /builds/panda-0735/test/build/tests/mochitest/extensions/[e-mail] to /tmp/tmpDPm3_H.
11:08:29     INFO -  INFO | runtests.py | Installing extension at /builds/panda-0735/test/build/tests/mochitest/extensions/worker to /tmp/tmpDPm3_H.

Comment 70

[Jeff Hammel]

(In reply to Jeff Hammel [:jhammel] from comment #68)
> As :jgriffin pointed out, while the android 4.2 rc runs are green with this
> patch, they both actually have the same error (see, e.g.
> https://tbpl.mozilla.org/php/getParsedLog.
> php?id=25731692&tree=Try&full=1#error0 and
> https://tbpl.mozilla.org/php/getParsedLog.
> php?id=25490156&tree=Try&full=1#error0 ):
> 

but 898165

Comment 71

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

Attachment: additional changes to fix bitrot and robocop tests

Comment 72

[Jeff Hammel]

unbitrotting

│curl https://bug688667.bugzilla.mozilla.org/attachment.cgi?id=781184 | command patch -p1 --dry-run
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 21653  100 21653    0     0  18647      0  0:00:01  0:00:01 --:--:-- 22391
patching file build/automation.py.in
patching file build/leaktest.py.in
patching file build/mach_bootstrap.py
patching file testing/mochitest/runtests.py
Hunk #1 FAILED at 6.
Hunk #2 FAILED at 603.
Hunk #3 FAILED at 943.
3 out of 3 hunks FAILED -- saving rejects to file testing/mochitest/runtests.py.rej

Comment 73

[Jeff Hammel]

Attachment: bug-688667

Comment 74

[Jeff Hammel]

SCRIPT_DIRECTORY -> SCRIPT_DIR; fixed in latest patch

(In reply to Jeff Hammel [:jhammel] from comment #73)
> Created attachment 782684 [details] [diff] [review]
> bug-688667

Comment 75

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

Comment on attachment 782684 [details] [diff] [review]
bug-688667

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

just some nits.

::: build/automation.py.in
@@ +371,5 @@
> +
> +    proxy = {'remote': str(self.webServer),
> +             'http': str(self.httpPort),
> +             'https': str(self.sslPort),
> +    #             'ws': str(self.webSocketPort)

why is this commented out?

::: build/leaktest.py.in
@@ +37,5 @@
>      t.setDaemon(True)
>      t.start()
>  
>      automation.setServerInfo("localhost", PORT)
> +    profile = automation.initializeProfile(PROFILE_DIRECTORY)

why do you store this when you didn't before?  I don't see changes that use this.

::: build/mach_bootstrap.py
@@ +49,5 @@
>      'testing/mozbase/mozprocess',
>      'testing/mozbase/mozprofile',
>      'testing/mozbase/mozrunner',
>      'testing/mozbase/mozinfo',
> +    'testing/mozbase/manifestdestiny',

are we using manifestdestiny in here?  is it a dependency of mozprofile?

::: testing/mochitest/runtests.py
@@ +432,5 @@
>      if options.browserChrome and options.timeout:
>        options.extraPrefs.append("testing.browserTestHarness.timeout=%d" % options.timeout)
> +
> +        # get extensions to install
> +    extensions = self.getExtensionsToInstall(options)

spacing on the comment here is inconsistent.

::: testing/mochitest/runtestsremote.py
@@ +606,5 @@
> +            if mochitest.localProfile:
> +                options.profilePath = mochitest.localProfile
> +                os.system("rm -Rf %s" % options.profilePath)
> +                options.profilePath = tempfile.mkdtemp()
> +                mochitest.localProfile = options.profilePath

please comment in here why this is happening:
# When running in a loop, we need to create a fresh profile for each cycle

Comment 76

[Jeff Hammel]

(In reply to Jeff Hammel [:jhammel] from comment #73)
> Created attachment 782684 [details] [diff] [review]
> bug-688667

pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=06ace441351b

Comment 77

[Jeff Hammel]

(In reply to Jeff Hammel [:jhammel] from comment #66)
> (In reply to Jeff Hammel [:jhammel] from comment #64)
> > Created attachment 780662 [details] [diff] [review]
> > bug-688667.profile.diff
> > 
> > Got the M1 test_websocket failure (I hope!). I started diffing the profile
> > and when it came to (big surprise) the network.proxy.autoconfig_url there
> > was a small (but huge) difference; the diff attached here shows
> > 
> > < if (isHttp)    return 'PROXY 127.0.0.1:8888';
> > < if (isHttps || isWebSocket || isWebSocketSSL)    return 'PROXY
> > 127.0.0.1:4443'
> > ;
> > ---
> > > if (isHttp) return 'PROXY 127.0.0.1:8888';
> > > if (isHttps) return 'PROXY 127.0.0.1:4443';
> > > if (isWebSocket) return 'PROXY 127.0.0.1:9988';
> > > if (isWebSocketSSL) return 'PROXY 127.0.0.1:4443';
> > 
> > Where < marks the profile where the patch is not applied and > with the
> > patch applied.
> > Changing 9988 -> 4443 does indeed fix at least this particular test; now
> > only to figure out what has put it "that way".
> 
> So what put it "that way" in such and such terms is a difference between how
> automation.py.in generates the proxy setting:
> 
> http://hg.mozilla.org/mozilla-central/file/a4c1961bf723/build/automation.py.
> in#l503
> vs
> https://github.com/mozilla/mozbase/blob/master/mozprofile/mozprofile/
> permissions.py#L350
> 
> So in one case, a websocket is matched via the SSL port....in the other, it
> actually gets its own port.
> Boo.
> 
> IIRC, there is/was a reason for this, though I can't recall it; short term,
> I will likely fix up the mozprofile code that causes the failure in this
> case and push to try.  Longer term, we should figure out what we want to do
> and do it.

Filed bug 899221

Comment 78

[Jeff Hammel]

(In reply to Joel Maher (:jmaher) from comment #75)
> Comment on attachment 782684 [details] [diff] [review]
> bug-688667
> 
> Review of attachment 782684 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> just some nits.
> 
> ::: build/automation.py.in
> @@ +371,5 @@
> > +
> > +    proxy = {'remote': str(self.webServer),
> > +             'http': str(self.httpPort),
> > +             'https': str(self.sslPort),
> > +    #             'ws': str(self.webSocketPort)
> 
> why is this commented out?

Hardly a nit.  The real answer is "I don't know, cargo-culting"; but see also comment 66 and bug 899221

> ::: build/leaktest.py.in
> @@ +37,5 @@
> >      t.setDaemon(True)
> >      t.start()
> >  
> >      automation.setServerInfo("localhost", PORT)
> > +    profile = automation.initializeProfile(PROFILE_DIRECTORY)
> 
> why do you store this when you didn't before?  I don't see changes that use
> this.

Otherwise, the scope is immediately killed and the profile cleaned up, ABICT.

> ::: build/mach_bootstrap.py
> @@ +49,5 @@
> >      'testing/mozbase/mozprocess',
> >      'testing/mozbase/mozprofile',
> >      'testing/mozbase/mozrunner',
> >      'testing/mozbase/mozinfo',
> > +    'testing/mozbase/manifestdestiny',
> 
> are we using manifestdestiny in here?  is it a dependency of mozprofile?

Yes it is: https://github.com/mozilla/mozbase/blob/master/mozprofile/mozprofile/addons.py#L11

Though the other answer is....really, all mozbase modules should be included here and this is a hack.

> ::: testing/mochitest/runtests.py
> @@ +432,5 @@
> >      if options.browserChrome and options.timeout:
> >        options.extraPrefs.append("testing.browserTestHarness.timeout=%d" % options.timeout)
> > +
> > +        # get extensions to install
> > +    extensions = self.getExtensionsToInstall(options)
> 
> spacing on the comment here is inconsistent.

Fixed.

> ::: testing/mochitest/runtestsremote.py
> @@ +606,5 @@
> > +            if mochitest.localProfile:
> > +                options.profilePath = mochitest.localProfile
> > +                os.system("rm -Rf %s" % options.profilePath)
> > +                options.profilePath = tempfile.mkdtemp()
> > +                mochitest.localProfile = options.profilePath
> 
> please comment in here why this is happening:
> # When running in a loop, we need to create a fresh profile for each cycle

Added, thanks

Comment 79

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

please add comments to:

> ::: build/automation.py.in
> @@ +371,5 @@
> > +
> > +    proxy = {'remote': str(self.webServer),
> > +             'http': str(self.httpPort),
> > +             'https': str(self.sslPort),
> > +    #             'ws': str(self.webSocketPort)
> 
> why is this commented out?

and:
> ::: build/leaktest.py.in
> @@ +37,5 @@
> >      t.setDaemon(True)
> >      t.start()
> >  
> >      automation.setServerInfo("localhost", PORT)
> > +    profile = automation.initializeProfile(PROFILE_DIRECTORY)
> 
> why do you store this when you didn't before?  I don't see changes that use
> this.

If there is some reference in the code as to why we do this, folks won't be as confused in the future.

Comment 80

[Jeff Hammel]

(In reply to Joel Maher (:jmaher) from comment #79)
> please add comments to:
> 
> > ::: build/automation.py.in
> > @@ +371,5 @@
> > > +
> > > +    proxy = {'remote': str(self.webServer),
> > > +             'http': str(self.httpPort),
> > > +             'https': str(self.sslPort),
> > > +    #             'ws': str(self.webSocketPort)
> > 
> > why is this commented out?
> 
> and:
> > ::: build/leaktest.py.in
> > @@ +37,5 @@
> > >      t.setDaemon(True)
> > >      t.start()
> > >  
> > >      automation.setServerInfo("localhost", PORT)
> > > +    profile = automation.initializeProfile(PROFILE_DIRECTORY)
> > 
> > why do you store this when you didn't before?  I don't see changes that use
> > this.
> 
> If there is some reference in the code as to why we do this, folks won't be
> as confused in the future.

Comments added: http://k0s.org/mozilla/hg/mozilla-central-patches/rev/be2a81c5415c

Comment 81

[Jeff Hammel]

Attachment: bug-688667

carrying r+ forward

Comment 82

[Jeff Hammel]

Attachment: unbitrot

r=jmaher; tried to land but...closed for bustage.  feel free to land when open

Comment 83

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/c22896a27564

Comment 84

[Jeff Hammel]

This doesn't use mozprocess, but I'm content leaving this closed as our current strategy is to factor the harnesses away from automation.py.in : https://bugzilla.mozilla.org/show_bug.cgi?id=746243#c31