Bug 1223028: Exhaust server-originated commands
☠☠ backed out by 4f00eab8d1c6 ☠ ☠
authorAndreas Tolfsen <ato@mozilla.com>
Mon, 09 Nov 2015 15:54:10 +0000
changeset 271956 a3f9ac7c8454439cbd17ba3c4cf08bf5da33484a
parent 271955 de2c852c98532ff6bf71a6e29746ee208765d9d3
child 271957 72f55d458dd63051f804e016b94cc1dfcca9bdcb
push id67821
push useratolfsen@mozilla.com
push dateTue, 10 Nov 2015 20:44:13 +0000
treeherdermozilla-inbound@a3f9ac7c8454 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1223028, 1211503
milestone45.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1223028: Exhaust server-originated commands The Python client does not currently exhaust all the command requests originating from the server as pointed out by :bhsu in https://bugzilla.mozilla.org/show_bug.cgi?id=1211503#c12. This prevents making multiple calls to runEmulatorCmd and runEmulatorShell inside executeScript/executeJSScript/executeAsyncScript. This failure makes marionette-webapi fail. We loop through all the commands originating from the server until we get sent back a response. r=dburns
testing/marionette/client/marionette/tests/unit/test_emulator.py
testing/marionette/driver/marionette_driver/marionette.py
testing/marionette/proxy.js
--- a/testing/marionette/client/marionette/tests/unit/test_emulator.py
+++ b/testing/marionette/client/marionette/tests/unit/test_emulator.py
@@ -7,40 +7,37 @@ from unittest import skip
 from marionette.marionette_test import MarionetteTestCase, skip_if_desktop, skip_unless_protocol
 from marionette_driver.errors import MarionetteException, JavascriptException
 
 
 class TestEmulatorContent(MarionetteTestCase):
     @skip_if_desktop
     def test_emulator_cmd(self):
         self.marionette.set_script_timeout(10000)
-        expected = ["<build>",
-                    "OK"]
-        result = self.marionette.execute_async_script("""
-        runEmulatorCmd("avd name", marionetteScriptFinished)
-        """);
-        self.assertEqual(result, expected)
+        expected = ["<build>", "OK"]
+        res = self.marionette.execute_async_script(
+            "runEmulatorCmd('avd name', marionetteScriptFinished)");
+        self.assertEqual(res, expected)
 
     @skip_if_desktop
     def test_emulator_shell(self):
         self.marionette.set_script_timeout(10000)
         expected = ["Hello World!"]
-        result = self.marionette.execute_async_script("""
-        runEmulatorShell(["echo", "Hello World!"], marionetteScriptFinished)
-        """);
-        self.assertEqual(result, expected)
+        res = self.marionette.execute_async_script(
+            "runEmulatorShell(['echo', 'Hello World!'], marionetteScriptFinished)")
+        self.assertEqual(res, expected)
 
     @skip_if_desktop
     def test_emulator_order(self):
         self.marionette.set_script_timeout(10000)
-        self.assertRaises(MarionetteException,
-                          self.marionette.execute_async_script,
-        """runEmulatorCmd("gsm status", function(result) {});
-           marionetteScriptFinished(true);
-        """);
+        with self.assertRaises(MarionetteException):
+            self.marionette.execute_async_script("""
+               runEmulatorCmd("gsm status", function(res) {});
+               marionetteScriptFinished(true);
+               """)
 
 
 class TestEmulatorChrome(TestEmulatorContent):
     def setUp(self):
         super(TestEmulatorChrome, self).setUp()
         self.marionette.set_context("chrome")
 
 
@@ -127,11 +124,29 @@ class TestEmulatorCallbacks(MarionetteTe
             self.assertEqual("shell response", res)
 
     @skip_unless_protocol(lambda level: level >= 3)
     def test_emulator_result_error_chrome(self):
         with self.marionette.using_context("chrome"):
             with self.assertRaisesRegexp(JavascriptException, "TypeError"):
                 self.marionette.execute_async_script("runEmulatorCmd()")
 
+    def test_multiple_callbacks(self):
+        res = self.marionette.execute_async_script("""
+            runEmulatorCmd("what");
+            runEmulatorCmd("ho");
+            marionetteScriptFinished("Frobisher");
+            """)
+        self.assertEqual("Frobisher", res)
+
+    # This should work, but requires work on emulator callbacks:
+    """
+    def test_multiple_nested_callbacks(self):
+        res = self.marionette.execute_async_script('''
+            runEmulatorCmd("what", function(res) {
+              runEmulatorCmd("ho", marionetteScriptFinished);
+            });''')
+        self.assertEqual("cmd response", res)
+    """
+
 
 def escape(word):
     return "'%s'" % word
--- a/testing/marionette/driver/marionette_driver/marionette.py
+++ b/testing/marionette/driver/marionette_driver/marionette.py
@@ -666,16 +666,37 @@ class Marionette(object):
         finally:
             s.close()
 
     def wait_for_port(self, timeout=60):
         return transport.wait_for_port(self.host, self.port, timeout=timeout)
 
     @do_crash_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.
+
+        In particular, the Python client can be instructed to carry out
+        a sequence of instructions on the connected emulator.  For this
+        reason, if ``execute_script``, ``execute_js_script``, or
+        ``execute_async_script`` is called, it will loop until all
+        commands requested from the server have been exhausted, and we
+        receive our expected response.
+
+        :param name: Requested command key.
+        :param params: Optional dictionary of key/value arguments.
+        :param key: Optional key to extract from response.
+
+        :returns: Full response from the server, or if `key` is given,
+            the value of said key in the response.
+        """
+
         if not self.session_id and name != "newSession":
             raise errors.MarionetteException("Please start a session")
 
         try:
             if self.protocol < 3:
                 data = {"name": name}
                 if params:
                     data["parameters"] = params
@@ -687,23 +708,26 @@ class Marionette(object):
 
         except IOError:
             if self.instance and not hasattr(self.instance, 'detached'):
                 # 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")
 
-        if isinstance(msg, transport.Command):
+        # support execution of commands on the client,
+        # loop until we receive our expected response
+        while isinstance(msg, transport.Command):
             if msg.name == "runEmulatorCmd":
                 self.emulator_callback_id = msg.params.get("id")
                 msg = self._emulator_cmd(msg.params["emulator_cmd"])
             elif msg.name == "runEmulatorShell":
                 self.emulator_callback_id = msg.params.get("id")
                 msg = self._emulator_shell(msg.params["emulator_shell"])
             else:
                 raise IOError("Unknown command: %s" % msg)
--- a/testing/marionette/proxy.js
+++ b/testing/marionette/proxy.js
@@ -109,17 +109,17 @@ ContentSender.prototype.send = function(
 
   this.curId = uuidgen.generateUUID().toString();
 
   let proxy = new Promise((resolve, reject) => {
     let removeListeners = (n, fn) => {
       let rmFn = msg => {
         if (this.curId !== msg.json.command_id) {
           logger.warn("Skipping out-of-sync response from listener: " +
-              `Expected response to \`${name}' with ID ${this.curId}, ` +
+              `Expected response to ${name} with ID ${this.curId}, ` +
               "but got: " + msg.name + msg.json.toSource());
           return;
         }
 
         this.removeListeners();
         modal.removeHandler(handleDialog);
 
         fn(msg);