Merge lp:~diegosarmentero/ubuntuone-control-panel/limit-bandwidth into lp:ubuntuone-control-panel

[Diego Sarmentero]

- Adding limit_bandwidth attribute to the info dict processed by preferences (LP: #944256).

[Natalia Bidart (nataliabidart)]

Thanks for working on this!

We should not add a new entry LIMIT_BW_KEY in the dictionary since that will require a FFe (since the API changes), and also because is not necessary.

The bug is clearly where you detected it (the missing call to limits_enabled = yield self.sd_client.bandwidth_throttling_enabled()), so we should indeed add that call, and then make both result[DOWNLOAD_KEY] = limits['download'] and result[UPLOAD_KEY] be -1, since the agreement between the UI and the backend is:

- if limits are -1, then it means that throttling is disabled.
- if any limit is not -1, then throttling is enabled for that limit.

Does the above make sense?

review: Needs Fixing

[lp:~diegosarmentero/ubuntuone-control-panel/limit-bandwidth]

302. By Diego Sarmentero on 2012-03-28

Improves in limit detection.

303. By Diego Sarmentero on 2012-03-28

Adding test for backend

[Natalia Bidart (nataliabidart)]

I really like the code!

One tiny note: the test added should have something like this:

    @inlineCallbacks
    def test_file_sync_settings_info_with_limit(self):
        """The settings_info method exercises its callback."""
        self.patch(self.be.sd_client, "throttling", False)
        self.be.sd_client.limits = {"download": 987456, "upload": 125698}

        # ...

            backend.DOWNLOAD_KEY: -1,
            backend.UPLOAD_KEY: -1,
        }

so we fully test that even if SD limits are not -1, the limits returned to the UI are -1 when throttling is disabled.

review: Needs Fixing

[Diego Sarmentero (diegosarmentero)]

> I really like the code!
>
> One tiny note: the test added should have something like this:
>
> @inlineCallbacks
> def test_file_sync_settings_info_with_limit(self):
> """The settings_info method exercises its callback."""
> self.patch(self.be.sd_client, "throttling", False)
> self.be.sd_client.limits = {"download": 987456, "upload": 125698}
>
> # ...
>
> backend.DOWNLOAD_KEY: -1,
> backend.UPLOAD_KEY: -1,
> }
>
> so we fully test that even if SD limits are not -1, the limits returned to the
> UI are -1 when throttling is disabled.

Fixed

[lp:~diegosarmentero/ubuntuone-control-panel/limit-bandwidth]

304. By Diego Sarmentero on 2012-03-28

Improving test.

[Natalia Bidart (nataliabidart)]

Looks and works great!

review: Approve

[dobey (dobey)]

review: Approve

=== modified file 'ubuntuone/controlpanel/backend.py'
--- ubuntuone/controlpanel/backend.py	2012-03-01 22:05:51 +0000
+++ ubuntuone/controlpanel/backend.py	2012-03-28 17:13:17 +0000
@@ -787,8 +787,13 @@
             result[name] = bool(value)
 
         limits = yield self.sd_client.get_throttling_limits()
-        result[DOWNLOAD_KEY] = limits['download']
-        result[UPLOAD_KEY] = limits['upload']
+        limits_enabled = yield self.sd_client.bandwidth_throttling_enabled()
+        if limits_enabled:
+            result[DOWNLOAD_KEY] = limits['download']
+            result[UPLOAD_KEY] = limits['upload']
+        else:
+            result[DOWNLOAD_KEY] = -1
+            result[UPLOAD_KEY] = -1
 
         returnValue(result)
 
 
=== modified file 'ubuntuone/controlpanel/gui/qt/tests/test_preferences.py'
--- ubuntuone/controlpanel/gui/qt/tests/test_preferences.py	2012-03-01 22:05:51 +0000
+++ ubuntuone/controlpanel/gui/qt/tests/test_preferences.py	2012-03-28 17:13:17 +0000
@@ -76,6 +76,21 @@
 
         self.assertFalse(self.ui.is_processing)
 
+    def test_process_info_limit_bandwidth(self):
+        """The ui is not processing when contents are load."""
+        self.ui.process_info(SAMPLE_SETTINGS)
+
+        self.assertTrue(self.ui.ui.limit_uploads_checkbox.isChecked())
+        self.assertTrue(self.ui.ui.limit_downloads_checkbox.isChecked())
+
+        settings = SAMPLE_SETTINGS.copy()
+        settings[gui.backend.DOWNLOAD_KEY] = -1
+        settings[gui.backend.UPLOAD_KEY] = -1
+        self.ui.process_info(settings)
+
+        self.assertFalse(self.ui.ui.limit_uploads_checkbox.isChecked())
+        self.assertFalse(self.ui.ui.limit_downloads_checkbox.isChecked())
+
     def test_info_is_requested_on_load(self):
         """The info is requested to the backend."""
         self.assert_backend_called('file_sync_settings_info')
 
=== modified file 'ubuntuone/controlpanel/tests/test_backend.py'
--- ubuntuone/controlpanel/tests/test_backend.py	2012-02-06 15:23:27 +0000
+++ ubuntuone/controlpanel/tests/test_backend.py	2012-03-28 17:13:17 +0000
@@ -1598,6 +1598,7 @@
     @inlineCallbacks
     def test_file_sync_settings_info(self):
         """The settings_info method exercises its callback."""
+        self.patch(self.be.sd_client, "throttling", True)
         self.be.sd_client.limits = {"download": 1000, "upload": 100}
         expected = {
             backend.AUTOCONNECT_KEY: self.be.sd_client.autoconnect,
@@ -1614,6 +1615,25 @@
         self.assertEqual(expected, result)
 
     @inlineCallbacks
+    def test_file_sync_settings_info_with_limit(self):
+        """The settings_info method exercises its callback."""
+        self.patch(self.be.sd_client, "throttling", False)
+        self.be.sd_client.limits = {"download": 987456, "upload": 125698}
+        expected = {
+            backend.AUTOCONNECT_KEY: self.be.sd_client.autoconnect,
+            backend.SHOW_ALL_NOTIFICATIONS_KEY:
+                self.be.sd_client.show_all_notifications,
+            backend.SHARE_AUTOSUBSCRIBE_KEY:
+                self.be.sd_client.share_autosubscribe,
+            backend.UDF_AUTOSUBSCRIBE_KEY:
+                self.be.sd_client.udf_autosubscribe,
+            backend.DOWNLOAD_KEY: -1,
+            backend.UPLOAD_KEY: -1,
+        }
+        result = yield self.be.file_sync_settings_info()
+        self.assertEqual(expected, result)
+
+    @inlineCallbacks
     def test_change_file_sync_setting_autoconnect(self):
         """The settings can be changed for autoconnect."""
         yield self.assert_boolean_setting_is_correct(backend.AUTOCONNECT_KEY)