Closed Bug 611230 Opened 14 years ago Closed 14 years ago

remote mochitest fails to honor --utility-path

Categories

(Testing :: General, defect)

x86
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: jmaher, Assigned: jmaher)

References

Details

(Whiteboard: [mobile_unittests])

Attachments

(1 file, 1 obsolete file)

Attached patch remote testing cleanup (1.0) (obsolete) — Splinter Review
when :bear started doing mochitest via buildbot, he ran into an issue where he was setting the --utility-path (required for the location of the host machine xpcshell binary) and it was not being honored.

I looked into this and saw that we were doing all kinds of crazy stuff to the utility path to deal with local vs remote.

In addition I added a few other small fixes:
 * fix the os detection logic for webserver ip detection so we can run on windows
 * add browser.console.showInPanel as a default option in our profile.

Clint, I have tested this patch locally, I would like to know how this affects your tracemonkey setup.
Attachment #489734 - Flags: review?(ctalbert)
runs green on try server.
Comment on attachment 489734 [details] [diff] [review]
remote testing cleanup (1.0)

>diff --git a/testing/mochitest/runtestsremote.py b/testing/mochitest/runtestsremote.py
>--- a/testing/mochitest/runtestsremote.py
>+++ b/testing/mochitest/runtestsremote.py
>@@ -102,42 +102,42 @@ class RemoteOptions(MochitestOptions):
>         defaults["closeWhenDone"] = True
>         defaults["testPath"] = ""
>         defaults["app"] = None
> 
>         self.set_defaults(**defaults)
> 
>     def verifyRemoteOptions(self, options, automation):
>         options.remoteTestRoot = automation._devicemanager.getDeviceRoot()
>+        productRoot = options.remoteTestRoot + "/" + automation._product
> 
>-        options.utilityPath = options.remoteTestRoot + "/bin"
>-        options.certPath = options.remoteTestRoot + "/certs"
>+        if (options.utilityPath == self._automation.DIST_BIN):
>+            options.utilityPath = productRoot + "/bin"
> 
>-       
>-        if options.remoteWebServer == None and os.name != "nt":
>-            options.remoteWebServer = automation.getLanIp()
>-        elif os.name == "nt":
>-            print "ERROR: you must specify a remoteWebServer ip address\n"
>-            return None
>+        if (options.certPath == self._automation.CERTS_SRC_DIR):
>+            options.certPath = options.remoteTestRoot + "/certs"
I think this is wrong.  We run ssltunnel on the controller box, not the device.  So the CERTS path should refer to the location of certs (where they have been unpacked on the controller, and not on the remote device).  I don't think you want to reset CERTS to point to a remote location.  I'd leave that unchanged.

>         if (options.remoteLogFile == None):
>-            options.remoteLogFile = automation._devicemanager.getDeviceRoot() + '/test.log'
>+            options.remoteLogFile = options.remoteTestRoot + '/test.log'
I'd make this mochitest.log just to be specific so that when you are looking on the device it will be easy to find it and know what you're looking at.

I like these changes in general, I just think that we need a few more tweaks to get it done.
Attachment #489734 - Flags: review?(ctalbert) → review-
updated with 'certs' stuff cleaned up.
Assignee: nobody → jmaher
Attachment #489734 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #491520 - Flags: review?(ctalbert)
Comment on attachment 491520 [details] [diff] [review]
remote testing cleanup (1.1)

This looks great, except I think you left one debugging thing in here...

># HG changeset patch
># Parent 6435d8877ae2d86381b16225145a26604c8dc0a9
>
>diff --git a/build/automation.py.in b/build/automation.py.in
>--- a/build/automation.py.in
>+++ b/build/automation.py.in
>@@ -327,16 +327,17 @@ class Automation(object):
>     os.mkdir(profileDir)
> 
>     # Set up permissions database
>     locations = self.readLocations()
>     self.setupPermissionsDatabase(profileDir,
>       {'allowXULXBL':[(l.host, 'noxul' not in l.options) for l in locations]});
> 
>     part = """\
>+user_pref("browser.console.showInPanel", true);
Why do you add this to the preferences ^ for the profile?  I think that's debugging stuff creeping in here?

r+ with that removed.
Attachment #491520 - Flags: review?(ctalbert) → review+
I had added that on purpose (see original comment).  I can remove that if you prefer.
Landed, decided to keep that pref. http://hg.mozilla.org/mozilla-central/rev/74a3cd2a9ff5
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Component: New Frameworks → General
You need to log in before you can comment on or make changes to this bug.