Merge lp:~mandel/ubuntuone-control-panel/auto-update-functions into lp:ubuntuone-control-panel

[Manuel de la Peña]

Moves auto-update code from the u1-windows-installer to the control panel.

Moves auto-update code from the u1-windows-installer to the control panel.

[Diego Sarmentero (diegosarmentero)]

utils.os.path.pardir, esta repetido:

158 + utils.os.path.pardir,
159 + utils.os.path.pardir,

review: Needs Fixing

[Manuel de la Peña (mandel)]

> utils.os.path.pardir, esta repetido:
>
> 158 + utils.os.path.pardir,
> 159 + utils.os.path.pardir,

Actually that is a fix, you need to move an extra level up since we now have utils as a pacakage :)

[Diego Sarmentero (diegosarmentero)]

+1

review: Approve

[Natalia Bidart (nataliabidart)]

A few minor tweaks:

* instead of duplicating the -i "test_windows.py" in run-tests, please define a new bash variable and use that

* we're trying to get rid of the authors section of the source headers, so instead of adding yourself, please remove all the authors in the files you modify :-)

* remember to always use %r for logging stuff

* why are we receiving the logger by parameter? that makes the api of the methods weird... let's grab the logger just like in the rest of the app (and then we can remove the fake logger altogether):

from ubuntuone.controlpanel import set_up_logger

logger = set_up_logger('utils')

* the logging itself could use some improvement, so when seen in a log file our of the code context, we can understand what information is giving us. So below I propose a new version of are_updates_present:

@defer.inlineCallbacks
def are_updates_present():
    """Return if there are updates for Ubuntu One."""
     result = False
     retcode = None
    update_path = _get_update_path()
    if update_path is not None:
        # If there is an update present we will get 0, non-zero otherwise
        retcode = yield getProcessValue(update_path, args=('--mode',
            'unattended'), path=os.path.dirname(update_path))
        result = retcode == 0
    logger.debug('are_updates_present: update path: %r, return code: %r, result: %r',
                 update_path, retcode, result)
    defer.returnValue(result)

* Remember to have cleanup code to be run ALWAYS, even if the test fails. In your tests, the cleanup code is only run if none of the asserts failed. So, please use self.addCleanup calls as soon as you modify the env for the test.

review: Needs Fixing

[lp:~mandel/ubuntuone-control-panel/auto-update-functions]

241. By Manuel de la Peña on 2011-10-26

Fixed branch according to review comments.

[Manuel de la Peña (mandel)]

> A few minor tweaks:
>
> * instead of duplicating the -i "test_windows.py" in run-tests, please define
> a new bash variable and use that
>
> * we're trying to get rid of the authors section of the source headers, so
> instead of adding yourself, please remove all the authors in the files you
> modify :-)
>
> * remember to always use %r for logging stuff
>
> * why are we receiving the logger by parameter? that makes the api of the
> methods weird... let's grab the logger just like in the rest of the app (and
> then we can remove the fake logger altogether):
>
> from ubuntuone.controlpanel import set_up_logger
>
> logger = set_up_logger('utils')
>
> * the logging itself could use some improvement, so when seen in a log file
> our of the code context, we can understand what information is giving us. So
> below I propose a new version of are_updates_present:
>
> @defer.inlineCallbacks
> def are_updates_present():
> """Return if there are updates for Ubuntu One."""
> result = False
> retcode = None
> update_path = _get_update_path()
> if update_path is not None:
> # If there is an update present we will get 0, non-zero otherwise
> retcode = yield getProcessValue(update_path, args=('--mode',
> 'unattended'), path=os.path.dirname(update_path))
> result = retcode == 0
> logger.debug('are_updates_present: update path: %r, return code: %r,
> result: %r',
> update_path, retcode, result)
> defer.returnValue(result)
>
> * Remember to have cleanup code to be run ALWAYS, even if the test fails. In
> your tests, the cleanup code is only run if none of the asserts failed. So,
> please use self.addCleanup calls as soon as you modify the env for the test.

Fixed all of them :)

[Natalia Bidart (nataliabidart)]

I'm having this test failure on windows:

===============================================================================
[FAIL]
Traceback (most recent call last):
  File "C:\Python27\lib\site-packages\ubuntuone-dev-tools\ubuntuone\devtools\tes
ting\txcheck.py", line 355, in run
    raise problem
ubuntuone.devtools.testing.txcheck.SuperNotCalled: SuperNotCalled for ubuntuone.
controlpanel.utils.tests.test_windows.GetPathTestCase.setUp

ubuntuone.devtools.testing.txcheck.TXCheckTest.runTest

review: Needs Fixing

[lp:~mandel/ubuntuone-control-panel/auto-update-functions]

243. By Manuel de la Peña on 2011-11-14

Call super.setUp()

[Natalia Bidart (nataliabidart)]

* addCleanup supports passing arguments next to the function, so instead of

+ self.addCleanup(lambda: self._reset_frozen_state(old_frozen,
+ old_exec_path))

you just have to call:

+ self.addCleanup(self._reset_frozen_state, old_frozen, old_exec_path)

Same for:

+ self.addCleanup(lambda: self._reset__file__(old_file))

you just have to call:

+ self.addCleanup(self._reset__file__, old_file)

* On ubuntuone/controlpanel/utils/windows.py, define logger globally so is accessible to the whole module, and to also maintain consistency with the rest of the project.

The rest looks good!

review: Needs Fixing

[lp:~mandel/ubuntuone-control-panel/auto-update-functions]

244. By Manuel de la Peña on 2011-11-15

Made changes according to reviews.

245. By Manuel de la Peña on 2011-11-15

Merged with parent.

[Natalia Bidart (nataliabidart)]

Looks good!

review: Approve

=== modified file 'run-tests'
--- run-tests	2011-10-05 18:17:47 +0000
+++ run-tests	2011-11-15 13:13:25 +0000
@@ -18,6 +18,7 @@
 
 QT_TESTS_PATH=ubuntuone/controlpanel/gui/qt/tests
 GTK_TESTS_PATH=ubuntuone/controlpanel/gui/gtk/tests
+WINDOWS_TESTS=test_windows.py
 
 set -e
 
@@ -50,9 +51,9 @@
 ./setup.py build
 echo "Running test suite for ""$MODULE"
 if [ "$USE_QT" -eq 0 ]; then
-    `which xvfb-run` u1trial --ignore-paths $QT_TESTS_PATH "$MODULE"
+    `which xvfb-run` u1trial --ignore-paths "$QT_TESTS_PATH" -i "$WINDOWS_TESTS" "$MODULE"
 else
-    `which xvfb-run` u1trial --ignore-paths $GTK_TESTS_PATH --reactor=qt4 --gui "$MODULE"
+    `which xvfb-run` u1trial --ignore-paths "$GTK_TESTS_PATH" -i "$WINDOWS_TESTS" --reactor=qt4 --gui "$MODULE"
 fi
 style_check
 rm -rf _trial_temp
 
=== modified file 'run-tests.bat'
--- run-tests.bat	2011-10-06 20:38:39 +0000
+++ run-tests.bat	2011-11-15 13:13:25 +0000
@@ -20,12 +20,13 @@
 
 SET MODULE="ubuntuone"
 SET PYTHONEXEPATH="C:\Python27"
-SET IGNORE_PATHS="ubuntuone\controlpanel\gui\gtk, ubuntuone\controlpanel\dbustests, ubuntuone\controlpanel\web_client\tests\test_libsoup.py"
+SET IGNORE_PATHS="ubuntuone\controlpanel\gui\gtk, ubuntuone\controlpanel\dbustests"
+SET IGNORE_MODULES="test_linux.py, test_libsoup.py"
 
 "%PYTHONEXEPATH%\python.exe" setup.py build
 ECHO Running tests
 :: execute the tests with a number of ignored linux only modules
-"%PYTHONEXEPATH%\python.exe" "%PYTHONEXEPATH%\Scripts\u1trial" --reactor=qt4 --gui -p %IGNORE_PATHS% %MODULE%
+"%PYTHONEXEPATH%\python.exe" "%PYTHONEXEPATH%\Scripts\u1trial" --reactor=qt4 --gui -p %IGNORE_PATHS% -i %IGNORE_MODULES% %MODULE%
 :: Clean the build from the setupt.py
 ECHO Cleaning the generated code before running the style checks...
 "%PYTHONEXEPATH%\python.exe" setup.py clean
 
=== added directory 'ubuntuone/controlpanel/utils'
=== renamed file 'ubuntuone/controlpanel/utils.py' => 'ubuntuone/controlpanel/utils/__init__.py'
--- ubuntuone/controlpanel/utils.py	2011-06-29 18:10:16 +0000
+++ ubuntuone/controlpanel/utils/__init__.py	2011-11-15 13:13:25 +0000
@@ -1,6 +1,5 @@
 # -*- coding: utf-8 -*-
 
-# Authors: Natalia B Bidart <[e-mail]>
 #
 # Copyright 2010 Canonical Ltd.
 #
@@ -19,6 +18,7 @@
 """Miscellaneous utilities."""
 
 import os
+import sys
 
 from ubuntuone.controlpanel.logger import setup_logging
 
@@ -30,6 +30,19 @@
 ERROR_TYPE = 'error_type'
 ERROR_MESSAGE = 'error_msg'
 
+# ignore issues with the name of the method
+# pylint: disable=C0103
+
+# import the platform dependent code.
+if sys.platform == 'win32':
+    from ubuntuone.controlpanel.utils import windows
+    are_updates_present = windows.are_updates_present
+    perform_update = windows.perform_update
+else:
+    are_updates_present = lambda *args, **kwargs: False
+    perform_update = lambda *args, **kwargs: None
+# pylint: enable=C0103
+
 
 def get_project_dir():
     """Return the absolute path to this project's data/ dir.
@@ -38,7 +51,8 @@
     """
     module = os.path.dirname(__file__)
     result = os.path.abspath(os.path.join(module, os.path.pardir,
-                                          os.path.pardir, DATA_SUFFIX))
+                                          os.path.pardir, os.path.pardir,
+                                          DATA_SUFFIX))
     logger.debug('get_project_dir: trying use data dir at %r (exists? %s)',
                   result, os.path.exists(result))
     if os.path.exists(result):
 
=== added directory 'ubuntuone/controlpanel/utils/tests'
=== added file 'ubuntuone/controlpanel/utils/tests/__init__.py'
--- ubuntuone/controlpanel/utils/tests/__init__.py	1970-01-01 00:00:00 +0000
+++ ubuntuone/controlpanel/utils/tests/__init__.py	2011-11-15 13:13:25 +0000
@@ -0,0 +1,18 @@
+# -*- coding: utf-8 -*-
+
+#
+# Copyright 2010 Canonical Ltd.
+#
+# This program is free software: you can redistribute it and/or modify it
+# under the terms of the GNU General Public License version 3, as published
+# by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranties of
+# MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
+# PURPOSE.  See the GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License along
+# with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Test utils."""
 
=== added file 'ubuntuone/controlpanel/utils/tests/test_linux.py'
--- ubuntuone/controlpanel/utils/tests/test_linux.py	1970-01-01 00:00:00 +0000
+++ ubuntuone/controlpanel/utils/tests/test_linux.py	2011-11-15 13:13:25 +0000
@@ -0,0 +1,38 @@
+# -*- coding: utf-8 -*-
+
+#
+# Copyright 2011 Canonical Ltd.
+#
+# This program is free software: you can redistribute it and/or modify it
+# under the terms of the GNU General Public License version 3, as published
+# by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranties of
+# MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
+# PURPOSE.  See the GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License along
+# with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Test the linux utils functions."""
+
+
+from twisted.internet import defer
+
+from ubuntuone.controlpanel import utils
+from ubuntuone.controlpanel.tests import TestCase
+
+
+class AutoupdaterTestCase(TestCase):
+    """Test the code that is used for the auto update process."""
+
+    @defer.inlineCallbacks
+    def test_are_updates_present(self):
+        """We should never have updates."""
+        are_present = yield utils.are_updates_present(None)
+        self.assertFalse(are_present)
+
+    def test_perform_update(self):
+        """Test the method that performs the update."""
+        self.assertEqual(None, utils.perform_update())
 
=== renamed file 'ubuntuone/controlpanel/tests/test_utils.py' => 'ubuntuone/controlpanel/utils/tests/test_utils.py'
--- ubuntuone/controlpanel/tests/test_utils.py	2011-10-24 21:48:27 +0000
+++ ubuntuone/controlpanel/utils/tests/test_utils.py	2011-11-15 13:13:25 +0000
@@ -73,7 +73,9 @@
         """The relative path for the data directory is correctly retrieved."""
         module = utils.os.path.dirname(utils.__file__)
         rel_data = utils.os.path.join(module, utils.os.path.pardir,
-                                      utils.os.path.pardir, utils.DATA_SUFFIX)
+                                      utils.os.path.pardir,
+                                      utils.os.path.pardir,
+                                      utils.DATA_SUFFIX)
         expected_dir = utils.os.path.abspath(rel_data)
 
         # ensure expected_path exists at os level
 
=== added file 'ubuntuone/controlpanel/utils/tests/test_windows.py'
--- ubuntuone/controlpanel/utils/tests/test_windows.py	1970-01-01 00:00:00 +0000
+++ ubuntuone/controlpanel/utils/tests/test_windows.py	2011-11-15 13:13:25 +0000
@@ -0,0 +1,179 @@
+# -*- coding: utf-8 -*-
+
+#
+# Copyright 2011 Canonical Ltd.
+#
+# This program is free software: you can redistribute it and/or modify it
+# under the terms of the GNU General Public License version 3, as published
+# by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranties of
+# MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
+# PURPOSE.  See the GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License along
+# with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Test the windows utils functions."""
+
+import os
+
+from twisted.internet import defer
+
+from ubuntuone.controlpanel import utils
+from ubuntuone.controlpanel.tests import TestCase
+
+# let me use protected methods
+# pylint: disable=W0212
+
+
+class AutoupdaterTestCase(TestCase):
+    """Test the code that is used for the auto update process."""
+
+    @defer.inlineCallbacks
+    def setUp(self):
+        """Prepare for the diff tests."""
+        yield super(AutoupdaterTestCase, self).setUp()
+        self.auto_update_path = r'path\to\exe'
+        self.return_from_call = 0
+        self.command = None
+        self.args = []
+
+        def fake_execute_process(command, args=None, path=None):
+            """Fake async process execution."""
+            self.command = command
+            self.args = args
+            return self.return_from_call
+
+        self.patch(utils.windows, 'getProcessValue', fake_execute_process)
+        self.patch(utils.windows, '_get_update_path',
+                   lambda: self.auto_update_path)
+
+    @defer.inlineCallbacks
+    def test_are_updates_present_true(self):
+        """Test when updates are present."""
+        # the idea is simple, set the value to be returned from
+        # the fake call, assert that we get true and also that
+        # we did use the correct parameters.
+        self.return_from_call = 0
+        are_present = yield utils.are_updates_present()
+        self.assertTrue(are_present, 'Updates should be present.')
+        # lets assert that we did use the correct args
+        expected_args = ('--mode', 'unattended')
+        self.assertEqual(expected_args, self.args)
+        self.assertEqual(self.command, self.auto_update_path)
+
+    @defer.inlineCallbacks
+    def test_are_updates_present_false(self):
+        """Test when updates are not present."""
+        # similar to test_are_updates_present_true but with diff retcode
+        self.return_from_call = 40
+        are_present = yield utils.are_updates_present()
+        self.assertFalse(are_present, 'Updates should NOT be present.')
+        # lets assert that we did use the correct args
+        expected_args = ('--mode', 'unattended')
+        self.assertEqual(expected_args, self.args)
+        self.assertEqual(self.command, self.auto_update_path)
+
+    def test_perform_update(self):
+        """Test the method that performs the update."""
+        self.patch(utils.windows.win32api, 'ShellExecute', self._set_called)
+        utils.perform_update()
+        self.assertIn(self.auto_update_path, self._called[0][2])
+        self.assertEqual('runas', self._called[0][1])
+        self.assertEqual('--unattendedmodeui none', self._called[0][3])
+        self.assertEqual('', self._called[0][4])
+        self.assertEqual(0, self._called[0][5])
+
+
+class GetPathTestCase(TestCase):
+    """Test the code that is used for the auto update process."""
+
+    @defer.inlineCallbacks
+    def setUp(self):
+        """Set the different tests."""
+        yield super(GetPathTestCase, self).setUp()
+        self.called = []
+        self.exec_path = 'path/to/current/exe'
+        self.exists = True
+
+        def fake_abspath(path):
+            """Fake os.path.abspath."""
+            self.called.append('os.path.abspath')
+            return path
+
+        def fake_dirname(path):
+            """Fake os.path.dirname."""
+            self.called.append('os.path.dirname')
+            return path
+
+        def fake_exists(path):
+            """Fake os.path.exists."""
+            self.called.append('os.path.exists')
+            return self.exists
+
+        # patch the os.path functions used
+        self.patch(utils.windows.os.path, 'abspath', fake_abspath)
+        self.patch(utils.windows.os.path, 'dirname', fake_dirname)
+        self.patch(utils.windows.os.path, 'exists', fake_exists)
+
+    def _delete_frozen_state(self):
+        """Delete the frozen state."""
+        del utils.windows.sys.frozen
+        del utils.windows.sys.executable
+
+    def test_get_auto_update_path_frozen(self):
+        """Test the method used to get the autoupdate."""
+        # patch the diff parts of sys so that we get fake paths
+        is_frozen = hasattr(utils.windows.sys, 'frozen')
+        if not is_frozen:
+            utils.windows.sys.frozen = True
+            utils.windows.sys.executable = self.exec_path
+            self.addCleanup(self._delete_frozen_state)
+
+        # called method and assert that we have the correct result
+        path = utils.windows._get_update_path()
+        self.assertEqual(os.path.join(self.exec_path,
+                         utils.windows.AUTOUPDATE_EXE_NAME), path)
+        self.assertTrue('os.path.abspath' in self.called)
+        self.assertTrue('os.path.dirname' in self.called)
+        self.assertTrue('os.path.exists' in self.called)
+
+    def _reset_frozen_state(self, old_frozen, old_exec_path):
+        """Reset the frozen state."""
+        utils.windows.sys.frozen = old_frozen
+        utils.windows.sys.executable = old_exec_path
+
+    def _reset__file__(self, path):
+        """Reset the value of __file__."""
+        utils.windows.__file__ = path
+
+    def test_get_auto_update_path_not_frozen(self):
+        """Test the method used to get the autoupdate."""
+        is_frozen = hasattr(utils.windows.sys, 'frozen')
+        if is_frozen:
+            old_frozen = utils.windows.sys.frozen
+            old_exec_path = utils.windows.sys.executable
+            del utils.windows.sys.frozen
+            del utils.windows.sys.executable
+            self.addCleanup(self._reset_frozen_state, old_frozen,
+                            old_exec_path)
+        # set a fake __file__ for the module
+        old_file = utils.windows.__file__
+        utils.windows.__file__ = self.exec_path
+        self.addCleanup(self._reset__file__, old_file)
+
+        path = utils.windows._get_update_path()
+        self.assertEqual(os.path.join(self.exec_path,
+                         utils.windows.AUTOUPDATE_EXE_NAME), path)
+        self.assertEqual(2, self.called.count('os.path.dirname'))
+        self.assertTrue('os.path.exists' in self.called)
+
+    def test_get_auto_update_path_not_present(self):
+        """Test the method used to get the autoupdate."""
+        self.exists = False
+
+        # called method and assert that we have the correct result
+        path = utils.windows._get_update_path()
+        self.assertEqual(None, path)
 
=== added file 'ubuntuone/controlpanel/utils/windows.py'
--- ubuntuone/controlpanel/utils/windows.py	1970-01-01 00:00:00 +0000
+++ ubuntuone/controlpanel/utils/windows.py	2011-11-15 13:13:25 +0000
@@ -0,0 +1,73 @@
+# -*- coding: utf-8 -*-
+
+#
+# Copyright 2011 Canonical Ltd.
+#
+# This program is free software: you can redistribute it and/or modify it
+# under the terms of the GNU General Public License version 3, as published
+# by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranties of
+# MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
+# PURPOSE.  See the GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License along
+# with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Autoupdate functions for windows."""
+
+import os
+import sys
+
+# Avoid pylint error on Linux
+# pylint: disable=F0401
+import win32api
+# pylint: enable=F0401
+
+from twisted.internet import defer
+from twisted.internet.utils import getProcessValue
+
+from ubuntuone.controlpanel.logger import setup_logging
+
+logger = setup_logging('utils')
+AUTOUPDATE_EXE_NAME = 'autoupdate-windows.exe'
+
+
+def _get_update_path():
+    """Return the path in which the autoupdate command is found."""
+    if hasattr(sys, 'frozen'):
+        exec_path = os.path.abspath(sys.executable)
+    else:
+        exec_path = os.path.dirname(__file__)
+    folder = os.path.dirname(exec_path)
+    update_path = os.path.join(folder, AUTOUPDATE_EXE_NAME)
+    if os.path.exists(update_path):
+        return update_path
+    return None
+
+
[e-mail]
+def are_updates_present():
+    """Return if there are updates for Ubuntu One."""
+    result = False
+    retcode = None
+    update_path = _get_update_path()
+    if update_path is not None:
+        # If there is an update present we will get 0, non-zero otherwise
+        retcode = yield getProcessValue(update_path, args=('--mode',
+            'unattended'), path=os.path.dirname(update_path))
+        result = retcode == 0
+    logger.debug('are_updates_present: update path: %r, return code: %r,'
+                 ' result: %r', update_path, retcode, result)
+    defer.returnValue(result)
+
+
+def perform_update():
+    """Spawn the autoupdate process and call the stop function."""
+    update_path = _get_update_path()
+    if update_path is not None:
+        # lets call the updater with the commands that are required,
+        win32api.ShellExecute(None, 'runas',
+            update_path,
+            '--unattendedmodeui none', '', 0)