Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Properly acquire local port after calculating it. #208

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
66 changes: 66 additions & 0 deletions test/test_multi_process.py
Original file line number Diff line number Diff line change
@@ -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)
30 changes: 13 additions & 17 deletions test/test_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thoughts on using a helper to create the AutomatorServer so each test doesn't have to specify local_port?

server.push = MagicMock()
server.push.return_value = ["bundle.jar", "uiautomator-stub.jar"]
server.ping = MagicMock()
Expand All @@ -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()
Expand All @@ -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())
Expand All @@ -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):
Expand All @@ -105,15 +101,15 @@ 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")

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)


Expand All @@ -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)
Expand All @@ -145,15 +141,15 @@ 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:
self.assertEqual(args[0][0], "push")
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
Expand All @@ -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()
Expand Down
47 changes: 35 additions & 12 deletions uiautomator/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -312,9 +312,13 @@ 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):

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good to add some test coverage here.

'''adb port forward. return 0 if success, else non-zero.'''
return self.cmd("forward", "tcp:%d" % local_port, "tcp:%d" % device_port).wait()
cmd = ["forward"]
if not rebind:
cmd.append("--no-rebind")
cmd += ["tcp:%d" % local_port, "tcp:%d" % device_port]
return self.cmd(*cmd).wait()

def forward_list(self):
'''adb forward --list'''
Expand Down Expand Up @@ -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__)
Expand All @@ -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():

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s, lp, rp could be named better. Also, I know this module doesn't have the best documentation, but I think a docstring would be good.

if s == self.adb.device_serial() and rp == 'tcp:%d' % self.device_port:
return int(lp[4:])
return None

@property
def local_port(self):

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here with the docstring.

# 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)))
Expand Down