POSIX: fixed removal of symlinked directories

3 files changed, 94 insertions, 90 deletions

diff --git a/testsuite/Testsrc/Testlib/TestClient/TestTools/TestPOSIX/TestDirectory.py b/testsuite/Testsrc/Testlib/TestClient/TestTools/TestPOSIX/TestDirectory.py
index 6dd130bee..16490808e 100644
--- a/testsuite/Testsrc/Testlib/TestClient/TestTools/TestPOSIX/TestDirectory.py
+++ b/testsuite/Testsrc/Testlib/TestClient/TestTools/TestPOSIX/TestDirectory.py
@@ -83,34 +83,35 @@ class TestPOSIXDirectory(TestPOSIXTool):
         mock_verify.assert_called_with(self.ptool, entry, modlist)
         mock_listdir.assert_called_with(entry.get("name"))
         self.assertEqual(len(entry.findall("Prune")), 0)
-    
+
     @patch("os.unlink")
-    @patch("os.path.isdir")
-    @patch("shutil.rmtree")
     @patch("Bcfg2.Client.Tools.POSIX.base.POSIXTool.install")
     @patch("Bcfg2.Client.Tools.POSIX.Directory.%s._exists" % test_obj.__name__)
     @patch("Bcfg2.Client.Tools.POSIX.Directory.%s._makedirs" %
            test_obj.__name__)
     def test_install(self, mock_makedirs, mock_exists, mock_install,
-                     mock_rmtree, mock_isdir, mock_unlink):
+                     mock_unlink):
         entry = lxml.etree.Element("Path", name="/test/foo/bar",
                                    type="directory", mode='0644',
                                    owner='root', group='root')
-        
+
+        self.ptool._makedirs = Mock()
+        self.ptool._remove = Mock()
+
         def reset():
             mock_exists.reset_mock()
             mock_install.reset_mock()
             mock_unlink.reset_mock()
-            mock_rmtree.reset_mock()
-            mock_rmtree.mock_makedirs()
+            self.ptool._makedirs.reset_mock()
+            self.ptool._remove.reset_mock()
 
-        mock_makedirs.return_value = True
+        self.ptool._makedirs.return_value = True
         mock_exists.return_value = False
         mock_install.return_value = True
         self.assertTrue(self.ptool.install(entry))
         mock_exists.assert_called_with(entry)
         mock_install.assert_called_with(self.ptool, entry)
-        mock_makedirs.assert_called_with(entry)
+        self.ptool._makedirs.assert_called_with(entry)
 
         reset()
         exists_rv = MagicMock()
@@ -119,7 +120,7 @@ class TestPOSIXDirectory(TestPOSIXTool):
         self.assertTrue(self.ptool.install(entry))
         mock_unlink.assert_called_with(entry.get("name"))
         mock_exists.assert_called_with(entry)
-        mock_makedirs.assert_called_with(entry)
+        self.ptool._makedirs.assert_called_with(entry)
         mock_install.assert_called_with(self.ptool, entry)
 
         reset()
@@ -138,20 +139,13 @@ class TestPOSIXDirectory(TestPOSIXTool):
         prune = ["/test/foo/bar/prune1", "/test/foo/bar/prune2"]
         for path in prune:
             lxml.etree.SubElement(entry, "Prune", path=path)
-        
+
         reset()
         mock_install.return_value = True
 
-        def isdir_rv(path):
-            if path.endswith("prune2"):
-                return True
-            else:
-                return False
-        mock_isdir.side_effect = isdir_rv
         self.assertTrue(self.ptool.install(entry))
         mock_exists.assert_called_with(entry)
         mock_install.assert_called_with(self.ptool, entry)
-        self.assertItemsEqual(mock_isdir.call_args_list,
-                              [call(p) for p in prune])
-        mock_unlink.assert_called_with("/test/foo/bar/prune1")
-        mock_rmtree.assert_called_with("/test/foo/bar/prune2")
+        self.assertItemsEqual([c[0][0].get("path")
+                               for c in self.ptool._remove.call_args_list],
+                              prune)
diff --git a/testsuite/Testsrc/Testlib/TestClient/TestTools/TestPOSIX/TestNonexistent.py b/testsuite/Testsrc/Testlib/TestClient/TestTools/TestPOSIX/TestNonexistent.py
index 676b18f5d..43242afb7 100644
--- a/testsuite/Testsrc/Testlib/TestClient/TestTools/TestPOSIX/TestNonexistent.py
+++ b/testsuite/Testsrc/Testlib/TestClient/TestTools/TestPOSIX/TestNonexistent.py
@@ -18,6 +18,7 @@ from Test__init import get_config, get_posix_object
 from Testbase import TestPOSIXTool
 from common import *
 
+
 class TestPOSIXNonexistent(TestPOSIXTool):
     test_obj = POSIXNonexistent
 
@@ -31,59 +32,41 @@ class TestPOSIXNonexistent(TestPOSIXTool):
             self.assertEqual(self.ptool.verify(entry, []), not val)
             mock_lexists.assert_called_with(entry.get("name"))
 
-    @patch("os.rmdir")
-    @patch("os.remove")
-    @patch("os.path.isdir")
-    @patch("shutil.rmtree")
-    def test_install(self, mock_rmtree, mock_isdir, mock_remove, mock_rmdir):
+    def test_install(self):
         entry = lxml.etree.Element("Path", name="/test", type="nonexistent")
 
-        def reset():
-            mock_isdir.reset_mock()
-            mock_remove.reset_mock()
-            mock_rmdir.reset_mock()
-            mock_rmtree.reset_mock()
+        self.ptool._remove = Mock()
 
-        mock_isdir.return_value = False
-        self.assertTrue(self.ptool.install(entry))
-        mock_remove.assert_called_with(entry.get("name"))
-
-        reset()
-        mock_remove.side_effect = OSError
-        self.assertFalse(self.ptool.install(entry))
-        mock_remove.assert_called_with(entry.get("name"))
+        def reset():
+            self.ptool._remove.reset_mock()
 
-        reset()
-        mock_isdir.return_value = True
         self.assertTrue(self.ptool.install(entry))
-        mock_rmdir.assert_called_with(entry.get("name"))
-
-        reset()
-        mock_rmdir.side_effect = OSError
-        self.assertFalse(self.ptool.install(entry))
-        mock_rmdir.assert_called_with(entry.get("name"))
+        self.ptool._remove.assert_called_with(entry, recursive=False)
 
         reset()
         entry.set("recursive", "true")
         self.assertTrue(self.ptool.install(entry))
-        mock_rmtree.assert_called_with(entry.get("name"))
-
-        reset()
-        mock_rmtree.side_effect = OSError
-        self.assertFalse(self.ptool.install(entry))
-        mock_rmtree.assert_called_with(entry.get("name"))
+        self.ptool._remove.assert_called_with(entry, recursive=True)
 
+        print "about to start"
         reset()
         child_entry = lxml.etree.Element("Path", name="/test/foo",
                                          type="nonexistent")
         ptool = self.get_obj(posix=get_posix_object(config=get_config([child_entry])))
-        mock_rmtree.side_effect = None
+        ptool._remove = Mock()
         self.assertTrue(ptool.install(entry))
-        mock_rmtree.assert_called_with(entry.get("name"))
+        ptool._remove.assert_called_with(entry, recursive=True)
 
         reset()
         child_entry = lxml.etree.Element("Path", name="/test/foo",
                                          type="file")
         ptool = self.get_obj(posix=get_posix_object(config=get_config([child_entry])))
-        mock_rmtree.side_effect = None
+        ptool._remove = Mock()
         self.assertFalse(ptool.install(entry))
+        self.assertFalse(ptool._remove.called)
+
+        reset()
+        entry.set("recursive", "false")
+        self.ptool._remove.side_effect = OSError
+        self.assertFalse(self.ptool.install(entry))
+        self.ptool._remove.assert_called_with(entry, recursive=False)
diff --git a/testsuite/Testsrc/Testlib/TestClient/TestTools/TestPOSIX/Testbase.py b/testsuite/Testsrc/Testlib/TestClient/TestTools/TestPOSIX/Testbase.py
index ec194d401..b3599db83 100644
--- a/testsuite/Testsrc/Testlib/TestClient/TestTools/TestPOSIX/Testbase.py
+++ b/testsuite/Testsrc/Testlib/TestClient/TestTools/TestPOSIX/Testbase.py
@@ -123,57 +123,84 @@ class TestPOSIXTool(Bcfg2TestCase):
         mock_walk.assert_called_with(entry.get("name"))
         self.assertItemsEqual(mock_set_perms.call_args_list, all_set_perms)
 
+    @patch('os.rmdir')
+    @patch('os.unlink')
+    @patch('shutil.rmtree')
+    @patch('os.path.isdir')
+    @patch('os.path.islink')
+    def test_remove(self, mock_islink, mock_isdir, mock_rmtree, mock_unlink,
+                    mock_rmdir):
+        entry = lxml.etree.Element("Path", name="/etc/foo")
+
+        def reset():
+            mock_islink.reset_mock()
+            mock_isdir.reset_mock()
+            mock_rmtree.reset_mock()
+            mock_unlink.reset_mock()
+            mock_rmdir.reset_mock()
+
+        mock_islink.return_value = True
+        mock_isdir.return_value = False
+        self.ptool._remove(entry)
+        mock_unlink.assert_called_with(entry.get('name'))
+        self.assertFalse(mock_rmtree.called)
+        self.assertFalse(mock_rmdir.called)
+
+        reset()
+        mock_islink.return_value = False
+        mock_isdir.return_value = True
+        self.ptool._remove(entry)
+        mock_rmtree.assert_called_with(entry.get('name'))
+        self.assertFalse(mock_unlink.called)
+        self.assertFalse(mock_rmdir.called)
+
+        reset()
+        self.ptool._remove(entry, recursive=False)
+        mock_rmdir.assert_called_with(entry.get('name'))
+        self.assertFalse(mock_unlink.called)
+        self.assertFalse(mock_rmtree.called)
+
+        reset()
+        mock_islink.return_value = False
+        mock_isdir.return_value = False
+        self.ptool._remove(entry, recursive=False)
+        mock_unlink.assert_called_with(entry.get('name'))
+        self.assertFalse(mock_rmtree.called)
+        self.assertFalse(mock_rmdir.called)
+
     @patch('os.lstat')
-    @patch("os.unlink")
-    @patch("os.path.isdir")
-    @patch("shutil.rmtree")
-    def test_exists(self, mock_rmtree, mock_isdir, mock_unlink, mock_lstat):
+    def test_exists(self, mock_lstat):
         entry = lxml.etree.Element("Path", name="/etc/foo", type="file")
 
+        self.ptool._remove = Mock()
+
+        def reset():
+            mock_lstat.reset_mock()
+            self.ptool._remove.reset_mock()
+
         mock_lstat.side_effect = OSError
         self.assertFalse(self.ptool._exists(entry))
         mock_lstat.assert_called_with(entry.get('name'))
-        self.assertFalse(mock_unlink.called)
+        self.assertFalse(self.ptool._remove.called)
 
-        mock_lstat.reset_mock()
-        mock_unlink.reset_mock()
+        reset()
         rv = MagicMock()
         mock_lstat.return_value = rv
         mock_lstat.side_effect = None
         self.assertEqual(self.ptool._exists(entry), rv)
         mock_lstat.assert_called_with(entry.get('name'))
-        self.assertFalse(mock_unlink.called)
-
-        mock_lstat.reset_mock()
-        mock_unlink.reset_mock()
-        mock_isdir.return_value = False
-        self.assertFalse(self.ptool._exists(entry, remove=True))
-        mock_isdir.assert_called_with(entry.get('name'))
-        mock_lstat.assert_called_with(entry.get('name'))
-        mock_unlink.assert_called_with(entry.get('name'))
-        self.assertFalse(mock_rmtree.called)
+        self.assertFalse(self.ptool._remove.called)
 
-        mock_lstat.reset_mock()
-        mock_isdir.reset_mock()
-        mock_unlink.reset_mock()
-        mock_rmtree.reset_mock()
-        mock_isdir.return_value = True
-        self.assertFalse(self.ptool._exists(entry, remove=True))
-        mock_isdir.assert_called_with(entry.get('name'))
+        reset()
+        self.assertEqual(self.ptool._exists(entry, remove=True), None)
         mock_lstat.assert_called_with(entry.get('name'))
-        mock_rmtree.assert_called_with(entry.get('name'))
-        self.assertFalse(mock_unlink.called)
+        self.ptool._remove.assert_called_with(entry)
 
-        mock_isdir.reset_mock()
-        mock_lstat.reset_mock()
-        mock_unlink.reset_mock()
-        mock_rmtree.reset_mock()
-        mock_rmtree.side_effect = OSError
+        reset()
+        self.ptool._remove.side_effect = OSError
         self.assertEqual(self.ptool._exists(entry, remove=True), rv)
-        mock_isdir.assert_called_with(entry.get('name'))
         mock_lstat.assert_called_with(entry.get('name'))
-        mock_rmtree.assert_called_with(entry.get('name'))
-        self.assertFalse(mock_unlink.called)
+        self.ptool._remove.assert_called_with(entry)
 
     @patch("os.chown")
     @patch("os.chmod")