Merge lp:~facundo/magicicada-client/new-trash into lp:magicicada-client

[Facundo Batista]

Start using a new way to send to trash, going away from Gnome.

Start using a new way to send to trash, going away from Gnome.

Now we start using a package installed in a virtualenv, so I normalized some stuff around it:

- create it at bootstrap time, not everytime on every make test

- install packages listed in requirements-{devel}.txt, not hardcoded

- pinpointed flake8 in the requirements, and removed it as a system dependency

Also cleaned up the `run-tests` script:

- no more building when running tests, only at bootstrap

- run u1trial from inside the venv

- removed quotes around $MODULE, so we can do now

      ./run-tests -t TestClass.test_foo tests/test_bleh.py
Start using a new way to send to trash, going away from Gnome.

Now we start using a package installed in a virtualenv, so I normalized some stuff around it:

- create it at bootstrap time, not everytime on every make test

- install packages listed in requirements-{devel}.txt, not hardcoded

- pinpointed flake8 in the requirements, and removed it as a system dependency

Also cleaned up the `run-tests` script:

- no more building when running tests, only at bootstrap

- run u1trial from inside the venv

- removed quotes around $MODULE, so we can do now

      ./run-tests -t TestClass.test_foo tests/test_bleh.py

[Natalia Bidart (nataliabidart)]

Looks great! Thanks

review: Approve

=== modified file 'Makefile'
--- Makefile	2018-03-14 21:55:27 +0000
+++ Makefile	2018-03-20 19:57:57 +0000
@@ -44,16 +44,19 @@
 update-protocol:
 	cd $(PROTOCOL_DIR) && bzr pull && python setup.py build
 
-bootstrap: deps $(PROTOCOL_DIR) $(PROTOCOL_LINK) update-protocol
+bootstrap: deps $(PROTOCOL_DIR) $(PROTOCOL_LINK) update-protocol venv
+	$(ENV)/bin/python setup.py build
 
 docker-bootstrap: clean
 	cat dependencies.txt | xargs apt-get install -y --no-install-recommends
 	cat dependencies-devel.txt | xargs apt-get install -y --no-install-recommends
 	$(MAKE) $(PROTOCOL_DIR) $(PROTOCOL_LINK) update-protocol
 
+venv: 
+	virtualenv --system-site-packages $(ENV)
+	$(ENV)/bin/pip install -r requirements.txt -r requirements-devel.txt
+
 lint:
-	virtualenv $(ENV)
-	$(ENV)/bin/pip install flake8
 	$(ENV)/bin/flake8 --filename='*.py' --exclude='u1fsfsm.py,test_run_hello.py' ubuntuone
 
 test: lint

=== modified file 'dependencies-devel.txt'
--- dependencies-devel.txt	2016-08-14 19:52:30 +0000
+++ dependencies-devel.txt	2018-03-20 19:57:57 +0000
@@ -1,6 +1,5 @@
 bzr
 make
-python-flake8
 python-mocker
 ubuntuone-dev-tools
 virtualenv

=== added file 'requirements-devel.txt'
--- requirements-devel.txt	1970-01-01 00:00:00 +0000
+++ requirements-devel.txt	2018-03-20 19:57:57 +0000
@@ -0,0 +1,1 @@
+flake8==3.5.0

=== added file 'requirements.txt'
--- requirements.txt	1970-01-01 00:00:00 +0000
+++ requirements.txt	2018-03-20 19:57:57 +0000
@@ -0,0 +1,1 @@
+Send2Trash==1.5.0

=== modified file 'run-tests'
--- run-tests	2018-03-14 21:01:56 +0000
+++ run-tests	2018-03-20 19:57:57 +0000
@@ -28,10 +28,6 @@
 # version.  If you delete this exception statement from all source
 # files in the program, then also delete it here.
 
-# Allow alternative python executable via environment variable.  This is
-# useful for virtualenv testing.
-PYTHON=${PYTHON:-'python'}
-
 set -e
 if [ $# -ne 0 ]; then
     # run specific module given by the caller
@@ -42,7 +38,6 @@
 fi
 
 SYSNAME=`uname -s`
-
 if [ "$SYSNAME" == "Darwin" ]; then
     IGNORE_FILES="test_linux.py,test_windows.py"
     IGNORE_PATHS="ubuntuone/platform/tests/linux"
@@ -54,8 +49,5 @@
 
 echo "*** Running test suite for ""$MODULE"" ***"
 export SSL_CERTIFICATES_DIR=/etc/ssl/certs
-$PYTHON ./setup.py build
-u1trial -i "$IGNORE_FILES" -p "$IGNORE_PATHS" "$MODULE"
-$PYTHON ./setup.py clean
+.env/bin/python /usr/bin/u1trial -i "$IGNORE_FILES" -p "$IGNORE_PATHS" $MODULE
 rm -rf _trial_temp
-rm -rf build

=== modified file 'ubuntuone/platform/os_helper/linux.py'
--- ubuntuone/platform/os_helper/linux.py	2013-02-12 23:15:48 +0000
+++ ubuntuone/platform/os_helper/linux.py	2018-03-20 19:57:57 +0000
@@ -31,17 +31,17 @@
 to support the linux platform.
 """
 
-import errno
 import logging
 import os
 import shutil
 
 try:
-    from gi.repository import Gio, GLib
+    from gi.repository import GObject
     has_gi = True
 except ImportError:
-    import gio
+    import gobject
     has_gi = False
+from send2trash import send2trash
 
 from ubuntuone.platform.os_helper import unix
 
@@ -50,44 +50,32 @@
 logger = logging.getLogger('ubuntuone.SyncDaemon')
 
 
+def _remove_path(path):
+    """Remove the path, no matter if file or dir structure."""
+    if os.path.isdir(path):
+        shutil.rmtree(path)
+    else:
+        os.remove(path)
+
+
 def move_to_trash(path):
     """Move the file or dir to trash.
 
     If had any error, or the system can't do it, just remove it.
     """
-    if has_gi:
-        not_supported = Gio.IOErrorEnum.NOT_SUPPORTED
-        try:
-            return_code = Gio.File.new_for_path(path).trash(None)
-        except GLib.GError:
-            exc = OSError()
-            exc.errno = errno.ENOENT
-            raise exc
-    else:
-        not_supported = gio.ERROR_NOT_SUPPORTED
-        try:
-            return_code = gio.File(path).trash()
-        except gio.Error:
-            exc = OSError()
-            exc.errno = errno.ENOENT
-            raise exc
-
-    if not return_code or return_code == not_supported:
-        logger.warning("Problems moving to trash! (%s) Removing anyway: %r",
-                       return_code, path)
-        if os.path.isdir(path):
-            shutil.rmtree(path)
-        else:
-            os.remove(path)
+    try:
+        send2trash(path)
+    except OSError as exc:
+        logger.warning(
+            "Problems moving to trash! (%s) Removing anyway: %r", exc, path)
+        _remove_path(path)
 
 
 def set_application_name(app_name):
     """Set the name of the application."""
     if has_gi:
-        from gi.repository import GObject
         GObject.set_application_name(app_name)
     else:
-        import gobject
         gobject.set_application_name(app_name)
 
 

=== modified file 'ubuntuone/platform/tests/os_helper/test_linux.py'
--- ubuntuone/platform/tests/os_helper/test_linux.py	2016-05-29 19:15:01 +0000
+++ ubuntuone/platform/tests/os_helper/test_linux.py	2018-03-20 19:57:57 +0000
@@ -29,17 +29,11 @@
 import logging
 import os
 
-try:
-    from gi.repository import Gio as gio
-    GIO_NOT_SUPPORTED = gio.IOErrorEnum.NOT_SUPPORTED
-except ImportError:
-    import gio
-    GIO_NOT_SUPPORTED = gio.ERROR_NOT_SUPPORTED
-
 from twisted.internet import defer
 from ubuntuone.devtools.handlers import MementoHandler
 
 from ubuntuone.platform.tests.os_helper import test_os_helper
+from ubuntuone.platform.os_helper import linux
 from ubuntuone.platform import (
     move_to_trash,
     open_file,
@@ -47,24 +41,6 @@
 )
 
 
-class FakeGIOFile(object):
-    """Fake File for gio."""
-
-    _bad_trash_call = None
-
-    def __init__(self, path):
-        pass
-
-    @classmethod
-    def new_for_path(klass, path):
-        """Fake new_for_path for GI."""
-        return klass(path)
-
-    def trash(self, *args):
-        """Fake trash call."""
-        return self._bad_trash_call
-
-
 class OSWrapperTests(test_os_helper.OSWrapperTests):
     """Tests for os wrapper functions."""
 
@@ -90,50 +66,31 @@
         self.assertNotEqual(os.stat(link).st_ino, stat_path(link).st_ino)
         self.assertEqual(os.lstat(link).st_ino, stat_path(link).st_ino)
 
-    def test_movetotrash_file_bad(self):
-        """Something bad happen when moving to trash, removed anyway."""
-        FakeGIOFile._bad_trash_call = False   # error
-        self.patch(gio, "File", FakeGIOFile)
-        path = os.path.join(self.basedir, 'foo')
-        open_file(path, 'w').close()
-        move_to_trash(path)
-        self.assertFalse(os.path.exists(path))
-        self.assertTrue(self.handler.check_warning("Problems moving to trash!",
-                                                   "Removing anyway", "foo"))
-
-    def test_movetotrash_dir_bad(self):
-        """Something bad happen when moving to trash, removed anyway."""
-        FakeGIOFile._bad_trash_call = False   # error
-        self.patch(gio, "File", FakeGIOFile)
-        path = os.path.join(self.basedir, 'foo')
-        os.mkdir(path)
-        open_file(os.path.join(path, 'file inside directory'), 'w').close()
-        move_to_trash(path)
-        self.assertFalse(os.path.exists(path))
-        self.assertTrue(self.handler.check_warning("Problems moving to trash!",
-                                                   "Removing anyway", "foo"))
-
-    def test_movetotrash_file_systemnotcapable(self):
-        """The system is not capable of moving into trash."""
-        FakeGIOFile._bad_trash_call = GIO_NOT_SUPPORTED
-        self.patch(gio, "File", FakeGIOFile)
-        path = os.path.join(self.basedir, 'foo')
-        open_file(path, 'w').close()
-        move_to_trash(path)
-        self.assertFalse(os.path.exists(path))
-        self.assertTrue(self.handler.check_warning("Problems moving to trash!",
-                                                   "Removing anyway", "foo",
-                                                   "ERROR_NOT_SUPPORTED"))
-
-    def test_movetotrash_dir_systemnotcapable(self):
-        """The system is not capable of moving into trash."""
-        FakeGIOFile._bad_trash_call = GIO_NOT_SUPPORTED
-        self.patch(gio, "File", FakeGIOFile)
-        path = os.path.join(self.basedir, 'foo')
-        os.mkdir(path)
-        open_file(os.path.join(path, 'file inside directory'), 'w').close()
-        move_to_trash(path)
-        self.assertFalse(os.path.exists(path))
-        self.assertTrue(self.handler.check_warning("Problems moving to trash!",
-                                                   "Removing anyway", "foo",
-                                                   "ERROR_NOT_SUPPORTED"))
+    def test_movetotrash_bad(self):
+        """Something bad happen when moving to trash, removed anyway.
+
+        Simulating this as giving a non-existant path to the function, which
+        will make it fail with OSError, which is the exception the send2trash
+        library raises on any problem.
+        """
+        called = []
+        self.patch(linux, '_remove_path', lambda p: called.append(p))
+
+        path = os.path.join(self.basedir, 'non-existant')
+        move_to_trash(path)
+        self.assertEqual(called[0], path)
+        self.assertTrue(self.handler.check_warning(
+            "Problems moving to trash!", "Removing anyway", path))
+
+    def test_remove_path_file(self):
+        path = os.path.join(self.basedir, 'foo')
+        open_file(path, 'w').close()
+        linux._remove_path(path)
+        self.assertFalse(os.path.exists(path))
+
+    def test_remove_path_dir(self):
+        path = os.path.join(self.basedir, 'foo')
+        os.mkdir(path)
+        open_file(os.path.join(path, 'file inside directory'), 'w').close()
+        linux._remove_path(path)
+        self.assertFalse(os.path.exists(path))