Bug 1282522 - Record stderr from xpcshell test processes; r?ted draft
authorGregory Szorc <gps@mozilla.com>
Tue, 05 Jul 2016 14:03:59 -0700
changeset 384229 12a920a5fcc01ded3353964ab7732d7e1ba8b6c6
parent 384060 dbb31bcad5a1f60a35b5600ea1578d9b9fa55237
child 524651 05a2b43dfb9c3b11287bc2dd0713f2864f9c09e4
push id22214
push userbmo:gps@mozilla.com
push dateTue, 05 Jul 2016 21:04:59 +0000
reviewersted
bugs1282522
milestone50.0a1
Bug 1282522 - Record stderr from xpcshell test processes; r?ted Before we were throwing away stderr wholesale. As the bug report demonstrates, this was throwing away useful information to help debug a crash. This patch explicitly captures and processes stderr output so it doesn't get lost. MozReview-Commit-ID: Kila7BQLnox
testing/xpcshell/runxpcshelltests.py
--- a/testing/xpcshell/runxpcshelltests.py
+++ b/testing/xpcshell/runxpcshelltests.py
@@ -218,26 +218,39 @@ class XPCShellTestThread(Thread):
     def communicate(self, proc):
         """
           Simple wrapper to communicate with a process.
           On a remote system, this is overloaded to handle remote process communication.
         """
         # Processing of incremental output put here to
         # sidestep issues on remote platforms, where what we know
         # as proc is a file pulled off of a device.
+        # TODO this is not the ideal way to implement communicate() for 2
+        # pipes: we should ideally poll() and read from the one with data
+        # until no data remains.
         if proc.stdout:
             while True:
                 line = proc.stdout.readline()
                 if not line:
                     break
                 self.process_line(line)
 
             if self.saw_proc_start and not self.saw_proc_end:
                 self.has_failure_output = True
 
+        if proc.stderr and proc.stderr != proc.stdout:
+            while True:
+                line = proc.stderr.readline()
+                if not line:
+                    break
+                self.process_line(line)
+
+            if self.saw_proc_start and not self.saw_proc_end:
+                self.has_failure_output = True
+
         return proc.communicate()
 
     def launchProcess(self, cmd, stdout, stderr, env, cwd, timeout=None):
         """
           Simple wrapper to launch a process.
           On a remote system, this is more complex and we need to overload this function.
         """
         # timeout is needed by remote and b2g xpcshell to extend the
@@ -690,31 +703,34 @@ class XPCShellTestThread(Thread):
                 # On mobile, "proc" is just a file.
                 self.proc_ident = name
 
             if self.interactive:
                 self.log.info("%s | Process ID: %d" % (name, self.proc_ident))
 
             # Communicate returns a tuple of (stdout, stderr), however we always
             # redirect stderr to stdout, so the second element is ignored.
-            process_output, _ = self.communicate(proc)
+            process_output, process_error = self.communicate(proc)
 
             if self.interactive:
                 # Not sure what else to do here...
                 self.keep_going = True
                 return
 
             if testTimer:
                 testTimer.cancel()
 
             if process_output:
                 # For the remote case, stdout is not yet depleted, so we parse
                 # it here all at once.
                 self.parse_output(process_output)
 
+            if process_error:
+                self.parse_output(process_error)
+
             return_code = self.getReturnCode(proc)
 
             # TSan'd processes return 66 if races are detected.  This isn't
             # good in the sense that there's no way to distinguish between
             # a process that would normally have returned zero but has races,
             # and a race-free process that returns 66.  But I don't see how
             # to do better.  This ambiguity is at least constrained to the
             # with-TSan case.  It doesn't affect normal builds.