Merge lp:~mikemc/ubuntu-sso-client/fix-1003692 into lp:ubuntu-sso-client

[Mike McCracken]

- Fixes "TypeError in got_state()" message and incorrect network detection on linux ( LP #1003692 )

- Fixes "TypeError in got_state()" message and incorrect network detection on linux ( LP #1003692 )

The linux version of is_machine_connected had an internal callback "got_state()", which expected an int while the NetworkManagerState will only ever give it one of {ONLINE, OFFLINE, UNKNOWN}. This branch fixes that callback.

The branch also adds tests for calling the callback with those three state values, and removes tests for calling the callback with int constants from NetworkManager that it will not be called with - they're only used inside NetworkManagerState.

Finally, it adds coverage in the tests of the NetworkManagerState class for three states from networkmanager that were not tested before - asleep, asleep_old, and unknown.

Note that the new tests say that the code should handle the "NetworkManager gave us state NM_UNKNOWN" case by returning "OFFLINE", not "UNKNOWN". This is to match the behavior of the code and its only client: syncdaemon's interaction_interfaces.py, where the state callback just compares the state to ONLINE and does treat UNKNOWN as offline, eg. in case of NetworkManager DBus error.

Tests pass on linux.

[Alejandro J. Cura (alecu)]

Code looks good, all tests pass on Linux. +1

review: Approve

[Manuel de la Peña (mandel)]

review: Approve

=== modified file 'ubuntu_sso/networkstate/linux.py'
--- ubuntu_sso/networkstate/linux.py	2012-04-23 12:22:03 +0000
+++ ubuntu_sso/networkstate/linux.py	2012-05-24 05:14:40 +0000
@@ -117,7 +117,10 @@
 
     def got_state(state):
         """The state was retrieved from the Network Manager."""
-        result = int(state) in NM_STATE_CONNECTED_LIST
+        if type(state) is not type(ONLINE):
+            logger.exception("bad callback argument in is_machine_connected")
+            raise NetworkFailException()
+        result = (state == ONLINE)
         d.callback(result)
 
     # pylint: disable=W0702, W0703

=== modified file 'ubuntu_sso/networkstate/tests/test_linux.py'
--- ubuntu_sso/networkstate/tests/test_linux.py	2012-04-19 19:29:37 +0000
+++ ubuntu_sso/networkstate/tests/test_linux.py	2012-05-24 05:14:40 +0000
@@ -178,98 +178,38 @@
         self.assertFalse(nms.state_signal.removed)
 
     @inlineCallbacks
-    def test_is_machine_connected_nm_state_connected_old(self):
-        """Fake the NetworkManagerState with connected return."""
-        self.patch(FakeNetworkManagerState, "connection_state",
-            NM_STATE_CONNECTED_OLD)
-        d = yield is_machine_connected()
-        self.assertTrue(d)
-
-    @inlineCallbacks
-    def test_is_machine_connected_nm_state_connected_global(self):
-        """Fake the NetworkManagerState with connected return."""
-        self.patch(FakeNetworkManagerState, "connection_state",
-            NM_STATE_CONNECTED_GLOBAL)
-        d = yield is_machine_connected()
-        self.assertTrue(d)
-
-    @inlineCallbacks
-    def test_is_machine_disconnected_nm_state_nm_state_unknown(self):
-        """Fake the NetworkManagerState with disconnected return."""
-        self.patch(FakeNetworkManagerState, "connection_state",
-            NM_STATE_UNKNOWN)
-        d = yield is_machine_connected()
-        self.assertFalse(d)
-
-    @inlineCallbacks
-    def test_is_machine_disconnected_nm_state_asleep_old(self):
-        """Fake the NetworkManagerState with disconnected return."""
-        self.patch(FakeNetworkManagerState, "connection_state",
-            NM_STATE_ASLEEP_OLD)
-        d = yield is_machine_connected()
-        self.assertFalse(d)
-
-    @inlineCallbacks
-    def test_is_machine_disconnected_nm_state_asleep(self):
-        """Fake the NetworkManagerState with disconnected return."""
-        self.patch(FakeNetworkManagerState, "connection_state",
-            NM_STATE_ASLEEP)
-        d = yield is_machine_connected()
-        self.assertFalse(d)
-
-    @inlineCallbacks
-    def test_is_machine_disconnected_nm_state_connecting_old(self):
-        """Fake the NetworkManagerState with disconnected return."""
-        self.patch(FakeNetworkManagerState, "connection_state",
-            NM_STATE_CONNECTING_OLD)
-        d = yield is_machine_connected()
-        self.assertFalse(d)
-
-    @inlineCallbacks
-    def test_is_machine_disconnected_nm_state_connecting(self):
-        """Fake the NetworkManagerState with disconnected return."""
-        self.patch(FakeNetworkManagerState, "connection_state",
-            NM_STATE_CONNECTING)
-        d = yield is_machine_connected()
-        self.assertFalse(d)
-
-    @inlineCallbacks
-    def test_is_machine_disconnected_nm_state_connected_local(self):
-        """Fake the NetworkManagerState with disconnected return."""
-        self.patch(FakeNetworkManagerState, "connection_state",
-            NM_STATE_CONNECTED_LOCAL)
-        d = yield is_machine_connected()
-        self.assertFalse(d)
-
-    @inlineCallbacks
-    def test_is_machine_disconnected_nm_state_connected_site(self):
-        """Fake the NetworkManagerState with disconnected return."""
-        self.patch(FakeNetworkManagerState, "connection_state",
-            NM_STATE_CONNECTED_SITE)
-        d = yield is_machine_connected()
-        self.assertFalse(d)
-
-    @inlineCallbacks
-    def test_is_machine_disconnected_nm_state_disconnected_old(self):
-        """Fake the NetworkManagerState with disconnected return."""
-        self.patch(FakeNetworkManagerState, "connection_state",
-            NM_STATE_DISCONNECTED_OLD)
-        d = yield is_machine_connected()
-        self.assertFalse(d)
-
-    @inlineCallbacks
-    def test_is_machine_disconnected_nm_state_disconnected(self):
-        """Fake the NetworkManagerState with disconnected return."""
-        self.patch(FakeNetworkManagerState, "connection_state",
-            NM_STATE_DISCONNECTED)
-        d = yield is_machine_connected()
-        self.assertFalse(d)
-
-    @inlineCallbacks
-    def test_is_machine_defer_error(self):
-        """Fake the NetworkManagerState with disconnected return."""
-        self.patch(FakeNetworkManagerState, "connection_state",
-            'no-int')
+    def test_is_machine_connected_nm_state_online(self):
+        """Callback given ONLINE should mean we are online"""
+        self.patch(FakeNetworkManagerState, "connection_state",
+                   ONLINE)
+        d = yield is_machine_connected()
+        self.assertTrue(d)
+
+    @inlineCallbacks
+    def test_is_machine_connected_nm_state_offline(self):
+        """Callback given OFFLINE should mean we are offline"""
+        self.patch(FakeNetworkManagerState, "connection_state",
+                   OFFLINE)
+        d = yield is_machine_connected()
+        self.assertFalse(d)
+
+    @inlineCallbacks
+    def test_is_machine_connected_nm_state_unknown(self):
+        """Callback given ONLINE should mean we are not online"""
+        self.patch(FakeNetworkManagerState, "connection_state",
+                   UNKNOWN)
+        d = yield is_machine_connected()
+        self.assertFalse(d)
+
+    @inlineCallbacks
+    def test_is_machine_connected_callback_error(self):
+        """Test bad argument to is_machine_connected's internal callback.
+
+        Passing anything other than ONLINE/OFFLINE/UNKNOWN should
+        cause an exception.
+        """
+        self.patch(FakeNetworkManagerState, "connection_state",
+                   NM_STATE_CONNECTED_GLOBAL)
         yield self.assertFailure(is_machine_connected(), NetworkFailException)
 
 
@@ -361,6 +301,18 @@
         super(NetworkManagerStateTestCase, self).setUp()
         self.connect_proxy()
 
+    def test_nm_asleep(self):
+        """Asleep status should mean offline."""
+        self.check_nm_state(self.assertOffline, NM_STATE_ASLEEP)
+
+    def test_nm_asleep_old(self):
+        """Asleep, old status, should mean offline."""
+        self.check_nm_state(self.assertOffline, NM_STATE_ASLEEP_OLD)
+
+    def test_nm_unknown(self):
+        """Unknown status should be treated the same as OFFLINE."""
+        self.check_nm_state(self.assertOffline, NM_STATE_UNKNOWN)
+
     def test_nm_online_old(self):
         """Check the connected, old status, case."""
         self.check_nm_state(self.assertOnline, NM_STATE_CONNECTED_OLD)