Closed Bug 688667 Opened 8 years ago Closed 6 years ago

refactor automation.py into mozprofile and load via virtualenv

Categories

(Testing :: General, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla25

People

(Reporter: k0scist, Assigned: k0scist)

References

(Depends on 1 open bug)

Details

(Whiteboard: [mozbase])

Attachments

(6 files, 12 obsolete files)

12.95 KB, patch
Details | Diff | Splinter Review
13.25 KB, patch
Details | Diff | Splinter Review
1.75 KB, patch
Details | Diff | Splinter Review
21.24 KB, patch
jmaher
: review+
Details | Diff | Splinter Review
21.57 KB, patch
k0scist
: review+
Details | Diff | Splinter Review
21.55 KB, patch
k0scist
: review+
Details | Diff | Splinter Review
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.
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).
Duplicate of this bug: 536388
Blocks: 775756
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.
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.
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
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?
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.
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
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.
Assignee: nobody → ahalberstadt
Status: NEW → ASSIGNED
Where are we on this, ahal?
Flags: needinfo?(ahalberstadt)
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.
Depends on: 865349
Flags: needinfo?(ahalberstadt)
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!)
Assignee: ahalberstadt → jhammel
Depends on: 875527
Joel, I assume the permissions.py fix we want anyway?  Ticketed separately for my sanity in bug 875527
yes we NEED that permissions fix, and then it won't need to be hacked on my patch for m-c!
(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
Attached patch unbitrot (obsolete) — Splinter Review
straight unbitrot of attachment 753366 [details] [diff] [review]; untested
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.
is there a specific scenario where this is failing?  Is this failing on tryserver?
(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
Will post output shortly
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? :(
See Also: → 794506
This problem fixed with http://k0s.org/mozilla/hg/mozilla-central-patches/rev/e383fb24a00d

We might want to do this thing moar betterz
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
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"}]})
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.
(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.
+    #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
+    #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:
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.
Attached patch modern form of patch, so far (obsolete) — Splinter Review
(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
Attached patch final form, take 1 (obsolete) — Splinter Review
Attachment #770250 - Attachment is obsolete: true
Attachment #770286 - Flags: review?(jgriffin)
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
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
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?
(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.
Jeff, also beware that if you are modifying runtests.py at all, my patch in bug 865349 will definitely bitrot it.
(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
(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.
(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.
Attached patch final form, take 2 (obsolete) — Splinter Review
Attachment #770286 - Attachment is obsolete: true
Attachment #770286 - Flags: review?(jgriffin)
Attachment #770397 - Flags: review?(jgriffin)
(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
Attached patch final form, take 3 (obsolete) — Splinter Review
and hopefully (maybe) the leaktest failures; will try when i can (currently try is a mess)
Attachment #770397 - Attachment is obsolete: true
Attachment #770397 - Flags: review?(jgriffin)
Attachment #770459 - Flags: review?(jgriffin)
Attached patch final form, take 4 (obsolete) — Splinter Review
Attachment #770459 - Attachment is obsolete: true
Attachment #770459 - Flags: review?(jgriffin)
Attachment #770465 - Flags: review?(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.
Attachment #770465 - Flags: review?(jgriffin) → review+
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.
Attached patch if it don't exist, don't use it (obsolete) — Splinter Review
(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
> I *hope* this at least fixes the leaktest failures

Nope.  :(  The profile is not getting made correctly.  Debugging now
Attachment #772906 - Attachment is obsolete: true
(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).
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/jhammel@mozilla.com-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/jhammel@mozilla.com-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) =========
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.
One solution is not to clean up the profile.  Easy enough to test if that works
Attached patch don't delete the profile (obsolete) — Splinter Review
(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
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.
Attached patch bug-688667 (obsolete) — Splinter Review
So our documentation sucks and is wrong;  that said, i'll follow up on that and this should work
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.
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:)
(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
So there seem to be two relevant failures:

M1 - something related to websockets
rc1, rc2 - something related to extensions
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".
Attached patch profilediffSplinter Review
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 )
See Also: → 896269
(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.
Attached patch bug-688667 (obsolete) — Splinter Review
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.
Attachment #753366 - Attachment is obsolete: true
Attachment #766861 - Attachment is obsolete: true
Attachment #770465 - Attachment is obsolete: true
Attachment #773698 - Attachment is obsolete: true
Attachment #774241 - Attachment is obsolete: true
Attachment #778027 - Attachment is obsolete: true
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/worker-test@mozilla.org/chrome.manifest'
"""
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/roboextender@mozilla.org 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.
(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
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
Attached patch bug-688667Splinter Review
Attachment #781184 - Attachment is obsolete: true
Attachment #782684 - Flags: review?(jmaher)
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 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
Attachment #782684 - Flags: review?(jmaher) → review+
(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
(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
(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
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.
(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
Attached patch bug-688667Splinter Review
carrying r+ forward
Attachment #782925 - Flags: review+
Attached patch unbitrotSplinter Review
r=jmaher; tried to land but...closed for bustage.  feel free to land when open
Attachment #782928 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/c22896a27564
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
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
Summary: refactor automation.py into mozprocess and mozprofile and load via virtualenv → refactor automation.py into mozprofile and load via virtualenv
Depends on: 903962
You need to log in before you can comment on or make changes to this bug.