fixed tests for pylint changes

6 files changed, 66 insertions, 86 deletions

diff --git a/testsuite/Testsrc/Testlib/TestClient/TestTools/TestPOSIX/Testbase.py b/testsuite/Testsrc/Testlib/TestClient/TestTools/TestPOSIX/Testbase.py
index b447ab642..bcb1d16af 100644
--- a/testsuite/Testsrc/Testlib/TestClient/TestTools/TestPOSIX/Testbase.py
+++ b/testsuite/Testsrc/Testlib/TestClient/TestTools/TestPOSIX/Testbase.py
@@ -23,15 +23,15 @@ from common import XI_NAMESPACE, XI, inPy3k, call, builtins, u, can_skip, \
 
 try:
     import selinux
-    has_selinux = True
+    HAS_SELINUX = True
 except ImportError:
-    has_selinux = False
+    HAS_SELINUX = False
 
 try:
     import posix1e
-    has_acls = True
+    HAS_ACLS = True
 except ImportError:
-    has_acls = False
+    HAS_ACLS = False
 
 class TestPOSIXTool(Bcfg2TestCase):
     test_obj = POSIXTool
@@ -298,9 +298,9 @@ class TestPOSIXTool(Bcfg2TestCase):
         mock_set_secontext.assert_called_with(entry, path=entry.get("name"))
         mock_set_acls.assert_called_with(entry, path=entry.get("name"))
 
-    @skipUnless(has_acls, "ACLS not found, skipping")
-    @patchIf(has_acls, "posix1e.ACL")
-    @patchIf(has_acls, "posix1e.Entry")
+    @skipUnless(HAS_ACLS, "ACLS not found, skipping")
+    @patchIf(HAS_ACLS, "posix1e.ACL")
+    @patchIf(HAS_ACLS, "posix1e.Entry")
     @patch("os.path.isdir")
     @patch("Bcfg2.Client.Tools.POSIX.base.%s._norm_uid" % test_obj.__name__)
     @patch("Bcfg2.Client.Tools.POSIX.base.%s._norm_gid" % test_obj.__name__)
@@ -311,9 +311,9 @@ class TestPOSIXTool(Bcfg2TestCase):
         entry = lxml.etree.Element("Path", name="/etc/foo", type="file")
 
         # disable acls for the initial test
-        Bcfg2.Client.Tools.POSIX.base.has_acls = False
+        Bcfg2.Client.Tools.POSIX.base.HAS_ACLS = False
         self.assertTrue(self.ptool._set_acls(entry))
-        Bcfg2.Client.Tools.POSIX.base.has_acls = True
+        Bcfg2.Client.Tools.POSIX.base.HAS_ACLS = True
 
         # build a set of file ACLs to return from posix1e.ACL(file=...)
         file_acls = []
@@ -442,16 +442,16 @@ class TestPOSIXTool(Bcfg2TestCase):
         self.assertItemsEqual(added_acls,
                               [(fileacl_rv, posix1e.ACL_GROUP, 100, 5)])
 
-    @skipUnless(has_selinux, "SELinux not found, skipping")
-    @patchIf(has_selinux, "selinux.restorecon")
-    @patchIf(has_selinux, "selinux.lsetfilecon")
+    @skipUnless(HAS_SELINUX, "SELinux not found, skipping")
+    @patchIf(HAS_SELINUX, "selinux.restorecon")
+    @patchIf(HAS_SELINUX, "selinux.lsetfilecon")
     def test_set_secontext(self, mock_lsetfilecon, mock_restorecon):
         entry = lxml.etree.Element("Path", name="/etc/foo", type="file")
 
         # disable selinux for the initial test
-        Bcfg2.Client.Tools.POSIX.base.has_selinux = False
+        Bcfg2.Client.Tools.POSIX.base.HAS_SELINUX = False
         self.assertTrue(self.ptool._set_secontext(entry))
-        Bcfg2.Client.Tools.POSIX.base.has_selinux = True
+        Bcfg2.Client.Tools.POSIX.base.HAS_SELINUX = True
 
         # no context given
         self.assertTrue(self.ptool._set_secontext(entry))
@@ -568,36 +568,36 @@ class TestPOSIXTool(Bcfg2TestCase):
 
         # disable selinux and acls for this call -- we test them
         # separately so that we can skip those tests as appropriate
-        states = (Bcfg2.Client.Tools.POSIX.base.has_selinux,
-                  Bcfg2.Client.Tools.POSIX.base.has_acls)
-        Bcfg2.Client.Tools.POSIX.base.has_selinux = False
-        Bcfg2.Client.Tools.POSIX.base.has_acls = False
+        states = (Bcfg2.Client.Tools.POSIX.base.HAS_SELINUX,
+                  Bcfg2.Client.Tools.POSIX.base.HAS_ACLS)
+        Bcfg2.Client.Tools.POSIX.base.HAS_SELINUX = False
+        Bcfg2.Client.Tools.POSIX.base.HAS_ACLS = False
         self.assertEqual(self.ptool._gather_data(path),
                          (stat_rv, '0', '10', '0660', None, None))
-        Bcfg2.Client.Tools.POSIX.base.has_selinux, \
-            Bcfg2.Client.Tools.POSIX.base.has_acls = states
+        Bcfg2.Client.Tools.POSIX.base.HAS_SELINUX, \
+            Bcfg2.Client.Tools.POSIX.base.HAS_ACLS = states
         mock_stat.assert_called_with(path)
 
-    @skipUnless(has_selinux, "SELinux not found, skipping")
+    @skipUnless(HAS_SELINUX, "SELinux not found, skipping")
     def test__gather_data_selinux(self):
         context = 'system_u:object_r:root_t:s0'
         path = '/test'
 
         @patch('os.stat')
-        @patchIf(has_selinux, "selinux.getfilecon")
+        @patchIf(HAS_SELINUX, "selinux.getfilecon")
         def inner(mock_getfilecon, mock_stat):
             mock_getfilecon.return_value = [len(context) + 1, context]
             mock_stat.return_value = MagicMock()
             # disable acls for this call and test them separately
-            state = Bcfg2.Client.Tools.POSIX.base.has_acls
-            Bcfg2.Client.Tools.POSIX.base.has_acls = False
+            state = Bcfg2.Client.Tools.POSIX.base.HAS_ACLS
+            Bcfg2.Client.Tools.POSIX.base.HAS_ACLS = False
             self.assertEqual(self.ptool._gather_data(path)[4], 'root_t')
-            Bcfg2.Client.Tools.POSIX.base.has_acls = state
+            Bcfg2.Client.Tools.POSIX.base.HAS_ACLS = state
             mock_getfilecon.assert_called_with(path)
 
         inner()
 
-    @skipUnless(has_acls, "ACLS not found, skipping")
+    @skipUnless(HAS_ACLS, "ACLS not found, skipping")
     @patch('os.stat')
     @patch("Bcfg2.Client.Tools.POSIX.base.%s._list_file_acls" %
            test_obj.__name__)
@@ -608,13 +608,13 @@ class TestPOSIXTool(Bcfg2TestCase):
         path = '/test'
         mock_stat.return_value = MagicMock()
         # disable selinux for this call and test it separately
-        state = Bcfg2.Client.Tools.POSIX.base.has_selinux
-        Bcfg2.Client.Tools.POSIX.base.has_selinux = False
+        state = Bcfg2.Client.Tools.POSIX.base.HAS_SELINUX
+        Bcfg2.Client.Tools.POSIX.base.HAS_SELINUX = False
         self.assertItemsEqual(self.ptool._gather_data(path)[5], acls)
-        Bcfg2.Client.Tools.POSIX.base.has_selinux = state
+        Bcfg2.Client.Tools.POSIX.base.HAS_SELINUX = state
         mock_list_file_acls.assert_called_with(path)
 
-    @patchIf(has_selinux, "selinux.matchpathcon")
+    @patchIf(HAS_SELINUX, "selinux.matchpathcon")
     @patch("Bcfg2.Client.Tools.POSIX.base.%s._verify_acls" % test_obj.__name__)
     @patch("Bcfg2.Client.Tools.POSIX.base.%s._gather_data" % test_obj.__name__)
     @patch("Bcfg2.Client.Tools.POSIX.base.%s._norm_entry_uid" %
@@ -667,8 +667,8 @@ class TestPOSIXTool(Bcfg2TestCase):
         # test when secontext is None
         entry = reset()
         gather_data_rv[4] = None
-        sestate = Bcfg2.Client.Tools.POSIX.base.has_selinux
-        Bcfg2.Client.Tools.POSIX.base.has_selinux = False
+        sestate = Bcfg2.Client.Tools.POSIX.base.HAS_SELINUX
+        Bcfg2.Client.Tools.POSIX.base.HAS_SELINUX = False
         mock_gather_data.return_value = tuple(gather_data_rv)
         self.assertTrue(self.ptool._verify_metadata(entry))
         mock_gather_data.assert_called_with(entry.get("name"))
@@ -677,7 +677,7 @@ class TestPOSIXTool(Bcfg2TestCase):
         for attr, idx, val in expected:
             if attr != 'current_secontext':
                 self.assertEqual(entry.get(attr), val)
-        Bcfg2.Client.Tools.POSIX.base.has_selinux = sestate
+        Bcfg2.Client.Tools.POSIX.base.HAS_SELINUX = sestate
 
         gather_data_rv = [MagicMock(), None, None, None, None, []]
         for attr, idx, val in expected:
@@ -704,7 +704,7 @@ class TestPOSIXTool(Bcfg2TestCase):
         failures = [('current_owner', 1, '10'),
                     ('current_group', 2, '100'),
                     ('current_perms', 3, '0660')]
-        if has_selinux:
+        if HAS_SELINUX:
             failures.append(('current_secontext', 4, 'root_t'))
         
         for fail_attr, fail_idx, fail_val in failures:
@@ -743,7 +743,7 @@ class TestPOSIXTool(Bcfg2TestCase):
             self.assertEqual(entry.get(attr), val)
         self.assertEqual(entry.get("current_mtime"), str(fail_mtime))
         
-        if has_selinux:
+        if HAS_SELINUX:
             # test success and failure for __default__ secontext
             entry = reset()
             entry.set("mtime", str(mtime))
@@ -782,7 +782,7 @@ class TestPOSIXTool(Bcfg2TestCase):
                 self.assertEqual(entry.get(attr), val)
             self.assertEqual(entry.get("current_mtime"), str(mtime))
 
-    @skipUnless(has_acls, "ACLS not found, skipping")
+    @skipUnless(HAS_ACLS, "ACLS not found, skipping")
     def test_list_entry_acls(self):
         entry = lxml.etree.Element("Path", name="/test", type="file")
         lxml.etree.SubElement(entry, "ACL", scope="user", type="default",
@@ -793,7 +793,7 @@ class TestPOSIXTool(Bcfg2TestCase):
                               {("default", posix1e.ACL_USER, "user"): 7,
                                ("access", posix1e.ACL_GROUP, "group"): 5})
 
-    @skipUnless(has_acls, "ACLS not found, skipping")
+    @skipUnless(HAS_ACLS, "ACLS not found, skipping")
     @patch("pwd.getpwuid")
     @patch("grp.getgrgid")
     @patch("os.path.isdir")
@@ -878,7 +878,7 @@ class TestPOSIXTool(Bcfg2TestCase):
         self.assertItemsEqual(mock_ACL.call_args_list,
                               [call(file=path), call(filedef=path)])
 
-    if has_acls:
+    if HAS_ACLS:
         # python 2.6 applies decorators at compile-time, not at
         # run-time, so we can't do these as decorators because
         # pylibacl might not be installed.  (If it's not, this test
@@ -886,7 +886,7 @@ class TestPOSIXTool(Bcfg2TestCase):
         # we're safe.)
         test_list_file_acls = patch("posix1e.ACL")(test_list_file_acls)
 
-    @skipUnless(has_acls, "ACLS not found, skipping")
+    @skipUnless(HAS_ACLS, "ACLS not found, skipping")
     @patch("Bcfg2.Client.Tools.POSIX.base.%s._list_file_acls" %
            test_obj.__name__)
     @patch("Bcfg2.Client.Tools.POSIX.base.%s._list_entry_acls" %
diff --git a/testsuite/Testsrc/Testlib/TestServer/TestPlugin/Testhelpers.py b/testsuite/Testsrc/Testlib/TestServer/TestPlugin/Testhelpers.py
index aea00c356..f555d15f1 100644
--- a/testsuite/Testsrc/Testlib/TestServer/TestPlugin/Testhelpers.py
+++ b/testsuite/Testsrc/Testlib/TestServer/TestPlugin/Testhelpers.py
@@ -81,7 +81,7 @@ class TestFunctions(Bcfg2TestCase):
 class TestDatabaseBacked(TestPlugin):
     test_obj = DatabaseBacked
 
-    @skipUnless(has_django, "Django not found")
+    @skipUnless(HAS_DJANGO, "Django not found")
     def test__use_db(self):
         core = Mock()
         core.setup.cfp.getboolean.return_value = True
@@ -93,7 +93,7 @@ class TestDatabaseBacked(TestPlugin):
         db = self.get_obj(core)
         self.assertFalse(db._use_db)
         
-        Bcfg2.Server.Plugin.helpers.has_django = False
+        Bcfg2.Server.Plugin.helpers.HAS_DJANGO = False
         core = Mock()
         db = self.get_obj(core)
         self.assertFalse(db._use_db)
@@ -102,7 +102,7 @@ class TestDatabaseBacked(TestPlugin):
         core.setup.cfp.getboolean.return_value = True
         db = self.get_obj(core)
         self.assertFalse(db._use_db)
-        Bcfg2.Server.Plugin.helpers.has_django = True
+        Bcfg2.Server.Plugin.helpers.HAS_DJANGO = True
 
 
 class TestPluginDatabaseModel(Bcfg2TestCase):
diff --git a/testsuite/Testsrc/Testlib/TestServer/TestPlugin/Testinterfaces.py b/testsuite/Testsrc/Testlib/TestServer/TestPlugin/Testinterfaces.py
index 1c0af33eb..f05795b1a 100644
--- a/testsuite/Testsrc/Testlib/TestServer/TestPlugin/Testinterfaces.py
+++ b/testsuite/Testsrc/Testlib/TestServer/TestPlugin/Testinterfaces.py
@@ -129,7 +129,7 @@ class TestThreadedStatistics(TestStatistics):
         queue.empty = Mock(side_effect=Empty)
         ts.work_queue = queue
 
-        mock_open.side_effect = OSError
+        mock_open.side_effect = IOError
         # test that save does _not_ raise an exception even when
         # everything goes pear-shaped
         ts._save()
@@ -188,7 +188,7 @@ class TestThreadedStatistics(TestStatistics):
 
         reset()
         mock_exists.return_value = True
-        mock_open.side_effect = OSError
+        mock_open.side_effect = IOError
         self.assertFalse(ts._load())
         mock_exists.assert_called_with(ts.pending_file)
         mock_open.assert_called_with(ts.pending_file, 'r')
@@ -330,7 +330,7 @@ class TestVersion(Bcfg2TestCase):
     test_obj = Version
     
     def get_obj(self):
-        return self.test_obj()
+        return self.test_obj(datastore)
 
     def test_get_revision(self):
         d = self.get_obj()
diff --git a/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestMetadata.py b/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestMetadata.py
index 9646ee66d..2aebb0705 100644
--- a/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestMetadata.py
+++ b/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestMetadata.py
@@ -1458,14 +1458,13 @@ class TestMetadata_ClientsXML(TestMetadataBase):
     # decorate setUp()
     if can_skip or has_django:
         __test__ = True
-    
+
     def load_clients_data(self, metadata=None, xdata=None):
         if metadata is None:
             metadata = self.get_obj()
         metadata.core.fam = Mock()
-        metadata._handle_file("clients.xml")
+        metadata.clients_xml = metadata._handle_file("clients.xml")
         metadata = TestMetadata.load_clients_data(self, metadata=metadata,
                                                   xdata=xdata)
         return TestMetadataBase.load_clients_data(self, metadata=metadata,
                                                     xdata=xdata)
-
diff --git a/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestProbes.py b/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestProbes.py
index 4b020dcff..34b2a0f0e 100644
--- a/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestProbes.py
+++ b/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestProbes.py
@@ -30,7 +30,7 @@ class FakeList(list):
 
 
 class TestProbesDB(DBModelTestCase):
-    if has_django:
+    if HAS_DJANGO:
         models = [ProbesGroupsModel, ProbesDataModel]
     
 
@@ -69,13 +69,14 @@ class TestProbeData(Bcfg2TestCase):
         self.assertIsNotNone(data.xdata)
         self.assertIsNotNone(data.xdata.find("test2"))
 
+    @skipUnless(HAS_JSON, "JSON libraries not found, skipping JSON tests")
     def test_json(self):
         jdata = json.dumps(test_data)
         data = ProbeData(jdata)
         self.assertIsNotNone(data.json)
         self.assertItemsEqual(test_data, data.json)
         
-    @skipUnless(has_yaml, "YAML libraries not found, skipping YAML tests")
+    @skipUnless(HAS_YAML, "YAML libraries not found, skipping YAML tests")
     def test_yaml(self):
         jdata = yaml.dump(test_data)
         data = ProbeData(jdata)
@@ -107,32 +108,10 @@ class TestProbeSet(TestEntrySet):
         fam.AddMonitor.assert_called_with(datastore, ps)
         TestEntrySet.test__init(self)
 
-    def test_HandleEvent(self):
-        ps = self.get_obj()
-        ps.handle_event = Mock()
-
-        # test that events on the data store itself are skipped
-        evt = Mock()
-        evt.filename = datastore
-        ps.HandleEvent(evt)
-        self.assertFalse(ps.handle_event.called)
-
-        # test that events on probed.xml are skipped
-        evt.reset_mock()
-        evt.filename = "probed.xml"
-        ps.HandleEvent(evt)
-        self.assertFalse(ps.handle_event.called)
-        
-        # test that other events are processed appropriately
-        evt.reset_mock()
-        evt.filename = "fooprobe"
-        ps.HandleEvent(evt)
-        ps.handle_event.assert_called_with(evt)
-
     @patch("%s.list" % builtins, FakeList)
     def test_get_probe_data(self):
         ps = self.get_obj()
-        
+
         # build some fairly complex test data for this.  in the end,
         # we want the probe data to include only the most specific
         # version of a given probe, and by basename only, not full
@@ -220,8 +199,9 @@ text
 """)
         rv["bar.example.com"] = ClientProbeDataSet(timestamp=time.time())
         rv["bar.example.com"]["empty"] = ProbeData("")
-        rv["bar.example.com"]["json"] = ProbeData(json.dumps(test_data))
-        if has_yaml:
+        if HAS_JSON:
+            rv["bar.example.com"]["json"] = ProbeData(json.dumps(test_data))
+        if HAS_YAML:
             rv["bar.example.com"]["yaml"] = ProbeData(yaml.dump(test_data))
         return rv
 
@@ -268,7 +248,7 @@ text
                                                             "use_database",
                                                             default=False)
 
-    @skipUnless(has_django, "Django not found, skipping")
+    @skipUnless(HAS_DJANGO, "Django not found, skipping")
     @patch("Bcfg2.Server.Plugins.Probes.Probes._write_data_db", Mock())
     @patch("Bcfg2.Server.Plugins.Probes.Probes._write_data_xml", Mock())
     def test_write_data_xml(self):
@@ -277,7 +257,7 @@ text
         probes._write_data_xml.assert_called_with("test")
         self.assertFalse(probes._write_data_db.called)
 
-    @skipUnless(has_django, "Django not found, skipping")
+    @skipUnless(HAS_DJANGO, "Django not found, skipping")
     @patch("Bcfg2.Server.Plugins.Probes.Probes._write_data_db", Mock())
     @patch("Bcfg2.Server.Plugins.Probes.Probes._write_data_xml", Mock())
     def test_write_data_db(self):
@@ -331,17 +311,18 @@ text
         self.assertIsNotNone(empty)
         self.assertIsNotNone(empty.get("value"))
         self.assertEqual(empty.get("value"), "")
-        jdata = bardata.find("Probe[@name='json']")
-        self.assertIsNotNone(jdata)
-        self.assertIsNotNone(jdata.get("value"))
-        self.assertItemsEqual(test_data, json.loads(jdata.get("value")))
-        if has_yaml:
+        if HAS_JSON:
+            jdata = bardata.find("Probe[@name='json']")
+            self.assertIsNotNone(jdata)
+            self.assertIsNotNone(jdata.get("value"))
+            self.assertItemsEqual(test_data, json.loads(jdata.get("value")))
+        if HAS_YAML:
             ydata = bardata.find("Probe[@name='yaml']")
             self.assertIsNotNone(ydata)
             self.assertIsNotNone(ydata.get("value"))
             self.assertItemsEqual(test_data, yaml.load(ydata.get("value")))
 
-    @skipUnless(has_django, "Django not found, skipping")
+    @skipUnless(HAS_DJANGO, "Django not found, skipping")
     def test__write_data_db(self):
         syncdb(TestProbesDB)
         probes = self.get_probes_object(use_db=True)
@@ -393,7 +374,7 @@ text
         pgroups = ProbesGroupsModel.objects.filter(hostname=cname).all()
         self.assertEqual(len(pgroups), len(probes.cgroups[cname]))
 
-    @skipUnless(has_django, "Django not found, skipping")
+    @skipUnless(HAS_DJANGO, "Django not found, skipping")
     @patch("Bcfg2.Server.Plugins.Probes.Probes._load_data_db", Mock())
     @patch("Bcfg2.Server.Plugins.Probes.Probes._load_data_xml", Mock())
     def test_load_data_xml(self):
@@ -402,7 +383,7 @@ text
         probes._load_data_xml.assert_any_call()
         self.assertFalse(probes._load_data_db.called)
 
-    @skipUnless(has_django, "Django not found, skipping")
+    @skipUnless(HAS_DJANGO, "Django not found, skipping")
     @patch("Bcfg2.Server.Plugins.Probes.Probes._load_data_db", Mock())
     @patch("Bcfg2.Server.Plugins.Probes.Probes._load_data_xml", Mock())
     def test_load_data_db(self):
@@ -434,7 +415,7 @@ text
         self.assertItemsEqual(probes.probedata, self.get_test_probedata())
         self.assertItemsEqual(probes.cgroups, self.get_test_cgroups())
 
-    @skipUnless(has_django, "Django not found, skipping")
+    @skipUnless(HAS_DJANGO, "Django not found, skipping")
     def test__load_data_db(self):
         syncdb(TestProbesDB)
         probes = self.get_probes_object(use_db=True)
diff --git a/testsuite/Testsrc/testmisc.py b/testsuite/Testsrc/test_code_checks.py
index 41a91caff..41a91caff 100644
--- a/testsuite/Testsrc/testmisc.py
+++ b/testsuite/Testsrc/test_code_checks.py