Merge d47006c into e83bcba

.pylintrc

@@ -142,7 +142,6 @@ disable=print-statement,
         line-too-long,
         duplicate-code,
         fixme,
-        lost-exception,
         too-many-arguments,
         too-many-branches,
         too-many-instance-attributes,

androidtv/adb_manager.py

@@ -84,8 +84,9 @@ def connect(self, always_log_errors=True, auth_timeout_s=DEFAULT_AUTH_TIMEOUT_S)
         if self._adb_lock.acquire(**LOCK_KWARGS):  # pylint: disable=unexpected-keyword-arg
             # Make sure that we release the lock
             try:
-                # Catch errors
+                # Catch exceptions
                 try:
+                    # Connect with authentication
                     if self.adbkey:
                         # private key
                         with open(self.adbkey) as f:
@@ -100,14 +101,16 @@ def connect(self, always_log_errors=True, auth_timeout_s=DEFAULT_AUTH_TIMEOUT_S)
 
                         signer = PythonRSASigner(pub, priv)
 
-                        # Connect to the device
                         self._adb.connect(rsa_keys=[signer], auth_timeout_s=auth_timeout_s)
+
+                    # Connect without authentication
                     else:
                         self._adb.connect(auth_timeout_s=auth_timeout_s)
 
                     # ADB connection successfully established
-                    self._available = True
                     _LOGGER.debug("ADB connection to %s successfully established", self.host)
+                    self._available = True
+                    return True
 
                 except OSError as exc:
                     if self._available or always_log_errors:
@@ -116,24 +119,25 @@ def connect(self, always_log_errors=True, auth_timeout_s=DEFAULT_AUTH_TIMEOUT_S)
                         _LOGGER.warning("Couldn't connect to host %s.  %s: %s", self.host, exc.__class__.__name__, exc.strerror)
 
                     # ADB connection attempt failed
-                    self._adb.close()
+                    self.close()
                     self._available = False
+                    return False
 
                 except Exception as exc:  # pylint: disable=broad-except
                     if self._available or always_log_errors:
                         _LOGGER.warning("Couldn't connect to host %s.  %s: %s", self.host, exc.__class__.__name__, exc)
 
                     # ADB connection attempt failed
-                    self._adb.close()
+                    self.close()
                     self._available = False
-
-                finally:
-                    return self._available
+                    return False
 
             finally:
                 self._adb_lock.release()
 
         # Lock could not be acquired
+        _LOGGER.warning("Couldn't connect to host %s because adb-shell lock not acquired.", self.host)
+        self.close()
         self._available = False
         return False
 
@@ -163,7 +167,7 @@ def shell(self, cmd):
                 self._adb_lock.release()
 
         # Lock could not be acquired
-        _LOGGER.debug("ADB command not sent to %s because adb-shell lock not acquired: %s", self.host, cmd)
+        _LOGGER.warning("ADB command not sent to %s because adb-shell lock not acquired: %s", self.host, cmd)
         return None
 
 
@@ -238,6 +242,12 @@ def available(self):
                 self._available = False
             return False
 
+        except Exception as exc:  # noqa pylint: disable=broad-except
+            if self._available:
+                _LOGGER.error('ADB server is unavailable, error: %s', exc)
+                self._available = False
+            return False
+
     def close(self):
         """Close the ADB server socket connection.
 
@@ -262,6 +272,7 @@ def connect(self, always_log_errors=True):
         if self._adb_lock.acquire(**LOCK_KWARGS):  # pylint: disable=unexpected-keyword-arg
             # Make sure that we release the lock
             try:
+                # Catch exceptions
                 try:
                     self._adb_client = Client(host=self.adb_server_ip, port=self.adb_server_port)
                     self._adb_device = self._adb_client.device(self.host)
@@ -270,27 +281,31 @@ def connect(self, always_log_errors=True):
                     if self._adb_device:
                         _LOGGER.debug("ADB connection to %s via ADB server %s:%s successfully established", self.host, self.adb_server_ip, self.adb_server_port)
                         self._available = True
+                        return True
 
                     # ADB connection attempt failed (without an exception)
-                    else:
-                        if self._available or always_log_errors:
-                            _LOGGER.warning("Couldn't connect to host %s via ADB server %s:%s", self.host, self.adb_server_ip, self.adb_server_port)
-                        self._available = False
+                    if self._available or always_log_errors:
+                        _LOGGER.warning("Couldn't connect to host %s via ADB server %s:%s", self.host, self.adb_server_ip, self.adb_server_port)
 
+                    self.close()
+                    self._available = False
+                    return False
+
+                # ADB connection attempt failed
                 except Exception as exc:  # noqa pylint: disable=broad-except
                     if self._available or always_log_errors:
                         _LOGGER.warning("Couldn't connect to host %s via ADB server %s:%s, error: %s", self.host, self.adb_server_ip, self.adb_server_port, exc)
 
-                    # ADB connection attempt failed
+                    self.close()
                     self._available = False
-
-                finally:
-                    return self._available
+                    return False
 
             finally:
                 self._adb_lock.release()
 
         # Lock could not be acquired
+        _LOGGER.warning("Couldn't connect to host %s via ADB server %s:%s because pure-python-adb lock not acquired.", self.host, self.adb_server_ip, self.adb_server_port)
+        self.close()
         self._available = False
         return False
 
@@ -320,5 +335,5 @@ def shell(self, cmd):
                 self._adb_lock.release()
 
         # Lock could not be acquired
-        _LOGGER.debug("ADB command not sent to %s via ADB server %s:%s because pure-python-adb lock not acquired: %s", self.host, self.adb_server_ip, self.adb_server_port, cmd)
+        _LOGGER.warning("ADB command not sent to %s via ADB server %s:%s because pure-python-adb lock not acquired: %s", self.host, self.adb_server_ip, self.adb_server_port, cmd)
         return None

tests/patchers.py

@@ -1,3 +1,5 @@
+"""Define patches used for androidtv tests."""
+
 try:
     # Python3
     from unittest.mock import patch
@@ -8,7 +10,13 @@
 
 class AdbDeviceFake(object):
     """A fake of the `adb_shell.adb_device.AdbDevice` class."""
+
     def __init__(self, *args, **kwargs):
+        """Initialize a fake `adb_shell.adb_device.AdbDevice` instance."""
+        self.available = False
+
+    def close(self):
+        """Close the socket connection."""
         self.available = False
 
     def connect(self, *args, **kwargs):
@@ -19,15 +27,12 @@ def shell(self, cmd):
         """Send an ADB shell command."""
         return None
 
-    def close(self):
-        """Close the socket connection."""
-        self.available = False
-
 
 class ClientFakeSuccess(object):
     """A fake of the `ppadb.client.Client` class when the connection and shell commands succeed."""
 
-    def __init__(self, host='127.0.0.1', port=5037):
+    def __init__(self, host="127.0.0.1", port=5037):
+        """Initialize a `ClientFakeSuccess` instance."""
         self._devices = []
 
     def devices(self):
@@ -44,7 +49,8 @@ def device(self, serial):
 class ClientFakeFail(object):
     """A fake of the `ppadb.client.Client` class when the connection and shell commands fail."""
 
-    def __init__(self, host='127.0.0.1', port=5037):
+    def __init__(self, host="127.0.0.1", port=5037):
+        """Initialize a `ClientFakeFail` instance."""
         self._devices = []
 
     def devices(self):
@@ -54,13 +60,13 @@ def devices(self):
     def device(self, serial):
         """Mock the `Client.device` method when the device is not connected via ADB."""
         self._devices = []
-        return None
 
 
 class DeviceFake(object):
     """A fake of the `ppadb.device.Device` class."""
 
     def __init__(self, host):
+        """Initialize a `DeviceFake` instance."""
         self.host = host
 
     def get_serial_no(self):
@@ -73,7 +79,7 @@ def shell(self, cmd):
 
 
 def patch_connect(success):
-    """Mock the `adb_shell.adb_device.AdbDevice` and `adb_messenger.client.Client` classes."""
+    """Mock the `adb_shell.adb_device.AdbDevice` and `ppadb.client.Client` classes."""
 
     def connect_success_python(self, *args, **kwargs):
         """Mock the `AdbDeviceFake.connect` method when it succeeds."""
@@ -84,11 +90,8 @@ def connect_fail_python(self, *args, **kwargs):
         raise OSError
 
     if success:
-        return {'python': patch('{}.AdbDeviceFake.connect'.format(__name__), connect_success_python), 'server': patch('androidtv.adb_manager.Client', ClientFakeSuccess)}
-    return {'python': patch('{}.AdbDeviceFake.connect'.format(__name__), connect_fail_python), 'server': patch('androidtv.adb_manager.Client', ClientFakeFail)}
-
-
-PATCH_CONNECT_PYTHON_FAIL_EXCEPTION = patch('{}.AdbDeviceFake.connect'.format(__name__), side_effect=Exception)
+        return {"python": patch("{}.AdbDeviceFake.connect".format(__name__), connect_success_python), "server": patch("androidtv.adb_manager.Client", ClientFakeSuccess)}
+    return {"python": patch("{}.AdbDeviceFake.connect".format(__name__), connect_fail_python), "server": patch("androidtv.adb_manager.Client", ClientFakeFail)}
 
 
 def patch_shell(response=None, error=False):
@@ -110,8 +113,15 @@ def shell_fail_server(self, cmd):
         raise ConnectionResetError
 
     if not error:
-        return {'python': patch('{}.AdbDeviceFake.shell'.format(__name__), shell_success), 'server': patch('{}.DeviceFake.shell'.format(__name__), shell_success)}
-    return {'python': patch('{}.AdbDeviceFake.shell'.format(__name__), shell_fail_python), 'server': patch('{}.DeviceFake.shell'.format(__name__), shell_fail_server)}
+        return {"python": patch("{}.AdbDeviceFake.shell".format(__name__), shell_success), "server": patch("{}.DeviceFake.shell".format(__name__), shell_success)}
+    return {"python": patch("{}.AdbDeviceFake.shell".format(__name__), shell_fail_python), "server": patch("{}.DeviceFake.shell".format(__name__), shell_fail_server)}
+
+
+PATCH_ADB_DEVICE = patch("androidtv.adb_manager.AdbDevice", AdbDeviceFake)
+
+
+class CustomException(Exception):
+    """A custom exception type."""
 
 
-patch_adb_device = patch('androidtv.adb_manager.AdbDevice', AdbDeviceFake)
+PATCH_CONNECT_FAIL_CUSTOM_EXCEPTION = {"python": patch("{}.AdbDeviceFake.connect".format(__name__), side_effect=CustomException), "server": patch("{}.ClientFakeSuccess.devices".format(__name__), side_effect=CustomException)}

tests/test_adb_manager.py

@@ -49,10 +49,6 @@ def open_priv_pub(infile):
         pass
 
 
-def raise_runtime_error(*args, **kwargs):
-    raise RuntimeError
-
-
 class LockedLock(object):
     @staticmethod
     def acquire(*args, **kwargs):
@@ -72,7 +68,7 @@ def setUp(self):
         """Create an `ADBPython` instance.
 
         """
-        with patchers.patch_adb_device, patchers.patch_connect(True)[self.PATCH_KEY]:
+        with patchers.PATCH_ADB_DEVICE, patchers.patch_connect(True)[self.PATCH_KEY]:
             self.adb = ADBPython('IP:5555')
 
     def test_connect_success(self):
@@ -93,11 +89,15 @@ def test_connect_fail(self):
             self.assertFalse(self.adb.available)
             self.assertFalse(self.adb._available)
 
-        if self.PATCH_KEY == 'python':
-            with patchers.PATCH_CONNECT_PYTHON_FAIL_EXCEPTION:
-                self.assertFalse(self.adb.connect())
-                self.assertFalse(self.adb.available)
-                self.assertFalse(self.adb._available)
+        with patchers.patch_connect(True)[self.PATCH_KEY]:
+            self.assertTrue(self.adb.connect())
+            self.assertTrue(self.adb.available)
+            self.assertTrue(self.adb._available)
+
+        with patchers.PATCH_CONNECT_FAIL_CUSTOM_EXCEPTION[self.PATCH_KEY]:
+            self.assertFalse(self.adb.connect())
+            self.assertFalse(self.adb.available)
+            self.assertFalse(self.adb._available)
 
     def test_connect_fail_lock(self):
         """Test when the connect attempt fails due to the lock.
@@ -148,25 +148,32 @@ def test_available(self):
         """
         with patchers.patch_connect(True)[self.PATCH_KEY]:
             self.assertTrue(self.adb.connect())
+            self.assertTrue(self.adb.available)
             self.adb._available = False
             self.assertTrue(self.adb.available)
+            self.assertTrue(self.adb._available)
 
-        with patchers.patch_connect(True)[self.PATCH_KEY], patch('{}.patchers.ClientFakeSuccess.devices'.format(__name__), raise_runtime_error):
+        with patchers.patch_connect(True)[self.PATCH_KEY], patch('{}.patchers.ClientFakeSuccess.devices'.format(__name__), side_effect=RuntimeError):
             self.assertFalse(self.adb.available)
 
         with patchers.patch_connect(True)[self.PATCH_KEY]:
             self.assertTrue(self.adb.connect())
-            self.adb._available = False
-            self.assertTrue(self.adb.available)
 
-        with patch('{}.patchers.DeviceFake.get_serial_no'.format(__name__), raise_runtime_error):
+        with patch('{}.patchers.DeviceFake.get_serial_no'.format(__name__), side_effect=RuntimeError):
+            self.assertFalse(self.adb.available)
+
+        with patchers.patch_connect(True)[self.PATCH_KEY]:
+            self.assertTrue(self.adb.connect())
+
+        with patch.object(self.adb._adb_client, 'devices', return_value=[]):
             self.assertFalse(self.adb.available)
 
         with patchers.patch_connect(True)[self.PATCH_KEY]:
             self.assertTrue(self.adb.connect())
 
-        with patch.object(self.adb._adb_client, 'devices', return_empty_list):
+        with patchers.PATCH_CONNECT_FAIL_CUSTOM_EXCEPTION[self.PATCH_KEY]:
             self.assertFalse(self.adb.available)
+            self.assertFalse(self.adb._available)
 
     def test_connect_fail_server(self):
         """Test that the ``connect`` method works correctly.
@@ -175,8 +182,10 @@ def test_connect_fail_server(self):
         with patchers.patch_connect(True)[self.PATCH_KEY]:
             self.assertTrue(self.adb.connect())
 
-        with patch('{}.patchers.ClientFakeSuccess.devices'.format(__name__), raise_runtime_error):#, patchers.patch_connect(True)[self.PATCH_KEY]:
+        with patch('{}.patchers.ClientFakeSuccess.devices'.format(__name__), side_effect=RuntimeError):
             self.assertFalse(self.adb.connect())
+            self.assertFalse(self.adb.available)
+            self.assertFalse(self.adb._available)
 
 
 class TestADBPythonWithAuthentication(unittest.TestCase):
@@ -188,7 +197,7 @@ def setUp(self):
         """Create an `ADBPython` instance.
 
         """
-        with patchers.patch_adb_device, patchers.patch_connect(True)[self.PATCH_KEY]:
+        with patchers.PATCH_ADB_DEVICE, patchers.patch_connect(True)[self.PATCH_KEY]:
             self.adb = ADBPython('IP:5555', 'adbkey')
 
     def test_connect_success_with_priv_key(self):
@@ -218,7 +227,7 @@ class TestADBPythonClose(unittest.TestCase):
     def test_close(self):
         """Test the `ADBPython.close` method.
         """
-        with patchers.patch_adb_device, patchers.patch_connect(True)[self.PATCH_KEY]:
+        with patchers.PATCH_ADB_DEVICE, patchers.patch_connect(True)[self.PATCH_KEY]:
             self.adb = ADBPython('IP:5555')
 
         with patchers.patch_connect(True)[self.PATCH_KEY]:

tests/test_androidtv.py

@@ -513,7 +513,7 @@ class TestAndroidTVPython(unittest.TestCase):
     ADB_ATTR = '_adb'
 
     def setUp(self):
-        with patchers.patch_adb_device, patchers.patch_connect(True)[self.PATCH_KEY], patchers.patch_shell('')[self.PATCH_KEY]:
+        with patchers.PATCH_ADB_DEVICE, patchers.patch_connect(True)[self.PATCH_KEY], patchers.patch_shell('')[self.PATCH_KEY]:
             self.atv = AndroidTV('IP:5555')
 
     def test_turn_on_off(self):

tests/test_basetv.py

@@ -87,7 +87,7 @@ class TestBaseTVPython(unittest.TestCase):
     ADB_ATTR = '_adb'
 
     def setUp(self):
-        with patchers.patch_adb_device, patchers.patch_connect(True)[self.PATCH_KEY], patchers.patch_shell('')[self.PATCH_KEY]:
+        with patchers.PATCH_ADB_DEVICE, patchers.patch_connect(True)[self.PATCH_KEY], patchers.patch_shell('')[self.PATCH_KEY]:
             self.btv = BaseTV('IP:5555')
 
     def test_available(self):

tests/test_firetv.py

@@ -128,7 +128,7 @@ class TestFireTVPython(unittest.TestCase):
     PATCH_KEY = 'python'
 
     def setUp(self):
-        with patchers.patch_adb_device, patchers.patch_connect(True)[self.PATCH_KEY], patchers.patch_shell('')[self.PATCH_KEY]:
+        with patchers.PATCH_ADB_DEVICE, patchers.patch_connect(True)[self.PATCH_KEY], patchers.patch_shell('')[self.PATCH_KEY]:
             self.ftv = FireTV('IP:5555')
 
     def test_turn_on_off(self):