expanded pylint coverage

5 files changed, 92 insertions, 64 deletions

diff --git a/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestMetadata.py b/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestMetadata.py
index 2aebb0705..13e514e77 100644
--- a/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestMetadata.py
+++ b/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestMetadata.py
@@ -96,11 +96,11 @@ def get_metadata_object(core=None, watch_clients=False, use_db=False):
 
 
 class TestMetadataDB(DBModelTestCase):
-    if has_django:
+    if HAS_DJANGO:
         models = [MetadataClientModel]
 
 
-if has_django or can_skip:
+if HAS_DJANGO or can_skip:
     class TestClientVersions(Bcfg2TestCase):
         test_clients = dict(client1="1.2.0",
                             client2="1.2.2",
@@ -109,7 +109,7 @@ if has_django or can_skip:
                             client5=None,
                             client6=None)
 
-        @skipUnless(has_django, "Django not found")
+        @skipUnless(HAS_DJANGO, "Django not found")
         def setUp(self):
             syncdb(TestMetadataDB)
             for client, version in self.test_clients.items():
@@ -421,7 +421,7 @@ class TestMetadata(_TestMetadata, TestStatistics, TestDatabaseBacked):
         return get_metadata_object(core=core, watch_clients=watch_clients,
                                    use_db=self.use_db)
 
-    @skipUnless(has_django, "Django not found")
+    @skipUnless(HAS_DJANGO, "Django not found")
     def test__use_db(self):
         # with the way we've set up our metadata tests, it's unweildy
         # to test _use_db.  however, given the way get_obj works, if
@@ -1184,7 +1184,7 @@ class TestMetadataBase(TestMetadata):
     __test__ = False
     use_db = True
 
-    @skipUnless(has_django, "Django not found")
+    @skipUnless(HAS_DJANGO, "Django not found")
     def setUp(self):
         syncdb(TestMetadataDB)
 
@@ -1291,7 +1291,7 @@ class TestMetadata_NoClientsXML(TestMetadataBase):
     # have django.  otherwise they'll all get run because our fake
     # skipping decorators for python < 2.7 won't work when they
     # decorate setUp()
-    if can_skip or has_django:
+    if can_skip or HAS_DJANGO:
         __test__ = True
 
     def load_groups_data(self, metadata=None, xdata=None):
@@ -1456,7 +1456,7 @@ class TestMetadata_ClientsXML(TestMetadataBase):
     # have django.  otherwise they'll all get run because our fake
     # skipping decorators for python < 2.7 won't work when they
     # decorate setUp()
-    if can_skip or has_django:
+    if can_skip or HAS_DJANGO:
         __test__ = True
 
     def load_clients_data(self, metadata=None, xdata=None):
diff --git a/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestProbes.py b/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestProbes.py
index 34b2a0f0e..0ad92ca72 100644
--- a/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestProbes.py
+++ b/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestProbes.py
@@ -25,6 +25,7 @@ from TestPlugin import TestEntrySet, TestProbing, TestConnector, \
 # test data for JSON and YAML tests
 test_data = dict(a=1, b=[1, 2, 3], c="test")
 
+
 class FakeList(list):
     pass
 
@@ -32,7 +33,7 @@ class FakeList(list):
 class TestProbesDB(DBModelTestCase):
     if HAS_DJANGO:
         models = [ProbesGroupsModel, ProbesDataModel]
-    
+
 
 class TestClientProbeDataSet(Bcfg2TestCase):
     def test__init(self):
@@ -40,11 +41,12 @@ class TestClientProbeDataSet(Bcfg2TestCase):
         self.assertLessEqual(ds.timestamp, time.time())
         self.assertIsInstance(ds, dict)
         self.assertNotIn("timestamp", ds)
-        
+
         ds = ClientProbeDataSet(timestamp=123)
         self.assertEqual(ds.timestamp, 123)
         self.assertNotIn("timestamp", ds)
 
+
 class TestProbeData(Bcfg2TestCase):
     def test_str(self):
         # a value that is not valid XML, JSON, or YAML
@@ -108,10 +110,32 @@ 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
@@ -209,7 +233,7 @@ text
         return {"foo.example.com": ["group", "group with spaces",
                                     "group-with-dashes"],
                 "bar.example.com": []}
-    
+
     def get_probes_object(self, use_db=False, load_data=None):
         core = Mock()
         core.setup.cfp.getboolean = Mock()
@@ -229,7 +253,7 @@ text
             return Probes(core, datastore)
 
         return inner()
-        
+
     def test__init(self):
         mock_load_data = Mock()
         probes = self.get_probes_object(load_data=mock_load_data)
@@ -272,7 +296,7 @@ text
         probes.probedata = self.get_test_probedata()
         probes.cgroups = self.get_test_cgroups()
         probes._write_data_xml(None)
-        
+
         mock_open.assert_called_with(os.path.join(datastore, probes.name,
                                                   "probed.xml"), "w")
         data = lxml.etree.XML(mock_open.return_value.write.call_args[0][0])
@@ -333,7 +357,7 @@ text
             client = Mock()
             client.hostname = cname
             probes._write_data_db(client)
-        
+
             pdata = ProbesDataModel.objects.filter(hostname=cname).all()
             self.assertEqual(len(pdata), len(probes.probedata[cname]))
 
diff --git a/testsuite/Testsrc/test_code_checks.py b/testsuite/Testsrc/test_code_checks.py
index 41a91caff..ac739f4e5 100644
--- a/testsuite/Testsrc/test_code_checks.py
+++ b/testsuite/Testsrc/test_code_checks.py
@@ -39,43 +39,20 @@ except OSError:
 
 # perform a full range of code checks on the listed files.
 full_checks = {
+    "lib/Bcfg2": ["*.py"],
     "lib/Bcfg2/Server": ["Lint",
                          "Plugin",
-                         "BuiltinCore.py",
-                         "CherryPyCore.py",
-                         "Core.py"],
-    "lib/Bcfg2/Server/Plugins": ["Bundler.py",
-                                 "Bzr.py",
-                                 "Cfg",
-                                 "Cvs.py",
-                                 "DBStats.py",
-                                 "Darcs.py",
-                                 "Defaults.py",
-                                 "FileProbes.py",
-                                 "Fossil.py",
-                                 "Git.py",
-                                 "GroupPatterns.py",
-                                 "Guppy.py",
-                                 "Hg.py",
-                                 "Ohai.py",
-                                 "Packages",
-                                 "Probes.py",
-                                 "Properties.py",
-                                 "PuppetENC.py",
-                                 "Rules.py",
-                                 "SEModules.py",
-                                 "ServiceCompat.py",
-                                 "Svn.py",
-                                 "Svn2.py",
-                                 "TemplateHelper.py",
-                                 "Trigger.py",
-                                 ],
+                         "FileMonitor",
+                         "*.py"],
+    "lib/Bcfg2/Server/Plugins": ["Cfg", "Packages", "*.py"],
+    "lib/Bcfg2/Client": ["*.py"],
     "lib/Bcfg2/Client/Tools": ["POSIX"],
     }
 
 # perform full code checks on the listed executables
 sbin_checks = {
-    "sbin": ["bcfg2-server", "bcfg2-yum-helper"]
+    "sbin": ["bcfg2-server", "bcfg2-yum-helper", "bcfg2-crypt", "bcfg2-test",
+             "bcfg2-lint"]
     }
 
 # perform limited, django-safe checks on the listed files
@@ -83,12 +60,42 @@ django_checks = {
     "lib/Bcfg2/Server": ["Reports", "models.py"]
     }
 
+# perform only error checking on the listed files
+error_checks = {
+    "lib/Bcfg2": ["Proxy.py", "SSLServer.py"],
+    "lib/Bcfg2/Server/Plugins": ["Decisions.py",
+                                 "Deps.py",
+                                 "Ldap.py",
+                                 "NagiosGen.py",
+                                 "Pkgmgr.py",
+                                 "SSHbase.py",
+                                 "SSLCA.py"]
+    }
+
 # perform no checks at all on the listed files
 no_checks = {
     "lib/Bcfg2/Client/Tools": ["APT.py", "RPMng.py", "rpmtools.py"],
-    "lib/Bcfg2/Server": ["Snapshots", "Hostbase"]
+    "lib/Bcfg2/Server": ["Snapshots", "Hostbase"],
+    "lib/Bcfg2": ["manage.py"],
+    "lib/Bcfg2/Server/Reports": ["manage.py"],
+    "lib/Bcfg2/Server/Plugins": ["Account.py",
+                                 "Base.py",
+                                 "Editor.py",
+                                 "Hostbase.py",
+                                 "Snapshots.py",
+                                 "Statistics.py",
+                                 "TCheetah.py",
+                                 "TGenshi.py"],
     }
 
+def expand_path_dict(pathdict):
+    """ given a path dict as above, return a list of all the paths """
+    rv = []
+    for parent, modules in pathdict.items():
+        for mod in modules:
+            rv.extend(glob.glob(os.path.join(srcpath, parent, mod)))
+    return rv
+
 
 class TestPylint(Bcfg2TestCase):
     pylint_cmd = ["pylint", "--rcfile", rcfile]
@@ -97,21 +104,18 @@ class TestPylint(Bcfg2TestCase):
     error_re = re.compile(r':\d+:\s+\[[EF]\d{4}')
 
     # build the blacklist
-    blacklist = []
-    for parent, modules in no_checks.items():
-        blacklist.extend([os.path.join(srcpath, parent, m) for m in modules])
+    blacklist = expand_path_dict(no_checks)
 
-    def _get_paths(self, pathlist):
-        paths = []
-        for parent, modules in pathlist.items():
-            paths.extend([os.path.join(srcpath, parent, m) for m in modules])
-        return list(set(paths) - set(self.blacklist))
+    def _get_paths(self, pathdict):
+        return list(set(expand_path_dict(pathdict)) - set(self.blacklist))
 
     @skipIf(not os.path.exists(srcpath), "%s does not exist" % srcpath)
     @skipIf(not os.path.exists(rcfile), "%s does not exist" % rcfile)
     @skipUnless(HAS_PYLINT, "pylint not found, skipping")
     def test_lib_full(self):
-        self._pylint_full(self._get_paths(full_checks))
+        full_list = list(set(self._get_paths(full_checks)) -
+                         set(expand_path_dict(error_checks)))
+        self._pylint_full(full_list)
 
     @skipIf(not os.path.exists(srcpath), "%s does not exist" % srcpath)
     @skipIf(not os.path.exists(rcfile), "%s does not exist" % rcfile)
@@ -126,7 +130,6 @@ class TestPylint(Bcfg2TestCase):
         if extra_args is None:
             extra_args = []
         args = self.pylint_cmd + extra_args + \
-            ["-f", "parseable"] + \
             [os.path.join(srcpath, p) for p in paths]
         pylint = Popen(args, stdout=PIPE, stderr=STDOUT)
         print(pylint.communicate()[0])
diff --git a/testsuite/install.sh b/testsuite/install.sh
index 565e158df..c8312bce9 100755
--- a/testsuite/install.sh
+++ b/testsuite/install.sh
@@ -10,8 +10,9 @@ if [[ "$WITH_OPTIONAL_DEPS" == "yes" ]]; then
     if [[ $PYVER == "2.5" ]]; then
         # markdown 2.2.0 is broken on py2.5, so until 2.2.1 is released use 2.1
         pip install --use-mirrors 'markdown<2.2'
+        pip install --use-mirrors simplejson
     fi
-    pip install --use-mirrors genshi cheetah 'django<1.4' M2Crypto
+    pip install --use-mirrors genshi cheetah 'django<1.4' M2Crypto yaml
 else
     # python < 2.6 requires M2Crypto for SSL communication, not just
     # for encryption support
diff --git a/testsuite/pylintrc.conf b/testsuite/pylintrc.conf
index 2c56a3e81..2e4279e75 100644
--- a/testsuite/pylintrc.conf
+++ b/testsuite/pylintrc.conf
@@ -33,7 +33,7 @@ load-plugins=
 # can either give multiple identifier separated by comma (,) or put this option
 # multiple time (only on the command line, not in the configuration file where
 # it should appear only once).
-disable=W0142,W0511,W0603,R0201,R0901,R0902,R0903,R0904,R0921,R0922,C0302,I0011
+disable=W0142,W0511,W0603,W1201,R0201,R0901,R0902,R0903,R0904,R0921,R0922,C0302,I0011
 
 
 [REPORTS]
@@ -121,7 +121,7 @@ zope=no
 # List of members which are set dynamically and missed by pylint inference
 # system, and so shouldn't trigger E0201 when accessed. Python regular
 # expressions are accepted.
-generated-members=REQUEST,acl_users,aq_parent,objects,DoesNotExist
+generated-members=objects,DoesNotExist,isoformat
 
 
 [MISCELLANEOUS]
@@ -136,7 +136,7 @@ notes=FIXME,XXX,TODO
 required-attributes=
 
 # List of builtins function names that should not be used, separated by a comma
-bad-functions=map,filter,apply,input
+bad-functions=map,apply
 
 # Regular expression which should only match correct module names
 module-rgx=(([a-z_][a-z0-9_]*)|([A-Z][a-zA-Z0-9]+))$
@@ -156,20 +156,20 @@ function-rgx=[a-z_][a-z0-9_]{2,30}$
 method-rgx=[A-z_][A-z0-9_]{2,30}$
 
 # Regular expression which should only match correct instance attribute names
-attr-rgx=(Entries|[a-z_][a-z0-9_]{2,30})$
+attr-rgx=(Entries|[a-z_][a-z0-9_]{2,30}(ID)?)$
 
 # Regular expression which should only match correct argument names
-argument-rgx=[a-z_][a-z0-9_]{2,30}$
+argument-rgx=[a-z_][a-z0-9_]{2,30}(ID)?$
 
 # Regular expression which should only match correct variable names
-variable-rgx=[a-z_][a-z0-9_]{2,30}$
+variable-rgx=[a-z_][a-z0-9_]{2,30}(ID)?$
 
 # Regular expression which should only match correct list comprehension /
 # generator expression variable names
 inlinevar-rgx=[A-Za-z_][A-Za-z0-9_]*$
 
 # Good variable names which should always be accepted, separated by a comma
-good-names=_,rv,el,fd,ca,re
+good-names=_,rv,el,fd,ca,re,i,j,iv
 
 # Bad variable names which should always be refused, separated by a comma
 bad-names=foo,bar,baz,toto,tutu,tata
@@ -200,7 +200,7 @@ int-import-graph=
 [DESIGN]
 
 # Maximum number of arguments for function / method
-max-args=6
+max-args=8
 
 # Argument names that match this expression will be ignored. Default to name
 # with leading underscore