Improve port picking logic. (#717)
* Do not check for adb ports if adb is not available.
* Replace the usage of the deprecated API.
diff --git a/mobly/controllers/android_device_lib/adb.py b/mobly/controllers/android_device_lib/adb.py
index 1814d19..11539fb 100644
--- a/mobly/controllers/android_device_lib/adb.py
+++ b/mobly/controllers/android_device_lib/adb.py
@@ -93,6 +93,19 @@
utils.cli_cmd_to_string(self.cmd), self.timeout)
+def is_adb_available():
+ """Checks if adb is available as a command line tool.
+
+ Returns:
+ True if adb binary is available in console, False otherwise.
+ """
+ ret, out, err = utils.run_command('which adb', shell=True)
+ clean_out = out.decode('utf-8').strip()
+ if clean_out:
+ return True
+ return False
+
+
def list_occupied_adb_ports():
"""Lists all the host ports occupied by adb forward.
diff --git a/mobly/utils.py b/mobly/utils.py
index 5d63c1d..5aca4a7 100644
--- a/mobly/utils.py
+++ b/mobly/utils.py
@@ -517,12 +517,15 @@
"""
# Only import adb module if needed.
from mobly.controllers.android_device_lib import adb
+ port = portpicker.pick_unused_port()
+ if not adb.is_adb_available():
+ return port
for _ in range(MAX_PORT_ALLOCATION_RETRY):
- port = portpicker.PickUnusedPort()
# Make sure adb is not using this port so we don't accidentally
# interrupt ongoing runs by trying to bind to the port.
if port not in adb.list_occupied_adb_ports():
return port
+ port = portpicker.pick_unused_port()
raise Error('Failed to find available port after {} retries'.format(
MAX_PORT_ALLOCATION_RETRY))
diff --git a/tests/mobly/controllers/android_device_lib/adb_test.py b/tests/mobly/controllers/android_device_lib/adb_test.py
index e97eaf2..099cff0 100755
--- a/tests/mobly/controllers/android_device_lib/adb_test.py
+++ b/tests/mobly/controllers/android_device_lib/adb_test.py
@@ -86,6 +86,17 @@
return mock_popen
@mock.patch('mobly.utils.run_command')
+ def test_is_adb_available(self, mock_run_command):
+ mock_run_command.return_value = (0, '/usr/local/bin/adb\n'.encode('utf-8'),
+ ''.encode('utf-8'))
+ self.assertTrue(adb.is_adb_available())
+
+ @mock.patch('mobly.utils.run_command')
+ def test_is_adb_available_negative(self, mock_run_command):
+ mock_run_command.return_value = (0, ''.encode('utf-8'), ''.encode('utf-8'))
+ self.assertFalse(adb.is_adb_available())
+
+ @mock.patch('mobly.utils.run_command')
def test_exec_cmd_no_timeout_success(self, mock_run_command):
mock_run_command.return_value = (0, MOCK_DEFAULT_STDOUT.encode('utf-8'),
MOCK_DEFAULT_STDERR.encode('utf-8'))
diff --git a/tests/mobly/utils_test.py b/tests/mobly/utils_test.py
index f047827..3b499a6 100755
--- a/tests/mobly/utils_test.py
+++ b/tests/mobly/utils_test.py
@@ -40,6 +40,7 @@
from tests.lib import multiple_subclasses_module
MOCK_AVAILABLE_PORT = 5
+ADB_MODULE_PACKAGE_NAME = 'mobly.controllers.android_device_lib.adb'
class UtilsTest(unittest.TestCase):
@@ -418,24 +419,34 @@
utils.create_dir(self.tmp_dir)
self.assertTrue(os.path.exists(self.tmp_dir))
- @mock.patch('mobly.controllers.android_device_lib.adb.list_occupied_adb_ports'
- )
- @mock.patch('portpicker.PickUnusedPort', return_value=MOCK_AVAILABLE_PORT)
- def test_get_available_port_positive(self, mock_list_occupied_adb_ports,
+ @mock.patch(f'{ADB_MODULE_PACKAGE_NAME}.is_adb_available', return_value=True)
+ @mock.patch(f'{ADB_MODULE_PACKAGE_NAME}.list_occupied_adb_ports')
+ @mock.patch('portpicker.pick_unused_port', return_value=MOCK_AVAILABLE_PORT)
+ def test_get_available_port_positive(self, mock_is_adb_available,
+ mock_list_occupied_adb_ports,
mock_pick_unused_port):
self.assertEqual(utils.get_available_host_port(), MOCK_AVAILABLE_PORT)
- @mock.patch(
- 'mobly.controllers.android_device_lib.adb.list_occupied_adb_ports',
- return_value=[MOCK_AVAILABLE_PORT])
- @mock.patch('portpicker.PickUnusedPort', return_value=MOCK_AVAILABLE_PORT)
- def test_get_available_port_negative(self, mock_list_occupied_adb_ports,
+ @mock.patch(f'{ADB_MODULE_PACKAGE_NAME}.is_adb_available', return_value=False)
+ @mock.patch(f'{ADB_MODULE_PACKAGE_NAME}.list_occupied_adb_ports')
+ @mock.patch('portpicker.pick_unused_port', return_value=MOCK_AVAILABLE_PORT)
+ def test_get_available_port_positive_no_adb(self, mock_is_adb_available,
+ mock_list_occupied_adb_ports,
+ mock_pick_unused_port):
+ self.assertEqual(utils.get_available_host_port(), MOCK_AVAILABLE_PORT)
+ mock_list_occupied_adb_ports.assert_not_called()
+
+ @mock.patch(f'{ADB_MODULE_PACKAGE_NAME}.is_adb_available', return_value=True)
+ @mock.patch(f'{ADB_MODULE_PACKAGE_NAME}.list_occupied_adb_ports',
+ return_value=[MOCK_AVAILABLE_PORT])
+ @mock.patch('portpicker.pick_unused_port', return_value=MOCK_AVAILABLE_PORT)
+ def test_get_available_port_negative(self, mock_is_adb_available,
+ mock_list_occupied_adb_ports,
mock_pick_unused_port):
with self.assertRaisesRegex(utils.Error, 'Failed to find.* retries'):
utils.get_available_host_port()
- @mock.patch('mobly.controllers.android_device_lib.adb.list_occupied_adb_ports'
- )
+ @mock.patch(f'{ADB_MODULE_PACKAGE_NAME}.list_occupied_adb_ports')
def test_get_available_port_returns_free_port(self,
mock_list_occupied_adb_ports):
"""Verifies logic to pick a free port on the host.
