Handle FAM monitor failures more gracefully:

* Where possible, create the file or directory that is about to be
  monitored.  This ensures that content can be added later without
  need to restart Bcfg2.  (Otherwise, adding the monitor would fail,
  and so when you did create the file in question, bcfg2-server would
  never be notified of it.)
* When not possible, give better error messages.

7 files changed, 99 insertions, 44 deletions

diff --git a/testsuite/Testsrc/Testlib/TestServer/TestPlugin/Testbase.py b/testsuite/Testsrc/Testlib/TestServer/TestPlugin/Testbase.py
index a1e624824..318f5ceaa 100644
--- a/testsuite/Testsrc/Testlib/TestServer/TestPlugin/Testbase.py
+++ b/testsuite/Testsrc/Testlib/TestServer/TestPlugin/Testbase.py
@@ -72,14 +72,32 @@ class TestPlugin(TestDebuggable):
         if core is None:
             core = Mock()
             core.setup = MagicMock()
-        return self.test_obj(core, datastore)
+        @patchIf(not isinstance(os.makedirs, Mock), "os.makedirs", Mock())
+        def inner():
+            return self.test_obj(core, datastore)
+        return inner()
 
-    def test__init(self):
+    @patch("os.makedirs")
+    @patch("os.path.exists")
+    def test__init(self, mock_exists, mock_makedirs):
         core = Mock()
         core.setup = MagicMock()
+
+        mock_exists.return_value = True
+        p = self.get_obj(core=core)
+        self.assertEqual(p.data, os.path.join(datastore, p.name))
+        self.assertEqual(p.core, core)
+        mock_exists.assert_any_call(p.data)
+        self.assertFalse(mock_makedirs.called)
+
+        mock_exists.reset_mock()
+        mock_makedirs.reset_mock()
+        mock_exists.return_value = False
         p = self.get_obj(core=core)
         self.assertEqual(p.data, os.path.join(datastore, p.name))
         self.assertEqual(p.core, core)
+        mock_exists.assert_any_call(p.data)
+        mock_makedirs.assert_any_call(p.data)
 
     @patch("os.makedirs")
     def test_init_repo(self, mock_makedirs):
diff --git a/testsuite/Testsrc/Testlib/TestServer/TestPlugin/Testhelpers.py b/testsuite/Testsrc/Testlib/TestServer/TestPlugin/Testhelpers.py
index fb51eb1fe..fceddcc69 100644
--- a/testsuite/Testsrc/Testlib/TestServer/TestPlugin/Testhelpers.py
+++ b/testsuite/Testsrc/Testlib/TestServer/TestPlugin/Testhelpers.py
@@ -158,6 +158,7 @@ class TestDirectoryBacked(Bcfg2TestCase):
         """ ensure that the child object has the correct interface """
         self.assertTrue(hasattr(self.test_obj.__child__, "HandleEvent"))
 
+    @patch("os.makedirs", Mock())
     def get_obj(self, fam=None):
         if fam is None:
             fam = Mock()
@@ -171,12 +172,26 @@ class TestDirectoryBacked(Bcfg2TestCase):
                                  fam)
         return inner()
 
-    def test__init(self):
+    @patch("os.makedirs")
+    @patch("os.path.exists")
+    def test__init(self, mock_exists, mock_makedirs):
         @patch("%s.%s.add_directory_monitor" % (self.test_obj.__module__,
                                                 self.test_obj.__name__))
         def inner(mock_add_monitor):
+            mock_exists.return_value = True
+            db = self.test_obj(datastore, Mock())
+            mock_add_monitor.assert_called_with('')
+            mock_exists.assert_called_with(db.data)
+            self.assertFalse(mock_makedirs.called)
+
+            mock_add_monitor.reset_mock()
+            mock_exists.reset_mock()
+            mock_makedirs.reset_mock()
+            mock_exists.return_value = False
             db = self.test_obj(datastore, Mock())
             mock_add_monitor.assert_called_with('')
+            mock_exists.assert_called_with(db.data)
+            mock_makedirs.assert_called_with(db.data)
 
         inner()
 
@@ -220,6 +235,7 @@ class TestDirectoryBacked(Bcfg2TestCase):
         mock_isdir.return_value = True
         for path in self.testpaths.values():
             reset()
+            print "testing %s" % path
             db.add_directory_monitor(path)
             db.fam.AddMonitor.assert_called_with(os.path.join(db.data, path),
                                                  db)
@@ -395,10 +411,14 @@ class TestXMLFileBacked(TestFileBacked):
     should_monitor = None
     path = os.path.join(datastore, "test", "test1.xml")
 
+    @patch("os.makedirs", Mock())
     def get_obj(self, path=None, fam=None, should_monitor=False):
         if path is None:
             path = self.path
-        return self.test_obj(path, fam=fam, should_monitor=should_monitor)
+        @patchIf(not isinstance(os.makedirs, Mock), "os.makedirs", Mock())
+        def inner():
+            return self.test_obj(path, fam=fam, should_monitor=should_monitor)
+        return inner()
 
     def test__init(self):
         fam = Mock()
@@ -1186,13 +1206,18 @@ class TestXMLDirectoryBacked(TestDirectoryBacked):
 class TestPrioDir(TestPlugin, TestGenerator, TestXMLDirectoryBacked):
     test_obj = PrioDir
 
-    @patch("Bcfg2.Server.Plugin.helpers.%s.add_directory_monitor" %
-           test_obj.__name__,
-           Mock())
     def get_obj(self, core=None):
         if core is None:
             core = Mock()
-        return self.test_obj(core, datastore)
+
+        @patch("%s.%s.add_directory_monitor" %
+               (self.test_obj.__module__, self.test_obj.__name__),
+               Mock())
+        @patchIf(not isinstance(os.makedirs, Mock), "os.makedirs", Mock())
+        def inner():
+            return self.test_obj(core, datastore)
+
+        return inner()
 
     def test_HandleEvent(self):
         TestXMLDirectoryBacked.test_HandleEvent(self)
@@ -1816,6 +1841,7 @@ class TestGroupSpool(TestPlugin, TestGenerator):
         return inner()
 
     def test__init(self):
+        @patchIf(not isinstance(os.makedirs, Mock), "os.makedirs", Mock())
         @patch("%s.%s.AddDirectoryMonitor" % (self.test_obj.__module__,
                                               self.test_obj.__name__))
         def inner(mock_Add):
diff --git a/testsuite/Testsrc/Testlib/TestServer/TestPlugin/Testinterfaces.py b/testsuite/Testsrc/Testlib/TestServer/TestPlugin/Testinterfaces.py
index 35f4e0700..1f5c4790b 100644
--- a/testsuite/Testsrc/Testlib/TestServer/TestPlugin/Testinterfaces.py
+++ b/testsuite/Testsrc/Testlib/TestServer/TestPlugin/Testinterfaces.py
@@ -97,11 +97,6 @@ class TestProbing(Bcfg2TestCase):
 class TestStatistics(TestPlugin):
     test_obj = Statistics
 
-    def get_obj(self, core=None):
-        if core is None:
-            core = Mock()
-        return self.test_obj(core, datastore)
-
     def test_process_statistics(self):
         s = self.get_obj()
         self.assertRaises(NotImplementedError,
@@ -354,12 +349,6 @@ class TestGoalValidator(Bcfg2TestCase):
 class TestVersion(TestPlugin):
     test_obj = Version
 
-    def get_obj(self, core=None):
-        if core is None:
-            core = Mock()
-            core.setup = MagicMock()
-        return self.test_obj(core, datastore)
-
     def test_get_revision(self):
         d = self.get_obj()
         self.assertRaises(NotImplementedError, d.get_revision)
diff --git a/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestGroupPatterns.py b/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestGroupPatterns.py
index a9346156c..c6e6f5ef7 100644
--- a/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestGroupPatterns.py
+++ b/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestGroupPatterns.py
@@ -92,7 +92,12 @@ class TestPatternFile(TestXMLFileBacked):
             core.fam = fam
         elif not core:
             core = Mock()
-        return self.test_obj(path, core=core)
+
+        @patchIf(not isinstance(lxml.etree.Element, Mock),
+                 "lxml.etree.Element", Mock())
+        def inner():
+            return self.test_obj(path, core=core)
+        return inner()
 
     @patch("Bcfg2.Server.Plugins.GroupPatterns.PatternMap")
     def test_Index(self, mock_PatternMap):
@@ -135,6 +140,14 @@ class TestPatternFile(TestXMLFileBacked):
 class TestGroupPatterns(TestPlugin, TestConnector):
     test_obj = GroupPatterns
 
+    def get_obj(self, core=None):
+        @patchIf(not isinstance(lxml.etree.Element, Mock),
+                 "lxml.etree.Element", Mock())
+        def inner():
+            return TestPlugin.get_obj(self, core=core)
+        return inner()
+
+
     def test_get_additional_groups(self):
         gp = self.get_obj()
         gp.config = Mock()
diff --git a/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestMetadata.py b/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestMetadata.py
index 69ea45de6..742946c42 100644
--- a/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestMetadata.py
+++ b/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestMetadata.py
@@ -94,7 +94,13 @@ def get_metadata_object(core=None, watch_clients=False, use_db=False):
         core.setup = MagicMock()
         core.metadata_cache = MagicMock()
     core.setup.cfp.getboolean = Mock(return_value=use_db)
-    return Metadata(core, datastore, watch_clients=watch_clients)
+
+    @patchIf(not isinstance(os.makedirs, Mock), "os.makedirs", Mock())
+    @patchIf(not isinstance(lxml.etree.Element, Mock),
+             "lxml.etree.Element", Mock())
+    def inner():
+        return Metadata(core, datastore, watch_clients=watch_clients)
+    return inner()
 
 
 class TestMetadataDB(DBModelTestCase):
@@ -203,7 +209,11 @@ class TestXMLMetadataConfig(TestXMLFileBacked):
     def get_obj(self, basefile="clients.xml", core=None, watch_clients=False):
         self.metadata = get_metadata_object(core=core,
                                             watch_clients=watch_clients)
-        return XMLMetadataConfig(self.metadata, watch_clients, basefile)
+        @patchIf(not isinstance(lxml.etree.Element, Mock),
+                 "lxml.etree.Element", Mock())
+        def inner():
+            return XMLMetadataConfig(self.metadata, watch_clients, basefile)
+        return inner()
 
     def test__init(self):
         xmc = self.get_obj()
@@ -1521,7 +1531,11 @@ class TestMetadata_ClientsXML(TestMetadataBase):
         if metadata is None:
             metadata = self.get_obj()
         metadata.core.fam = Mock()
-        metadata.clients_xml = metadata._handle_file("clients.xml")
+        @patchIf(not isinstance(lxml.etree.Element, Mock),
+                 "lxml.etree.Element", Mock())
+        def inner():
+            metadata.clients_xml = metadata._handle_file("clients.xml")
+        inner()
         metadata = TestMetadata.load_clients_data(self, metadata=metadata,
                                                   xdata=xdata)
         return TestMetadataBase.load_clients_data(self, metadata=metadata,
diff --git a/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestProbes.py b/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestProbes.py
index 899fb24a0..2163aa037 100644
--- a/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestProbes.py
+++ b/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestProbes.py
@@ -201,9 +201,7 @@ class TestProbes(TestProbing, TestConnector, TestDatabaseBacked):
     test_obj = Probes
 
     def get_obj(self, core=None):
-        if core is None:
-            core = MagicMock()
-        return self.test_obj(core, datastore)
+        return TestDatabaseBacked.get_obj(self, core=core)
 
     def get_test_probedata(self):
         test_xdata = lxml.etree.Element("test")
@@ -247,9 +245,10 @@ text
         # test__init(), which relies on being able to check the calls
         # of load_data(), and thus on load_data() being consistently
         # mocked)
-        @patch("Bcfg2.Server.Plugins.Probes.Probes.load_data", new=load_data)
+        @patch("%s.%s.load_data" % (self.test_obj.__module__,
+                                    self.test_obj.__name__), new=load_data)
         def inner():
-            return Probes(core, datastore)
+            return self.get_obj(core)
 
         return inner()
 
diff --git a/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestProperties.py b/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestProperties.py
index 93e2fff51..896f5861e 100644
--- a/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestProperties.py
+++ b/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestProperties.py
@@ -418,8 +418,8 @@ class TestXMLPropertyFile(TestPropertyFile, TestStructFile):
         self.assertFalse(mock_copy.called)
 
 
-class TestPropDirectoryBacked(TestDirectoryBacked):
-    test_obj = PropDirectoryBacked
+class TestProperties(TestPlugin, TestConnector, TestDirectoryBacked):
+    test_obj = Properties
     testfiles = ['foo.xml', 'bar.baz.xml']
     if HAS_JSON:
         testfiles.extend(["foo.json", "foo.xml.json"])
@@ -428,17 +428,13 @@ class TestPropDirectoryBacked(TestDirectoryBacked):
     ignore = ['foo.xsd', 'bar.baz.xsd', 'quux.xml.xsd']
     badevents = ['bogus.txt']
 
-
-class TestProperties(TestPlugin, TestConnector):
-    test_obj = Properties
-
-    def test__init(self):
-        TestPlugin.test__init(self)
-
-        core = Mock()
-        p = self.get_obj(core=core)
-        self.assertIsInstance(p.store, PropDirectoryBacked)
-        self.assertEqual(Bcfg2.Server.Plugins.Properties.SETUP, core.setup)
+    def get_obj(self, core=None):
+        @patch("%s.%s.add_directory_monitor" % (self.test_obj.__module__,
+                                                self.test_obj.__name__),
+               Mock())
+        def inner():
+            return TestPlugin.get_obj(self, core=core)
+        return inner()
 
     @patch("copy.copy")
     def test_get_additional_data(self, mock_copy):
@@ -446,11 +442,11 @@ class TestProperties(TestPlugin, TestConnector):
 
         p = self.get_obj()
         metadata = Mock()
-        p.store.entries = {"foo.xml": Mock(),
-                           "foo.yml": Mock()}
+        p.entries = {"foo.xml": Mock(),
+                     "foo.yml": Mock()}
         rv = p.get_additional_data(metadata)
         expected = dict()
-        for name, entry in p.store.entries.items():
+        for name, entry in p.entries.items():
             entry.get_additional_data.assert_called_with(metadata)
             expected[name] = entry.get_additional_data.return_value
         self.assertItemsEqual(rv, expected)