Bug 1257476 - Marionette has to force close the process if it doesn't shut down itself. r?automatedtester draft
authorHenrik Skupin <mail@hskupin.info>
Fri, 22 Jul 2016 14:36:47 +0200
changeset 392981 40f922b87605fc786cec8e13a5dbffebc6f5fbd0
parent 392980 c822b556c0933a4ba61afebd884ba46ee08e2e00
child 526446 87012f77b1d45448907fbab5e23e97812940fa02
push id24162
push userbmo:hskupin@gmail.com
push dateTue, 26 Jul 2016 17:10:58 +0000
reviewersautomatedtester
bugs1257476
milestone50.0a1
Bug 1257476 - Marionette has to force close the process if it doesn't shut down itself. r?automatedtester Under some circumstances Marionette currently fails to stop the application in case of socket issues. To ensure that the application always gets closed - in the case when Marionette started it - the check for crashes decorator gets updated to do a full process check. MozReview-Commit-ID: DAiF2ZjAjT5
testing/marionette/client/marionette_driver/decorators.py
testing/marionette/client/marionette_driver/marionette.py
--- a/testing/marionette/client/marionette_driver/decorators.py
+++ b/testing/marionette/client/marionette_driver/decorators.py
@@ -1,55 +1,61 @@
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
-from errors import MarionetteException
+from errors import MarionetteException, TimeoutException
 from functools import wraps
 import socket
 import sys
 import traceback
 
 
 def _find_marionette_in_args(*args, **kwargs):
     try:
         m = [a for a in args + tuple(kwargs.values()) if hasattr(a, 'session')][0]
     except IndexError:
         print("Can only apply decorator to function using a marionette object")
         raise
     return m
 
 
-def do_crash_check(func, always=False):
-    """Decorator which checks for crashes after the function has run.
+def do_process_check(func, always=False):
+    """Decorator which checks the process after the function has run.
+
+    There is a check for crashes which always gets executed. And in the case of
+    connection issues the process will be force closed.
 
     :param always: If False, only checks for crashes if an exception
                    was raised. If True, always checks for crashes.
     """
     @wraps(func)
     def _(*args, **kwargs):
-        def check():
-            m = _find_marionette_in_args(*args, **kwargs)
+        m = _find_marionette_in_args(*args, **kwargs)
+
+        def check_for_crash():
             try:
                 m.check_for_crash()
             except:
                 # don't want to lose the original exception
                 traceback.print_exc()
 
         try:
             return func(*args, **kwargs)
         except (MarionetteException, socket.error, IOError) as e:
             exc, val, tb = sys.exc_info()
             if not isinstance(e, MarionetteException) or type(e) is MarionetteException:
                 if not always:
-                    check()
+                    check_for_crash()
+            if not isinstance(e, MarionetteException) or type(e) is TimeoutException:
+                m.force_shutdown()
             raise exc, val, tb
         finally:
             if always:
-                check()
+                check_for_crash(m)
     return _
 
 
 def uses_marionette(func):
     """Decorator which creates a marionette session and deletes it
     afterwards if one doesn't already exist.
     """
     @wraps(func)
--- a/testing/marionette/client/marionette_driver/marionette.py
+++ b/testing/marionette/client/marionette_driver/marionette.py
@@ -2,22 +2,23 @@
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 import base64
 import ConfigParser
 import json
 import os
 import socket
+import sys
 import traceback
 import warnings
 
 from contextlib import contextmanager
 
-from decorators import do_crash_check
+from decorators import do_process_check
 from keys import Keys
 
 import geckoinstance
 import errors
 import transport
 
 WEBELEMENT_KEY = "ELEMENT"
 W3C_WEBELEMENT_KEY = "element-6066-11e4-a52e-4f735466cecf"
@@ -639,22 +640,22 @@ class Marionette(object):
             return False
         finally:
             s.close()
 
     def wait_for_port(self, timeout=None):
         timeout = timeout or self.DEFAULT_STARTUP_TIMEOUT
         return transport.wait_for_port(self.host, self.port, timeout=timeout)
 
-    @do_crash_check
+    @do_process_check
     def raise_for_port(self, port_obtained):
         if not port_obtained:
             raise IOError("Timed out waiting for port!")
 
-    @do_crash_check
+    @do_process_check
     def _send_message(self, name, params=None, key=None):
         """Send a blocking message to the server.
 
         Marionette provides an asynchronous, non-blocking interface and
         this attempts to paper over this by providing a synchronous API
         to the user.
 
         :param name: Requested command key.
@@ -674,24 +675,16 @@ class Marionette(object):
                 if params:
                     data["parameters"] = params
                 self.client.send(data)
                 msg = self.client.receive()
 
             else:
                 msg = self.client.request(name, params)
 
-        except IOError:
-            if self.instance:
-                # If we've launched the binary we've connected to, wait
-                # for it to shut down.
-                returncode = self.instance.runner.wait(timeout=self.DEFAULT_STARTUP_TIMEOUT)
-                raise IOError("process died with returncode %s" % returncode)
-            raise
-
         except socket.timeout:
             self.session = None
             self.window = None
             self.client.close()
             raise errors.TimeoutException("Connection timed out")
 
         res, err = msg.result, msg.error
         if err:
@@ -746,16 +739,38 @@ class Marionette(object):
             if self.instance.runner.check_for_crashes(
                     test_name=self.test_name):
                 crashed = True
         if returncode is not None:
             print ('PROCESS-CRASH | %s | abnormal termination with exit code %d' %
                    (name, returncode))
         return crashed
 
+    def force_shutdown(self):
+        """Force a shutdown of the running instance.
+
+        If we've launched the binary we are connected to, wait for it to shut down.
+        In the case when it doesn't happen, force its shut down.
+
+        """
+        if self.instance:
+            exc, val, tb = sys.exc_info()
+
+            returncode = self.instance.runner.returncode
+            if returncode is None:
+                self.instance.runner.stop()
+                message = 'Process killed because the connection was lost'
+            else:
+                message = 'Process died with returncode "{returncode}"'
+
+            if exc:
+                message += ' (Reason: {reason})'
+
+            raise exc, message.format(returncode=returncode, reason=val), tb
+
     @staticmethod
     def convert_keys(*string):
         typing = []
         for val in string:
             if isinstance(val, Keys):
                 typing.append(val)
             elif isinstance(val, int):
                 val = str(val)