Merge lp:~diegosarmentero/ubuntuone-windows-installer/connect-files into lp:ubuntuone-windows-installer

[Diego Sarmentero]

Fixed: Should call self.backend.connect_files as soon as valid credentials are created (LP: #852085).

Fixed: Should call self.backend.connect_files as soon as valid credentials are created (LP: #852085).

[lp:~diegosarmentero/ubuntuone-windows-installer/connect-files]

83. By Diego Sarmentero on 2011-10-18

Fixed lint issues.

[Natalia Bidart (nataliabidart)]

* Unless I'm missing something, there is no need to define:

    @defer.inlineCallbacks
    def connect_file_sync(self):
        """Connect file sync service on login successful."""
        yield self.backend.connect_files()

you can just call self.backend.connect_files() in initializePage.

* On the other hand, the FakedBackend does not need to define the clear() method (AFAIK) and the self.properties['connect_files'] is not used, ergo not needed?

* There is a lint issue.

review: Needs Fixing

[lp:~diegosarmentero/ubuntuone-windows-installer/connect-files]

84. By Diego Sarmentero on 2011-10-18

Fixed lint issue.
Modified connect_sync_files

85. By Diego Sarmentero on 2011-10-20

Conflict resolved.

[Natalia Bidart (nataliabidart)]

== Python Lint Notices ==

ubuntuone_installer/gui/qt/tests/__init__.py:
    62: [C0111, FakeBackend] Missing docstring

ubuntuone_installer/gui/qt/utils/windows.py:
    41: [E0611] No name 'tools' in module 'ubuntuone.platform'

The last warning should be disabled (can't be fixed).

review: Needs Fixing

[Roberto Alsina (ralsina)]

Besides the lint problem Natalia mentioned, the code looks god to me.

review: Approve

[lp:~diegosarmentero/ubuntuone-windows-installer/connect-files]

86. By Diego Sarmentero on 2011-10-20

Fixed lint issues.

[Diego Sarmentero (diegosarmentero)]

> Besides the lint problem Natalia mentioned, the code looks god to me.

Fixed remaining lint issue about tools

[lp:~diegosarmentero/ubuntuone-windows-installer/connect-files]

87. By Diego Sarmentero on 2011-10-21

Fixed lint issue

[Natalia Bidart (nataliabidart)]

Conflict in:

Text conflict in ubuntuone_installer/gui/qt/utils/windows.py
1 conflicts encountered.

review: Needs Fixing

[Diego Sarmentero (diegosarmentero)]

> Conflict in:
>
> Text conflict in ubuntuone_installer/gui/qt/utils/windows.py
> 1 conflicts encountered.

Fixed

[lp:~diegosarmentero/ubuntuone-windows-installer/connect-files]

88. By Diego Sarmentero on 2011-10-25

Fixed conflict.

[Natalia Bidart (nataliabidart)]

So, perhaps I was not clear before... but I still see no point in having this patched connect_files method:

    def connect_files(self, func=None):
        """Fake connect_files"""
        self.properties['connect_files_called'] = True
        self.properties['connect_files'] = func

Why do we need the func=None parameter, if the original method does not receive any paramter? Why are we storing the func value, if then we're just passing None and testing against it?

Also, the self._called = self has no much sense as well, as far as I can see.

If I understand the fake correctly, I think this is a simpler option:

class FakeBackend(object):

    """Fake ControlBackend."""

    def __init__(self, *args, **kwargs):
        self.called = []
        self.clear = lambda: None
        self.connect_files = lambda: self.called.append('connect_files')

and have the test only test for assertEqual(called, ['connect_files']).

review: Needs Fixing

[lp:~diegosarmentero/ubuntuone-windows-installer/connect-files]

89. By Diego Sarmentero on 2011-10-28

Tests Fixed.

90. By Diego Sarmentero on 2011-10-28

Conflict resolved.

[Natalia Bidart (nataliabidart)]

Tests errors after the merge with trunk:

===============================================================================
[FAIL]
Traceback (most recent call last):
  File "/usr/lib/pymodules/python2.7/ubuntuone-dev-tools/ubuntuone/devtools/testing/txcheck.py", line 355, in run
    raise problem
ubuntuone.devtools.testing.txcheck.MissingReturnValue: MissingReturnValue for ubuntuone_installer.gui.qt.tests.test_gui.SuccesPageTestCase.setUp

ubuntuone.devtools.testing.txcheck.TXCheckTest.runTest
===============================================================================
[FAIL]
Traceback (most recent call last):
  File "/usr/lib/pymodules/python2.7/ubuntuone-dev-tools/ubuntuone/devtools/testing/txcheck.py", line 355, in run
    raise problem
ubuntuone.devtools.testing.txcheck.SuperResultDiscarded: SuperResultDiscarded for ubuntuone_installer.gui.qt.tests.test_gui.SuccesPageTestCase.setUp

ubuntuone.devtools.testing.txcheck.TXCheckTest.runTest
-------------------------------------------------------------------------------
Ran 146 tests in 6.151s

FAILED (failures=2, successes=146)

review: Needs Fixing

[lp:~diegosarmentero/ubuntuone-windows-installer/connect-files]

91. By Diego Sarmentero on 2011-10-28

Test fixed.

[Natalia Bidart (nataliabidart)]

C'mon! :-)

ubuntuone_installer/gui/qt/tests/__init__.py:
    73: [E0202, FakeBackend.clear] An attribute inherited from FakeBackend hide this method

review: Needs Fixing

[lp:~diegosarmentero/ubuntuone-windows-installer/connect-files]

92. By Diego Sarmentero on 2011-10-28

Fixed lint issues

[Natalia Bidart (nataliabidart)]

As per our latest conversation with Diego, we'd need to unify all the fakes control panel backend that are all around.

[Diego Sarmentero (diegosarmentero)]

> As per our latest conversation with Diego, we'd need to unify all the fakes
> control panel backend that are all around.

Refactor done!

[lp:~diegosarmentero/ubuntuone-windows-installer/connect-files]

93. By Diego Sarmentero on 2011-11-04

Unified Backend Fakes.

94. By Diego Sarmentero on 2011-11-04

Fixed tests.

95. By Diego Sarmentero on 2011-11-04

Tests fixed.

[Natalia Bidart (nataliabidart)]

== Python Lint Notices ==

ubuntuone_installer/gui/qt/tests/test_local_folders.py:
    42: [C0103] Invalid name "backend_account_info_data" (should match (([A-Z_][A-Z0-9_]*)|(__.*__))$)
    46: [C0103] Invalid name "backend_volume_info_data" (should match (([A-Z_][A-Z0-9_]*)|(__.*__))$)

ubuntuone_installer/gui/qt/tests/test_sync_now_or_later.py:
    40: [C0103] Invalid name "backend_volume_info_data_sync" (should match (([A-Z_][A-Z0-9_]*)|(__.*__))$)

review: Needs Fixing

[lp:~diegosarmentero/ubuntuone-windows-installer/connect-files]

96. By Diego Sarmentero on 2011-11-07

Fixed lint issues.

[Diego Sarmentero (diegosarmentero)]

> == Python Lint Notices ==
>
> ubuntuone_installer/gui/qt/tests/test_local_folders.py:
> 42: [C0103] Invalid name "backend_account_info_data" (should match
> (([A-Z_][A-Z0-9_]*)|(__.*__))$)
> 46: [C0103] Invalid name "backend_volume_info_data" (should match
> (([A-Z_][A-Z0-9_]*)|(__.*__))$)
>
> ubuntuone_installer/gui/qt/tests/test_sync_now_or_later.py:
> 40: [C0103] Invalid name "backend_volume_info_data_sync" (should match
> (([A-Z_][A-Z0-9_]*)|(__.*__))$)

Fixed

[lp:~diegosarmentero/ubuntuone-windows-installer/connect-files]

97. By Diego Sarmentero on 2011-11-08

merge

98. By Diego Sarmentero on 2011-11-08

Lint issues fixed.

[Natalia Bidart (nataliabidart)]

Looks good!

review: Approve

=== modified file 'ubuntuone_installer/gui/qt/gui.py'
--- ubuntuone_installer/gui/qt/gui.py	2011-10-17 15:11:02 +0000
+++ ubuntuone_installer/gui/qt/gui.py	2011-11-08 12:07:43 +0000
@@ -49,6 +49,7 @@
     SuccessController,
 )
 
+from ubuntuone.controlpanel import backend
 from ubuntuone.controlpanel.gui.qt.loadingoverlay import LoadingOverlay
 
 from ubuntuone.platform.credentials import (
@@ -214,6 +215,7 @@
     def __init__(self, parent=None):
         super(SuccessPage, self).__init__(
             Ui_SuccessPage(), SuccessController(), parent)
+        self.backend = backend.ControlBackend()
         self.finish_button = None
         self.next_button = None
 
@@ -236,6 +238,7 @@
         self.next_button.setDefault(True)
         self.next_button.style().unpolish(self.next_button)
         self.next_button.style().polish(self.next_button)
+        self.backend.connect_files()
 
     def nextId(self):
         """Provide the next id."""

=== modified file 'ubuntuone_installer/gui/qt/preferences.py'
--- ubuntuone_installer/gui/qt/preferences.py	2011-09-09 19:13:49 +0000
+++ ubuntuone_installer/gui/qt/preferences.py	2011-11-08 12:07:43 +0000
@@ -45,6 +45,7 @@
     # pylint: disable=C0103
 
     def initializePage(self):
+        """Initialize the page before the Wizard show it."""
         self.wizard().setOption(QtGui.QWizard.HaveCustomButton1, True)
         self.wizard().setOption(QtGui.QWizard.HaveCustomButton2, True)
         # This is just to catch an exception thrown when nothing

=== modified file 'ubuntuone_installer/gui/qt/tests/__init__.py'
--- ubuntuone_installer/gui/qt/tests/__init__.py	2011-10-28 11:00:42 +0000
+++ ubuntuone_installer/gui/qt/tests/__init__.py	2011-11-08 12:07:43 +0000
@@ -60,6 +60,55 @@
             self.target(*args)
 
 
+class FakeBackend(object):
+
+    """Fake ControlBackend."""
+
+    account_info_data = None
+    volumes_info_data = None
+
+    def __init__(self, *args, **kwargs):
+        self.called = []
+        self._called = self
+        self.clear = lambda: None
+        self.connect_files = lambda: self.called.append('connect_files')
+        self._is_valid = True
+        self.volume_setings_changes = []
+        self.folders_created = []
+
+    def account_info(self, *args):
+        """Fake account info."""
+        return defer.succeed(self.account_info_data)
+
+    def volumes_info(self, *args):
+        """Fake volumes info."""
+        return defer.succeed(self.volumes_info_data)
+
+    def validate_path_for_folder(self, path):
+        """Fake folder validation."""
+        return self._is_valid
+
+    def change_volume_settings(self, *args):
+        """Fake change volume settings."""
+        self.volume_setings_changes.append(args)
+
+    def create_folder(self, *args):
+        """Fake folder creation."""
+        self.folders_created.append(args)
+
+
+class FakeFailingBackend(object):
+    """Fake Control Panel backend that fails."""
+
+    def account_info(self, *args):
+        """Fake account info."""
+        return defer.fail(Exception())
+
+    def volumes_info(self, *args):
+        """Fake account info."""
+        return defer.fail(Exception())
+
+
 class FakeController(object):
 
     """A fake controller for the tests."""

=== modified file 'ubuntuone_installer/gui/qt/tests/test_gui.py'
--- ubuntuone_installer/gui/qt/tests/test_gui.py	2011-10-28 11:00:42 +0000
+++ ubuntuone_installer/gui/qt/tests/test_gui.py	2011-11-08 12:07:43 +0000
@@ -28,6 +28,7 @@
 from ubuntuone_installer.gui.qt import gui, forgotten, utils
 from ubuntuone_installer.gui.qt.tests import (
     BaseTestCase,
+    FakeBackend,
     FakeController,
     FakeOverlay,
     FakeSignal,
@@ -119,6 +120,7 @@
         self.patch(qt.preferences, "PreferencesPanel", FakePreferencesPanel)
         self.patch(qt.folders, "FoldersPanel", FakeFoldersPanel)
         self.patch(gui.qt.utils, "add_syncdaemon_to_autostart", NO_OP)
+        self.patch(gui.backend, "ControlBackend", FakeBackend)
         yield super(MainWindowTestCase, self).setUp()
         setup_page = self.ui.page(self.ui.setup_account_page_id)
         setup_page.initializePage()
@@ -655,6 +657,12 @@
 
     class_ui = gui.SuccessPage
 
+    @defer.inlineCallbacks
+    def setUp(self):
+        """Initialize this test instance."""
+        self.patch(gui.backend, "ControlBackend", FakeBackend)
+        yield super(SuccesPageTestCase, self).setUp()
+
     def test_after_initialize(self):
         """Check the page is initialized correctly."""
         wizard = FakeMainWindow()
@@ -673,10 +681,11 @@
          QtGui.QWizard.NextButton])
         self.assertTrue(self.ui.next_button.isDefault())
         self.assertFalse(self.ui.finish_button.isDefault())
+        self.assertEqual(self.ui.backend.called, ['connect_files'])
 
 
 class SignInPageTestCase(BaseTestCase):
-    """Test the SuccessPage code."""
+    """Test the SignInPage code."""
 
     @defer.inlineCallbacks
     def setUp(self):

=== modified file 'ubuntuone_installer/gui/qt/tests/test_local_folders.py'
--- ubuntuone_installer/gui/qt/tests/test_local_folders.py	2011-10-28 11:00:42 +0000
+++ ubuntuone_installer/gui/qt/tests/test_local_folders.py	2011-11-08 12:07:43 +0000
@@ -19,6 +19,7 @@
 """Tests for the local folders page."""
 
 import os
+import copy
 import Queue
 import shutil
 import tempfile
@@ -28,10 +29,44 @@
 
 from ubuntuone_installer.gui import NEXT
 from ubuntuone_installer.gui.qt import local_folders
-from ubuntuone_installer.gui.qt.tests import BaseTestCase, TestCase
+from ubuntuone_installer.gui.qt.tests import (
+    BaseTestCase,
+    FakeBackend,
+    FakeFailingBackend,
+    TestCase,
+)
 from ubuntuone_installer.gui.qt.tests.test_gui import FakeMainWindow
 
 
+# Define backend data for account_info and volume_info
+BACKEND_ACCOUNT_INFO_DATA = {
+                                "quota_total": 1000,
+                                "quota_used": 200,
+                            }
+BACKEND_VOLUME_INFO_DATA = ((None, None,
+                                [
+                                {
+                                    'type': u'UDF',
+                                    'path': os.path.expanduser(u'~/xyzzy'),
+                                    'volume_id': 'asdfgh',
+                                    'subscribed': True,
+                                },
+                                {
+                                    'type': u'UDF',
+                                    'path': os.path.expanduser(u'~/zxyzzy'),
+                                    'volume_id': 'qwerty',
+                                    'subscribed': False,
+                                },
+                                {
+                                    'type': u'SHARE',
+                                    'path': os.path.expanduser(u'~/foobar'),
+                                    'volume_id': 'shared',
+                                    'subscribed': False,
+                                },
+                                ],
+                            ),)
+
+
 class FakeFileDialog(object):
 
     """A fake QFileDialog class."""
@@ -56,72 +91,6 @@
         self._warning = (args, kwargs)
 
 
-class FakeCPBackend(object):
-    """Fake Control Panel backend."""
-
-    def __init__(self):
-        """Initialize."""
-        self._is_valid = True
-        self.volume_setings_changes = []
-        self.folders_created = []
-
-    def account_info(self, *args):
-        """Fake account info."""
-        return defer.succeed({
-            "quota_total": 1000,
-            "quota_used": 200,
-            })
-
-    def volumes_info(self, *args):
-        """Fake volumes info."""
-        return defer.succeed(((None, None,
-            [
-            {
-                'type': u'UDF',
-                'path': os.path.expanduser(u'~/xyzzy'),
-                'volume_id': 'asdfgh',
-                'subscribed': True,
-            },
-            {
-                'type': u'UDF',
-                'path': os.path.expanduser(u'~/zxyzzy'),
-                'volume_id': 'qwerty',
-                'subscribed': False,
-            },
-            {
-                'type': u'SHARE',
-                'path': os.path.expanduser(u'~/foobar'),
-                'volume_id': 'shared',
-                'subscribed': False,
-            },
-            ],
-        ),))
-
-    def validate_path_for_folder(self, path):
-        """Fake folder validation."""
-        return self._is_valid
-
-    def change_volume_settings(self, *args):
-        """Fake change volume settings."""
-        self.volume_setings_changes.append(args)
-
-    def create_folder(self, *args):
-        """Fake folder creation."""
-        self.folders_created.append(args)
-
-
-class FakeFailingCPBackend(object):
-    """Fake Control Panel backend that fails."""
-
-    def account_info(self, *args):
-        """Fake account info."""
-        return defer.fail(Exception())
-
-    def volumes_info(self, *args):
-        """Fake account info."""
-        return defer.fail(Exception())
-
-
 class FakeCalculateSize(object):
 
     """A fake CalculateSize."""
@@ -181,7 +150,11 @@
         f.close()
         self.addCleanup(shutil.rmtree, self.tmpdir)
         self.fake_wizard = FakeMainWindow()
-        self.patch(local_folders.backend, "ControlBackend", FakeCPBackend)
+        self.patch(local_folders.backend, "ControlBackend", FakeBackend)
+        self.patch(FakeBackend, "account_info_data",
+            copy.copy(BACKEND_ACCOUNT_INFO_DATA))
+        self.patch(FakeBackend, "volumes_info_data",
+            copy.copy(BACKEND_VOLUME_INFO_DATA))
         yield super(LocalFoldersWithGetSizeTestCase, self).setUp()
         self.patch(self.ui, "wizard", lambda: self.fake_wizard)
 
@@ -223,7 +196,11 @@
         self.tmpdir = tempfile.mkdtemp()
         self.addCleanup(shutil.rmtree, self.tmpdir)
         self.fake_wizard = FakeMainWindow()
-        self.patch(local_folders.backend, "ControlBackend", FakeCPBackend)
+        self.patch(local_folders.backend, "ControlBackend", FakeBackend)
+        self.patch(FakeBackend, "account_info_data",
+            copy.copy(BACKEND_ACCOUNT_INFO_DATA))
+        self.patch(FakeBackend, "volumes_info_data",
+            copy.copy(BACKEND_VOLUME_INFO_DATA))
         self.patch(os.path, 'getsize', lambda *args: 0)
         yield super(LocalFoldersTestCase, self).setUp()
         self.patch(self.ui, "wizard", lambda: self.fake_wizard)
@@ -536,8 +513,7 @@
 
     def test_exception_on_account_info(self):
         """When account_info fails, nothing should happen."""
-        self.patch(self.ui,
-            "cp_backend", FakeFailingCPBackend())
+        self.patch(self.ui, "cp_backend", FakeFailingBackend())
         self.ui.get_info()
         # Still here
 

=== modified file 'ubuntuone_installer/gui/qt/tests/test_sync_now_or_later.py'
--- ubuntuone_installer/gui/qt/tests/test_sync_now_or_later.py	2011-10-28 11:00:42 +0000
+++ ubuntuone_installer/gui/qt/tests/test_sync_now_or_later.py	2011-11-08 12:07:43 +0000
@@ -25,25 +25,26 @@
 
 from ubuntuone_installer.gui import NEXT
 from ubuntuone_installer.gui.qt import sync_now_or_later
-from ubuntuone_installer.gui.qt.tests import BaseTestCase
+from ubuntuone_installer.gui.qt.tests import (
+    BaseTestCase,
+    FakeBackend,
+)
 from ubuntuone_installer.gui.qt.tests.test_gui import FakeMainWindow
 from ubuntuone_installer.gui.qt.tests.test_local_folders import (
-    FakeCPBackend,
+    BACKEND_ACCOUNT_INFO_DATA,
+    BACKEND_VOLUME_INFO_DATA,
 )
 
 
-class FakeEmptyCPBackend(FakeCPBackend):
-    """A CP backend that returns only one folder."""
-    def volumes_info(self, *args):
-        """Fake volumes info."""
-        return defer.succeed(((None, None,
-            [
-            {
-                'type': u'ROOT',
-                'path': os.path.expanduser(u'~/Ubuntu One'),
-                'volume_id': 'asdfgh',
-                'subscribed': True,
-            }]),))
+# Define backend data for account_info and volume_info
+BACKEND_VOLUME_INFO_DATA_SYNC = ((None, None,
+                            [
+                            {
+                                'type': u'ROOT',
+                                'path': os.path.expanduser(u'~/Ubuntu One'),
+                                'volume_id': 'asdfgh',
+                                'subscribed': True,
+                            }]),)
 
 
 class SyncNowOrLaterTestCase(BaseTestCase):
@@ -57,7 +58,9 @@
         yield super(SyncNowOrLaterTestCase, self).setUp()
         self.main_window = FakeMainWindow()
         self.patch(self.ui, "wizard", lambda: self.main_window)
-        self.patch(sync_now_or_later.backend, "ControlBackend", FakeCPBackend)
+        self.patch(sync_now_or_later.backend, "ControlBackend", FakeBackend)
+        self.patch(FakeBackend, "account_info_data", BACKEND_ACCOUNT_INFO_DATA)
+        self.patch(FakeBackend, "volumes_info_data", BACKEND_VOLUME_INFO_DATA)
 
     @defer.inlineCallbacks
     def test_has_folders_in_cloud(self):
@@ -68,8 +71,9 @@
     @defer.inlineCallbacks
     def test_has_no_folders_in_cloud(self):
         """test behaviour when the user has no folders in the cloud."""
-        self.patch(sync_now_or_later.backend,
-            "ControlBackend", FakeEmptyCPBackend)
+        self.patch(sync_now_or_later.backend, "ControlBackend", FakeBackend)
+        self.patch(FakeBackend, "volumes_info_data",
+            BACKEND_VOLUME_INFO_DATA_SYNC)
         yield self.ui.get_info()
         self.assertFalse(self.ui.has_cloud_folders)
 

=== modified file 'ubuntuone_installer/gui/qt/utils/windows.py'
--- ubuntuone_installer/gui/qt/utils/windows.py	2011-10-20 14:33:35 +0000
+++ ubuntuone_installer/gui/qt/utils/windows.py	2011-11-08 12:07:43 +0000
@@ -41,6 +41,7 @@
 # pylint: disable=E0611
 from ubuntuone.platform import tools
 # pylint: enable=E0611
+
 from ubuntuone.platform.credentials import CredentialsManagementTool
 
 AUTORUN_KEY = r"Software\Microsoft\Windows\CurrentVersion\Run"
@@ -202,7 +203,7 @@
     # XXXX to be replaced by calls to xdg_base_directory's
     # special_folders. Also the correct funtion to call is
     # ShGetFolderPath per http://msdn.microsoft.com/en-us
-    # /library/windows/desktop/bb762204(v=vs.85).aspx
+    # /library/windows/desktop/bb762204(v=vs.85).aspx
     dll = ctypes.windll.shell32
     buf = ctypes.create_unicode_buffer(300)
     dll.SHGetSpecialFolderPathW(None, buf, CSIDL_PERSONAL, False)