From 9d82e3ac28890554f063aae92ac6bcb7759dcdee Mon Sep 17 00:00:00 2001
From: Raphael Marvie <raphael@softosapiens.fr>
Date: Fri, 5 Oct 2018 10:25:18 +0200
Subject: [PATCH] Testing utils.filesystem.rm

- removing existing file with proper permissions
- removing existing folder with proper permissions

- non existant or no permissions prevent removing file
- non existant or no permissions prevent removing folder

Fixed potential OSError handling from shutil.rmtree
---
 moulinette/utils/filesystem.py            | 19 +++---
 notes.txt                                 |  4 ++
 tests/moulinette/utils/test_filesystem.py | 83 +++++++++++++++++++++++
 3 files changed, 98 insertions(+), 8 deletions(-)

diff --git a/moulinette/utils/filesystem.py b/moulinette/utils/filesystem.py
index 8f429f1c..c86d65ed 100644
--- a/moulinette/utils/filesystem.py
+++ b/moulinette/utils/filesystem.py
@@ -1,9 +1,12 @@
+# encoding: utf-8
+
 import os
 import yaml
 import errno
 import shutil
 import json
 import grp
+
 from pwd import getpwnam
 
 from moulinette import m18n
@@ -278,13 +281,13 @@ def rm(path, recursive=False, force=False):
         - force -- Ignore nonexistent files
 
     """
-    if recursive and os.path.isdir(path):
-        shutil.rmtree(path, ignore_errors=force)
-    else:
-        try:
+    try:
+        if recursive and os.path.isdir(path):
+            shutil.rmtree(path, ignore_errors=force)
+        else:
             os.remove(path)
-        except OSError as e:
-            if not force:
-                raise MoulinetteError(errno.EIO,
-                                      m18n.g('error_removing',
+    except OSError as e:
+        if not force:
+            raise MoulinetteError(errno.EIO,
+                                  m18n.g('error_removing',
                                              path=path, error=str(e)))
diff --git a/notes.txt b/notes.txt
index 2f5978f5..1807d2bf 100644
--- a/notes.txt
+++ b/notes.txt
@@ -21,3 +21,7 @@
 
 # renamed multiprocessing.process.Process to BaseProcess
 
+# writing tests for utils (only ones available) using mocks
+
+the goal is to be able to run moulinette tests on any environment just
+after a git clone
diff --git a/tests/moulinette/utils/test_filesystem.py b/tests/moulinette/utils/test_filesystem.py
index c1fe5e9d..e8a9e1e5 100644
--- a/tests/moulinette/utils/test_filesystem.py
+++ b/tests/moulinette/utils/test_filesystem.py
@@ -2,6 +2,11 @@
 
 """
 Testing moulinette utils filesystem
+
+
+WARNING These tests have been written based on the actual implementation and
+thus are dependant on this implementation. This is fragile but allow for a
+first instroduction of tests before doing refactorings.
 """
 
 import mock
@@ -276,6 +281,84 @@ def fake_open_for_write():
             fake_file)
 
 
+########################################################################
+# Test file remove
+########################################################################
+
+#
+# Removing a file
+
+@mock.patch('os.remove')
+def test_rm_remove_file_if_it_exists(remove):
+    filename = 'file.txt'
+
+    filesystem.rm(filename)
+
+    remove.assert_called_with(filename)
+
+
+@mock.patch('os.remove')
+def test_rm_cannot_remove_non_existing_file(remove):
+    filename = 'do_not_exist.txt'
+    remove.side_effect = FileNotFoundError()
+
+    with pytest.raises(MoulinetteError):
+        filesystem.rm(filename)
+
+
+@mock.patch('os.remove')
+def test_rm_cannot_remove_file_without_permission(remove):
+    filename = 'not_mine.txt'
+    remove.side_effect = PermissionError()
+
+    with pytest.raises(MoulinetteError):
+        filesystem.rm(filename)
+
+
+@mock.patch('os.remove')
+def test_rm_cannot_remove_folder(remove):
+    filename = './folder'
+    remove.side_effect = IsADirectoryError()
+
+    with pytest.raises(MoulinetteError):
+        filesystem.rm(filename)
+
+#
+# Removing a folder
+
+@mock.patch('os.path.isdir')
+@mock.patch('shutil.rmtree')
+def test_rm_remove_folder_if_it_exists(rmtree, isdir):
+    isdir.return_value = True
+    foldername = 'folder'
+
+    filesystem.rm(foldername, recursive=True)
+
+    rmtree.assert_called_with(foldername, ignore_errors=False)
+
+
+@mock.patch('os.path.isdir')
+@mock.patch('os.remove')
+def test_rm_cannot_remove_non_existing_folder(remove, isdir):
+    isdir.return_value = False
+    foldername = 'do_not_exist'
+    remove.side_effect = FileNotFoundError()
+
+    with pytest.raises(MoulinetteError):
+        filesystem.rm(foldername, recursive=True)
+
+
+@mock.patch('os.path.isdir')
+@mock.patch('shutil.rmtree')
+def test_rm_cannot_remove_folder_without_permission(rmtree, isdir):
+    isdir.return_value = True
+    foldername = 'not_mine'
+    rmtree.side_effect = PermissionError()
+
+    with pytest.raises(MoulinetteError):
+        filesystem.rm(foldername, recursive=True)
+
+
 ################################################################################
 ##   Test file remove                                                          #
 ################################################################################