From 9e1abdb98c8097d6416e64897e5b17eb2aaaebd7 Mon Sep 17 00:00:00 2001 From: Andreas Tolfsen Date: Mon, 9 Nov 2015 15:54:10 +0000 Subject: [PATCH] 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 --- .../marionette/tests/unit/test_emulator.py | 45 ++++++++++++------- .../driver/marionette_driver/marionette.py | 26 ++++++++++- testing/marionette/proxy.js | 2 +- 3 files changed, 56 insertions(+), 17 deletions(-) diff --git a/testing/marionette/client/marionette/tests/unit/test_emulator.py b/testing/marionette/client/marionette/tests/unit/test_emulator.py index 7a132a9ccdf..9514490527a 100644 --- a/testing/marionette/client/marionette/tests/unit/test_emulator.py +++ b/testing/marionette/client/marionette/tests/unit/test_emulator.py @@ -12,30 +12,27 @@ class TestEmulatorContent(MarionetteTestCase): @skip_if_desktop def test_emulator_cmd(self): self.marionette.set_script_timeout(10000) - expected = ["", - "OK"] - result = self.marionette.execute_async_script(""" - runEmulatorCmd("avd name", marionetteScriptFinished) - """); - self.assertEqual(result, expected) + expected = ["", "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): @@ -132,6 +129,24 @@ class TestEmulatorCallbacks(MarionetteTestCase): 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 diff --git a/testing/marionette/driver/marionette_driver/marionette.py b/testing/marionette/driver/marionette_driver/marionette.py index 8d3e0216137..2becbc3c17e 100644 --- a/testing/marionette/driver/marionette_driver/marionette.py +++ b/testing/marionette/driver/marionette_driver/marionette.py @@ -671,6 +671,27 @@ class Marionette(object): @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") @@ -692,13 +713,16 @@ class Marionette(object): 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"]) diff --git a/testing/marionette/proxy.js b/testing/marionette/proxy.js index a36f8bc087a..f0464a26c79 100644 --- a/testing/marionette/proxy.js +++ b/testing/marionette/proxy.js @@ -114,7 +114,7 @@ ContentSender.prototype.send = function(name, args) { 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; }