From 569b03fdde5b5b01828477aee63639f0df09cb71 Mon Sep 17 00:00:00 2001 From: Artem Iglikov Date: Sat, 25 Mar 2017 18:04:40 -0700 Subject: [PATCH 1/4] Properly acquire local port after calculating it. This makes the AutomatorServer to properly acquire the local port chosen for forwarding traffic to the server running on the device, so that other instances of uiautomator-based scripts don't acquire it before the current instance. --- README.md | 2 ++ test/test_multi_process.py | 66 ++++++++++++++++++++++++++++++++++++++ test/test_server.py | 30 ++++++++--------- uiautomator/__init__.py | 45 +++++++++++++++++++------- 4 files changed, 115 insertions(+), 28 deletions(-) create mode 100644 test/test_multi_process.py diff --git a/README.md b/README.md index 93dae72..a2931f9 100644 --- a/README.md +++ b/README.md @@ -704,12 +704,14 @@ Selector supports below parameters. Refer to [UiSelector java doc](http://develo - Qian Jin ([@QianJin2013][]) - Xu Jingjie ([@xiscoxu][]) - Xia Mingyuan ([@mingyuan-xia][]) +- Artem Iglikov, Google Inc. ([@artikz][]) [@xiaocong]: https://github.com/xiaocong [@yuanyuan]: https://github.com/yuanyuanzou [@QianJin2013]: https://github.com/QianJin2013 [@xiscoxu]: https://github.com/xiscoxu [@mingyuan-xia]: https://github.com/mingyuan-xia +[@artikz]: https://github.com/artikz ## Issues & Discussion diff --git a/test/test_multi_process.py b/test/test_multi_process.py new file mode 100644 index 0000000..e0e7ffa --- /dev/null +++ b/test/test_multi_process.py @@ -0,0 +1,66 @@ +#!/usr/bin/env python +# -*- coding: utf-8 -*- + +import unittest +import logging +import mock +import os +import subprocess +import uiautomator +import multiprocessing + + +def _create_next_local_port_stub(ports): + def _next_local_port_stub(_): + max_used_port = max(x[1] for x in ports) if ports else 0 + return max(max_used_port, uiautomator.LOCAL_PORT) + 1 + return _next_local_port_stub + + +def _create_adb_forward_stub(serial, ports): + def _adb_forward_stub(local_port, device_port, rebind=True): + ports.append([serial, local_port, device_port]) + return _adb_forward_stub + + +def _create_adb_forward_list_stub(ports): + def _adb_forward_list_stub(): + return [[x[0], "tcp:" + str(x[1]), "tcp:" + str(x[2])] for x in ports] + return _adb_forward_list_stub + + +class TestMultiProcess(unittest.TestCase): + + def setUp(self): + self.ports = [] + + def create_device(self, serial): + device = uiautomator.Device(serial=serial) + device.server.adb = mock.MagicMock() + device.server.adb.device_serial = lambda: serial + device.server.adb.forward = _create_adb_forward_stub(serial, self.ports) + device.server.adb.forward_list = _create_adb_forward_list_stub(self.ports) + device.server.ping = mock.MagicMock(return_value="pong") + return device + + def test_run_sequential(self): + uiautomator.next_local_port = _create_next_local_port_stub(self.ports) + + device1 = self.create_device("1") + device1.server.start() + + device2 = self.create_device("2") + device2.server.start() + + self.assertNotEqual(device1.server.local_port, device2.server.local_port) + + def test_run_interleaving(self): + uiautomator.next_local_port = _create_next_local_port_stub(self.ports) + + device1 = self.create_device("1") + device2 = self.create_device("2") + + device1.server.start() + device2.server.start() + + self.assertNotEqual(device1.server.local_port, device2.server.local_port) diff --git a/test/test_server.py b/test/test_server.py index e5e7169..7b755b7 100644 --- a/test/test_server.py +++ b/test/test_server.py @@ -29,21 +29,17 @@ def test_local_port_forwarded(self): def test_local_port_scanning(self): with patch('uiautomator.next_local_port') as next_local_port: - self.Adb.return_value.forward_list.return_value = [] + self.Adb.return_value.device_serial.return_value = "abcd" + self.Adb.return_value.forward_list.side_effect = [[], [["abcd", "tcp:1234", "tcp:9008"]]] next_local_port.return_value = 1234 self.assertEqual(AutomatorServer("abcd", None).local_port, next_local_port.return_value) - next_local_port.return_value = 14321 - self.Adb.return_value.forward_list.return_value = Exception("error") - self.assertEqual(AutomatorServer("abcd", None).local_port, - next_local_port.return_value) - def test_device_port(self): self.assertEqual(AutomatorServer().device_port, 9008) def test_start_success(self): - server = AutomatorServer() + server = AutomatorServer(local_port=1234) server.push = MagicMock() server.push.return_value = ["bundle.jar", "uiautomator-stub.jar"] server.ping = MagicMock() @@ -53,7 +49,7 @@ def test_start_success(self): server.adb.cmd.assert_valled_onec_with('shell', 'uiautomator', 'runtest', 'bundle.jar', 'uiautomator-stub.jar', '-c', 'com.github.uiautomatorstub.Stub') def test_start_error(self): - server = AutomatorServer() + server = AutomatorServer(local_port=1234) server.push = MagicMock() server.push.return_value = ["bundle.jar", "uiautomator-stub.jar"] server.ping = MagicMock() @@ -76,7 +72,7 @@ def side_effect(): raise result return result JsonRPCMethod.return_value.side_effect = side_effect - server = AutomatorServer() + server = AutomatorServer(local_port=1234) server.start = MagicMock() server.stop = MagicMock() self.assertEqual("ok", server.jsonrpc.any_method()) @@ -89,14 +85,14 @@ def side_effect(): raise result return result JsonRPCMethod.return_value.side_effect = side_effect - server = AutomatorServer() + server = AutomatorServer(local_port=1234) server.start = MagicMock() server.stop = MagicMock() self.assertEqual("ok", server.jsonrpc.any_method()) server.start.assert_called_once_with() with patch("uiautomator.JsonRPCMethod") as JsonRPCMethod: JsonRPCMethod.return_value.side_effect = JsonRPCError(-32000-2, "error msg") - server = AutomatorServer() + server = AutomatorServer(local_port=1234) server.start = MagicMock() server.stop = MagicMock() with self.assertRaises(JsonRPCError): @@ -105,7 +101,7 @@ def side_effect(): def test_start_ping(self): with patch("uiautomator.JsonRPCClient") as JsonRPCClient: JsonRPCClient.return_value.ping.return_value = "pong" - server = AutomatorServer() + server = AutomatorServer(local_port=1234) server.adb = MagicMock() server.adb.forward.return_value = 0 self.assertEqual(server.ping(), "pong") @@ -113,7 +109,7 @@ def test_start_ping(self): def test_start_ping_none(self): with patch("uiautomator.JsonRPCClient") as JsonRPCClient: JsonRPCClient.return_value.ping.side_effect = Exception("error") - server = AutomatorServer() + server = AutomatorServer(local_port=1234) self.assertEqual(server.ping(), None) @@ -132,7 +128,7 @@ def tearDown(self): self.urlopen_patch.stop() def test_screenshot(self): - server = AutomatorServer() + server = AutomatorServer(local_port=1234) server.sdk_version = MagicMock() server.sdk_version.return_value = 17 self.assertEqual(server.screenshot(), None) @@ -145,7 +141,7 @@ def test_screenshot(self): def test_push(self): jars = ["bundle.jar", "uiautomator-stub.jar"] - server = AutomatorServer() + server = AutomatorServer(local_port=1234) server.adb = MagicMock() self.assertEqual(set(server.push()), set(jars)) for args in server.adb.cmd.call_args_list: @@ -153,7 +149,7 @@ def test_push(self): self.assertEqual(args[0][2], "/data/local/tmp/") def test_stop_started_server(self): - server = AutomatorServer() + server = AutomatorServer(local_port=1234) server.adb = MagicMock() server.uiautomator_process = process = MagicMock() process.poll.return_value = None @@ -174,7 +170,7 @@ def test_stop(self): b"USER PID PPID VSIZE RSS WCHAN PC NAME\rsystem 372 126 635596 104808 ffffffff 00000000 S uiautomator" ] for r in results: - server = AutomatorServer() + server = AutomatorServer(local_port=1234) server.adb = MagicMock() server.adb.cmd.return_value.communicate.return_value = (r, "") server.stop() diff --git a/uiautomator/__init__.py b/uiautomator/__init__.py index 677b0b7..13f8238 100644 --- a/uiautomator/__init__.py +++ b/uiautomator/__init__.py @@ -312,8 +312,12 @@ def devices(self): raise EnvironmentError("adb is not working.") return dict([s.split("\t") for s in out[index + len(match):].strip().splitlines() if s.strip()]) - def forward(self, local_port, device_port): + def forward(self, local_port, device_port, rebind=True): '''adb port forward. return 0 if success, else non-zero.''' + cmd = ["forward"] + if not rebind: + cmd += "--no-rebind" + cmd += ["tcp:%d" % local_port, "tcp:%d" % device_port] return self.cmd("forward", "tcp:%d" % local_port, "tcp:%d" % device_port).wait() def forward_list(self): @@ -380,17 +384,11 @@ def __init__(self, serial=None, local_port=None, device_port=None, adb_server_ho self.adb = Adb(serial=serial, adb_server_host=adb_server_host, adb_server_port=adb_server_port) self.device_port = int(device_port) if device_port else DEVICE_PORT if local_port: - self.local_port = local_port + # Assume that the caller acquired the port correctly. + self.__local_port = local_port else: - try: # first we will try to use the local port already adb forwarded - for s, lp, rp in self.adb.forward_list(): - if s == self.adb.device_serial() and rp == 'tcp:%d' % self.device_port: - self.local_port = int(lp[4:]) - break - else: - self.local_port = next_local_port(adb_server_host) - except: - self.local_port = next_local_port(adb_server_host) + # Port will be assigned later when communication actually starts. + self.__local_port = None def push(self): base_dir = os.path.dirname(__file__) @@ -404,6 +402,31 @@ def install(self): for apk in self.__apk_files: self.adb.cmd("install", "-r -t", os.path.join(base_dir, apk)).wait() + def get_forwarded_port(self): + for s, lp, rp in self.adb.forward_list(): + if s == self.adb.device_serial() and rp == 'tcp:%d' % self.device_port: + return int(lp[4:]) + return None + + @property + def local_port(self): + # If the port was already assigned, just return it. + if self.__local_port: + return self.__local_port + + # Otherwise, find and acquire an available port. + while True: + # First, check whether there is an already set up port. + forwarded_port = self.get_forwarded_port() + if forwarded_port: + self.__local_port = forwarded_port + return self.__local_port + + # If port is not set up yet, try to set it up. + port = next_local_port(self.adb.adb_server_host) + # Try to acquire the port, so that other processes don't take it. + self.adb.forward(port, self.device_port, rebind=False) + @property def jsonrpc(self): return self.jsonrpc_wrap(timeout=int(os.environ.get("jsonrpc_timeout", 90))) From 95e62aa41163a607d1bcd8ccd570798cb88ba329 Mon Sep 17 00:00:00 2001 From: Artem Iglikov Date: Mon, 27 Mar 2017 22:51:52 +0100 Subject: [PATCH 2/4] Really use in Adb.forward() --- uiautomator/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/uiautomator/__init__.py b/uiautomator/__init__.py index 13f8238..62f0efd 100644 --- a/uiautomator/__init__.py +++ b/uiautomator/__init__.py @@ -318,7 +318,7 @@ def forward(self, local_port, device_port, rebind=True): if not rebind: cmd += "--no-rebind" cmd += ["tcp:%d" % local_port, "tcp:%d" % device_port] - return self.cmd("forward", "tcp:%d" % local_port, "tcp:%d" % device_port).wait() + return self.cmd(*cmd).wait() def forward_list(self): '''adb forward --list''' From 69fde90bb54d325654eedca239b06c8993131fbd Mon Sep 17 00:00:00 2001 From: Artem Iglikov Date: Wed, 29 Mar 2017 11:17:16 +0100 Subject: [PATCH 3/4] Fix typo --- uiautomator/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/uiautomator/__init__.py b/uiautomator/__init__.py index 62f0efd..1f65063 100644 --- a/uiautomator/__init__.py +++ b/uiautomator/__init__.py @@ -316,7 +316,7 @@ def forward(self, local_port, device_port, rebind=True): '''adb port forward. return 0 if success, else non-zero.''' cmd = ["forward"] if not rebind: - cmd += "--no-rebind" + cmd.append("--no-rebind") cmd += ["tcp:%d" % local_port, "tcp:%d" % device_port] return self.cmd(*cmd).wait() From 4fdbb323a60dea91c69af49a3ee5bc1c686fce6a Mon Sep 17 00:00:00 2001 From: Artem Iglikov Date: Sun, 21 May 2017 17:54:20 +0100 Subject: [PATCH 4/4] Add some test coverage, docstrings, clean up tests implementation.. --- test/test_adb.py | 14 ++++++++++++++ test/test_server.py | 26 +++++++++++++++----------- uiautomator/__init__.py | 22 +++++++++++++++++++--- 3 files changed, 48 insertions(+), 14 deletions(-) diff --git a/test/test_adb.py b/test/test_adb.py index ac97b9d..a645007 100644 --- a/test/test_adb.py +++ b/test/test_adb.py @@ -89,6 +89,20 @@ def test_forward(self): adb.cmd.assert_called_once_with("forward", "tcp:90", "tcp:91") adb.cmd.return_value.wait.assert_called_once_with() + def test_forward_rebind_false(self): + adb = Adb() + adb.cmd = MagicMock() + adb.forward(90, 91, rebind=False) + adb.cmd.assert_called_once_with("forward", "--no-rebind", "tcp:90", "tcp:91") + adb.cmd.return_value.wait.assert_called_once_with() + + def test_forward_rebind_true(self): + adb = Adb() + adb.cmd = MagicMock() + adb.forward(90, 91, rebind=True) + adb.cmd.assert_called_once_with("forward", "tcp:90", "tcp:91") + adb.cmd.return_value.wait.assert_called_once_with() + def test_adb_raw_cmd(self): import subprocess adb = Adb() diff --git a/test/test_server.py b/test/test_server.py index 7b755b7..216f934 100644 --- a/test/test_server.py +++ b/test/test_server.py @@ -6,6 +6,10 @@ from uiautomator import AutomatorServer, JsonRPCError +def _create_server_for_test(): + return AutomatorServer(local_port=1234) + + class TestAutomatorServer(unittest.TestCase): def setUp(self): @@ -39,7 +43,7 @@ def test_device_port(self): self.assertEqual(AutomatorServer().device_port, 9008) def test_start_success(self): - server = AutomatorServer(local_port=1234) + server = _create_server_for_test() server.push = MagicMock() server.push.return_value = ["bundle.jar", "uiautomator-stub.jar"] server.ping = MagicMock() @@ -49,7 +53,7 @@ def test_start_success(self): server.adb.cmd.assert_valled_onec_with('shell', 'uiautomator', 'runtest', 'bundle.jar', 'uiautomator-stub.jar', '-c', 'com.github.uiautomatorstub.Stub') def test_start_error(self): - server = AutomatorServer(local_port=1234) + server = _create_server_for_test() server.push = MagicMock() server.push.return_value = ["bundle.jar", "uiautomator-stub.jar"] server.ping = MagicMock() @@ -72,7 +76,7 @@ def side_effect(): raise result return result JsonRPCMethod.return_value.side_effect = side_effect - server = AutomatorServer(local_port=1234) + server = _create_server_for_test() server.start = MagicMock() server.stop = MagicMock() self.assertEqual("ok", server.jsonrpc.any_method()) @@ -85,14 +89,14 @@ def side_effect(): raise result return result JsonRPCMethod.return_value.side_effect = side_effect - server = AutomatorServer(local_port=1234) + server = _create_server_for_test() server.start = MagicMock() server.stop = MagicMock() self.assertEqual("ok", server.jsonrpc.any_method()) server.start.assert_called_once_with() with patch("uiautomator.JsonRPCMethod") as JsonRPCMethod: JsonRPCMethod.return_value.side_effect = JsonRPCError(-32000-2, "error msg") - server = AutomatorServer(local_port=1234) + server = _create_server_for_test() server.start = MagicMock() server.stop = MagicMock() with self.assertRaises(JsonRPCError): @@ -101,7 +105,7 @@ def side_effect(): def test_start_ping(self): with patch("uiautomator.JsonRPCClient") as JsonRPCClient: JsonRPCClient.return_value.ping.return_value = "pong" - server = AutomatorServer(local_port=1234) + server = _create_server_for_test() server.adb = MagicMock() server.adb.forward.return_value = 0 self.assertEqual(server.ping(), "pong") @@ -109,7 +113,7 @@ def test_start_ping(self): def test_start_ping_none(self): with patch("uiautomator.JsonRPCClient") as JsonRPCClient: JsonRPCClient.return_value.ping.side_effect = Exception("error") - server = AutomatorServer(local_port=1234) + server = _create_server_for_test() self.assertEqual(server.ping(), None) @@ -128,7 +132,7 @@ def tearDown(self): self.urlopen_patch.stop() def test_screenshot(self): - server = AutomatorServer(local_port=1234) + server = _create_server_for_test() server.sdk_version = MagicMock() server.sdk_version.return_value = 17 self.assertEqual(server.screenshot(), None) @@ -141,7 +145,7 @@ def test_screenshot(self): def test_push(self): jars = ["bundle.jar", "uiautomator-stub.jar"] - server = AutomatorServer(local_port=1234) + server = _create_server_for_test() server.adb = MagicMock() self.assertEqual(set(server.push()), set(jars)) for args in server.adb.cmd.call_args_list: @@ -149,7 +153,7 @@ def test_push(self): self.assertEqual(args[0][2], "/data/local/tmp/") def test_stop_started_server(self): - server = AutomatorServer(local_port=1234) + server = _create_server_for_test() server.adb = MagicMock() server.uiautomator_process = process = MagicMock() process.poll.return_value = None @@ -170,7 +174,7 @@ def test_stop(self): b"USER PID PPID VSIZE RSS WCHAN PC NAME\rsystem 372 126 635596 104808 ffffffff 00000000 S uiautomator" ] for r in results: - server = AutomatorServer(local_port=1234) + server = _create_server_for_test() server.adb = MagicMock() server.adb.cmd.return_value.communicate.return_value = (r, "") server.stop() diff --git a/uiautomator/__init__.py b/uiautomator/__init__.py index 1f65063..8e0e001 100644 --- a/uiautomator/__init__.py +++ b/uiautomator/__init__.py @@ -403,13 +403,29 @@ def install(self): self.adb.cmd("install", "-r -t", os.path.join(base_dir, apk)).wait() def get_forwarded_port(self): - for s, lp, rp in self.adb.forward_list(): - if s == self.adb.device_serial() and rp == 'tcp:%d' % self.device_port: - return int(lp[4:]) + '''Returns local port used for forwarding with current device and port. + + Returns: + int, local port set up for forwarding with current device and port + None, if no forwarding was set up with current device and port. + ''' + for serial, local_port, remote_port in self.adb.forward_list(): + if (serial == self.adb.device_serial() and + remote_port == 'tcp:%d' % self.device_port): + return int(local_port[4:]) return None @property def local_port(self): + '''Finds a free local port and acquires it for use with adb forwarding. + + Will try to find a free local port and set up adb port forwarding with + it. If forwarding fails, will try next free local port. + + Returns: + int, local port used for adb forwarding. + ''' + # If the port was already assigned, just return it. if self.__local_port: return self.__local_port