assumes process.txt exists even when we provide a log file



9 years ago
2 years ago


(Reporter: jmaher, Assigned: jmaher)


Firefox Tracking Flags

(Not tracked)



(1 attachment, 1 obsolete attachment)

for all remote testing, we by default redirect the output to process.txt:
fennec args > path/process.txt

when we query the process in, we look for proc.stdout which in this case is a deviceManager.getFile(process.txt).  Since we are passing in an output file, we should treat the stdout as a deviceManager.getFile(<logfilename>).

I suggest modifying this code:

so that it goes from:
  def launchProcess(self, cmd, outputFile = "process.txt", cwd = ''):
    if (outputFile == "process.txt"):
      outputFile = self.getDeviceRoot() + '/' + "process.txt"

    cmdline = subprocess.list2cmdline(cmd)
    self.fireProcess(cmdline + " > " + outputFile)
    handle = outputFile

    return handle

  def launchProcess(self, cmd, outputFile = "process.txt", cwd = ''):
    cmdline = subprocess.list2cmdline(cmd)
    if (outputFile == "process.txt" || outputFile == None):
      outputFile = self.getDeviceRoot() + '/' + "process.txt"
      self.fireProcess(cmdline + " > " + outputFile)
    return outputFile

Next in, change:
        def __init__(self, dm, cmd, stdout = None, stderr = None, env = None, cwd = '.'):
   = dm
            print "going to launch process: " + str(
            self.proc = dm.launchProcess(cmd)
            exepath = cmd[0]
            name = exepath.split('/')[-1]
            self.procName = name

        def __init__(self, dm, cmd, stdout = None, stderr = None, env = None, cwd = '.'):
   = dm
            print "going to launch process: " + str(
            self.proc = dm.launchProcess(cmd, stdout)
            exepath = cmd[0]
            name = exepath.split('/')[-1]
            self.procName = name

Finally in the RemoteAutomation.init() add a remoteLogFile parameter so we can set it as a member of the class and overload Process.stdout to be self.remoteLogFile.

Of course this is just my idea of how to solve this.  Please comment if this doesn't make sense, otherwise it needs some testing:)
Blocks: 579566
Yeah this makes sense, Let's go your route there, Joel.
when you specify a --remote-logfile='name', we will use that and have reftest log directly to that log file.  This patch builds on that logic and removes the redirect code in devicemanager and remoteautomation and fixes the code to look for the remoteLogFile variable instead of a hardcoded process.txt.

Testing using remote testing on n900.

as a note, this is the that is currently in the works for talos which would make them synchronized (for once)
Assignee: nobody → jmaher
Attachment #475386 - Flags: review?(ctalbert)
Comment on attachment 475386 [details] [diff] [review]
remote reftest uses direct logging instead of stdout (1.0)

>diff --git a/build/mobile/ b/build/mobile/
>   def launchProcess(self, cmd, outputFile = "process.txt", cwd = ''):
>-    if (outputFile == "process.txt"):
>+    cmdline = subprocess.list2cmdline(cmd)
>+    if (outputFile == "process.txt" or outputFile == None):
>       outputFile = self.getDeviceRoot() + '/' + "process.txt"
>+      cmdline += " > " + outputFile
This is kind of hacky, but I'm ok with that.  I can't think of a cleaner way to do it without extensive refactoring, so let's go with this approach for now.

>@@ -711,8 +710,34 @@ class DeviceManager:
>   """
>   def uninstallAppAndReboot(self, appName, installPath=None):
>     cmd = 'uninst ' + appName
>     if installPath:
>       cmd += ' ' + installPath
>     self.sendCMD([cmd])
>     return True
>+  """
>+    edit the user.js in the profile (on the host machine) and 
>+    add the xpconnect priviledges for the remote server
>+  """
>+  def addRemoteServerPref(self, profile_dir, server):
>+    user_js_filename = os.path.join(profile_dir, 'user.js')
>+    user_js_file = open(user_js_filename, 'a+')
>+    #TODO: p2 is hardcoded, how do we determine what prefs.js has hardcoded?
>+    remoteCode = """
>+user_pref("capability.principal.codebase.p1.granted", "UniversalPreferencesWrite UniversalXPConnect UniversalPreferencesRead");
>+user_pref("", "http://%(server)s");
>+user_pref("capability.principal.codebase.p1.subjectName", "");
>+""" % { "server": server }
>+    user_js_file.write(remoteCode)
>+    user_js_file.close()
This function doesn't belong here.  This should go into the remote testing code that handles setting up profiles for testing.  We should focus on keeping a simple wrapper to communicate with a remote device.  I want to resist putting specialized things like this into it because if that continues it will make this code a lot harder to maintain.  Please put this into the necessary talos remote code.

The rest of the patch looks great.  r+ with that change.
Attachment #475386 - Flags: review?(ctalbert) → review+
updated with the cleanup requested.  kicked off a try server run just to be safe.
Attachment #475386 - Attachment is obsolete: true
Attachment #475609 - Flags: review+
Closed: 9 years ago
Resolution: --- → FIXED
Component: New Frameworks → General
You need to log in before you can comment on or make changes to this bug.