545905 - add remotereftests.py to mozilla-central

Status: RESOLVED FIXED

(Testing :: General, defect)

Description

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

in order to run reftests remotely, we need to subclass runreftests.py and make it work with a test-agent.exe interface.  This depends on bug 493792 and devicemanager.py.

Comment 1

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

Attachment: subclass of reftest and related classes (WIP)

WIP patch that demonstrates an almost working reftest on windows mobile.

Comment 2

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

Attachment: subclass of reftest and related classes (1)

updated for bitrot.  I still haven't seen this work in my environment with the registration.

Comment 3

[cmtalbert]

Comment on attachment 431887 [details] [diff] [review]
subclass of reftest and related classes (1)

>diff --git a/layout/tools/reftest/remotereftest.py b/layout/tools/reftest/remotereftest.py
>+    def waitForFinish(self, proc, utilityPath, timeout, maxTime, startTime):
>+        status = proc.wait()
>+        print proc.stdout
>+        # todo: consider pulling log file from remote
>+        return status
We should pull the remote log file back to the reftest directory.  I believe this is process.txt as you're launching this currently.

>+    class RProcess(object):
>+        #device manager process
>+        dm = None
>+        def __init__(self, dm, product, cmd, stdout = None, stderr = None, env = None, cwd = '.'):
>+            self.dm = dm
>+            print "going to launch process: " + str(self.dm.host)
>+            self.proc = dm.launchProcess(cmd)
>+            #TODO: this is hardcoded for winmo/wince
>+            if (product == "fennec"):
>+              self.procName = "fennec.exe"
>+            else:
>+              self.procName = "firefox.exe"
>+            self.timeout = 600
>+            time.sleep(5)
We need to add an --appname to handle this, we can't hard code this.

>+class RemoteOptions(ReftestOptions):
>+    def __init__(self, automation):
>+        ReftestOptions.__init__(self, automation)
>+
>+        defaults = {}
>+        defaults["logFile"] = "mochitest.log"
It ought to default to reftest.log, honestly.

>+        if (automation._product == "fennec"):
>+          defaults["app"] = "/tests/" + automation._product + "/fennec.exe"
>+        else:
>+          defaults["app"] = "/tests/" + automation._product + "/firefox.exe"
Got to use an appname here.

>+        defaults["xrePath"] = "/tests/" + automation._product + "/xulrunner"
>+        defaults["utilityPath"] = "/tests/" + automation._product + "/bin"
>+
>+        self.add_option("--device", action="store",
>+                    type = "string", dest = "device",
>+                    help = "ip address of remote device to test")
>+        defaults["device"] = None
>+
>+        self.add_option("--devicePort", action="store",
>+                    type = "string", dest = "devicePort",
>+                    help = "port of remote device to test")
>+        defaults["devicePort"] = 27020
>+
>+        self.add_option("--remoteProductName", action="store",
>+                    type = "string", dest = "remoteProductName",
>+                    help = "Name of remote product to test - either fennec or firefox, defaults to fennec")
>+        defaults["remoteProductName"] = "fennec"
>+
>+        self.add_option("--remoteAppName", action="store",
>+                    type = "string", dest = "remoteAppName",
>+                    help = "Executable name for remote device, OS dependent, defaults to fennec.exe")
>+        defaults["remoteAppName"] = "fennec.exe"
Oh we can use this to eliminate the hardcoding above!

>+
>+class RemoteReftest(RefTest):
>+    remoteProfileDir = '/tests/profile'
>+    remoteApp = '/tests/fennec/fennec.exe'
>+    remoteTestRoot = '/tests'
This should use the --app option that is passed in.

>+
>+    def registerExtension(self, browserEnv, options, profileDir):
>+        #todo, the silent option doesn't appear to work, so I kill this manually for now
>+        pass
>+#        RefTest.registerExtension(self, browserEnv, options, 
>+#                        self.remoteProfileDir, 
>+#                        extraArgs = ["-silent"])
This obviously doesn't do anything, and in testing this, it doesn't seem like it is needed for fennec?  I'm not sure why it isn't though. Curious. Can we just pass on it without the commented out bits?

>+
>+    def cleanup(self, profileDir):
>+#        self._devicemanager.removeDir(self.remoteProfileDir)
>+#        self._devicemanager.removeDir(self.remoteTestRoot)
>+        RefTest.cleanup(self, profileDir)
Let's get rid of the commented out code if it isn't needed.

>+
>+def main():
>+    dm = DeviceManager(None, None)
>+    automation = RemoteAutomation(dm, "fennec")
The "fennec" here ought to be an option.

r=me with those things addressed.  Since I'm testing this code out tonight, I'll see if I can put a patch together that addresses these issues.

Comment 4

[cmtalbert]

Attachment: remotereftest1.1

Removed all the hard coding, verified that it still works.  I punted on the pulling log file back to the device in RProcess because that will require deeper changes to the automation.py infrastructure so that the name of the log file is known and can be passed down.  I think instead, I will handle that directly on the buildbot side for now.

Comment 5

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

Attachment: subclass of reftest and related classes (2)

updated with feedback.  I noticed the remotereftest1.1 attachment didn't resolve the majority of the feedback mentioned.  I have found a way to remove the hardcoding and cleaned up a few pieces of the code as well.

Would like some basic review on this before checking it in.

Comment 6

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

as a note for regular desktop reftest, the output is sent to stdout and parsed with the buildbot scripts.  As it stands, this script will do the same thing.  I don't believe there is a need to define a log file (local or remote) in the code.

Comment 7

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

Attachment: subclass of reftest and related classes (3)

added initial support for --extra-profile-file.

Comment 8

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

Attachment: subclass of reftest and related classes (4)

updated patch with changes to terminate test on common file transaction failures on the device.

Comment 9

[cmtalbert]

Comment on attachment 432581 [details] [diff] [review]
subclass of reftest and related classes (4)

Looks good.  My only nit here is to capitalize the error messages that are thrown, i.e. "failed..." -> "Failed..."

r=me.

Comment 10

[cmtalbert]

Fixed the nit and Checked in as changeset: d348c121f6a6

Comment 11

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

This needs a copyright header: <http://www.mozilla.org/MPL/boilerplate-1.1/mpl-tri-license-sh>