more pylint checks

2 files changed, 114 insertions, 77 deletions

diff --git a/testsuite/Testsrc/testmisc.py b/testsuite/Testsrc/testmisc.py
index 3ea80310e..41a91caff 100644
--- a/testsuite/Testsrc/testmisc.py
+++ b/testsuite/Testsrc/testmisc.py
@@ -29,91 +29,135 @@ srcpath = os.path.abspath(os.path.join(os.path.dirname(__file__), "..", "..",
 rcfile = os.path.abspath(os.path.join(os.path.dirname(__file__), "..",
                                       "pylintrc.conf"))
 
+# test for pylint existence
+try:
+    Popen(['pylint'], stdout=PIPE, stderr=STDOUT).wait()
+    HAS_PYLINT = True
+except OSError:
+    HAS_PYLINT = False
 
-class TestPylint(Bcfg2TestCase):
-    # right now, too many things fail pylint miserably to just test
-    # everything, or even to do a blacklist, so we just whitelist the
-    # things we do want to do a full check on and only check most
-    # stuff for errors and fatal errors.  This is a dict of
-    # <directory> => <file globs within that directory>.  <directory>
-    # is relative to src/
-    whitelist = {
-        "lib/Bcfg2/Server": ["Lint",
-                             "Plugin",
-                             "BuiltinCore.py",
-                             "CherryPyCore.py",
-                             "Core.py"],
-        "lib/Bcfg2/Server/Plugins": ["PuppetENC.py",
-                                     "Rules.py",
-                                     "DBStats.py",
-                                     "Trigger.py",
-                                     "Defaults.py",
-                                     "Probes.py",
-                                     "TemplateHelper.py",
-                                     "Guppy.py",
-                                     "FileProbes.py",
-                                     "ServiceCompat.py",
-                                     "Properties.py",
-                                     "SEModules.py",
-                                     "Darcs.py",
-                                     "Git.py",
-                                     "Hg.py",
-                                     "Cvs.py",
-                                     "Fossil.py",
-                                     "Svn.py",
-                                     "Svn2.py",
-                                     "Bzr.py",
-                                     "Cfg",
-                                     "Packages"],
-        "lib/Bcfg2/Client/Tools": ["POSIX"],
-        }
 
+# perform a full range of code checks on the listed files.
+full_checks = {
+    "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",
+                                 ],
+    "lib/Bcfg2/Client/Tools": ["POSIX"],
+    }
+
+# perform full code checks on the listed executables
+sbin_checks = {
+    "sbin": ["bcfg2-server", "bcfg2-yum-helper"]
+    }
+
+# perform limited, django-safe checks on the listed files
+django_checks = {
+    "lib/Bcfg2/Server": ["Reports", "models.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"]
+    }
+
+
+class TestPylint(Bcfg2TestCase):
     pylint_cmd = ["pylint", "--rcfile", rcfile]
 
     # regex to find errors and fatal errors
     error_re = re.compile(r':\d+:\s+\[[EF]\d{4}')
 
-    @skipIf(not os.path.exists(srcpath), "%s does not exist" % srcpath)
-    @skipIf(not os.path.exists(rcfile), "%s does not exist" % rcfile)
-    def test_pylint_full(self):
+    # build the blacklist
+    blacklist = []
+    for parent, modules in no_checks.items():
+        blacklist.extend([os.path.join(srcpath, parent, m) for m in modules])
+
+    def _get_paths(self, pathlist):
         paths = []
-        for parent, modules in self.whitelist.items():
+        for parent, modules in pathlist.items():
             paths.extend([os.path.join(srcpath, parent, m) for m in modules])
-        args = self.pylint_cmd + paths
-        try:
-            pylint = Popen(args, stdout=PIPE, stderr=STDOUT)
-            print(pylint.communicate()[0])
-            rv = pylint.wait()
-        except OSError:
-            if can_skip:
-                return skip("pylint not found")
-            else:
-                print("pylint not found")
-                return
-        self.assertEqual(rv, 0)
+        return list(set(paths) - 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))
+
+    @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_sbin_full(self):
+        self._pylint_full(self._get_paths(sbin_checks),
+                          extra_args=["--module-rgx",
+                                      "[a-z_-][a-z0-9_-]*$"])
+
+    def _pylint_full(self, paths, extra_args=None):
+        """ test select files for all pylint errors """
+        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])
+        self.assertEqual(pylint.wait(), 0)
 
+    @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_sbin_errors(self):
-        return self._pylint_errors(glob.glob("sbin/*"))
+        flist = list(set(os.path.join(srcpath, p)
+                         for p in glob.glob("sbin/*")) - set(self.blacklist))
+        return self._pylint_errors(flist)
 
     @skipUnless(HAS_DJANGO, "Django not found, skipping")
+    @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_django_errors(self):
-        return self._pylint_errors(["lib/Bcfg2/Server/Reports",
-                                    "lib/Bcfg2/Server/models.py"],
+        return self._pylint_errors(self._get_paths(django_checks),
                                    extra_args=["-d", "E1101"])
 
+    @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_errors(self):
-        # we ignore stuff that uses django (Reports, Hostbase,
-        # models.py) or that is deprecated and raises lots of errors
-        # (Snapshots, Hostbase), or that just raises a lot of errors
-        # (APT.py, RPMng.py, rpmtools.py).  Reports is tested by
-        # test_django_errors
-        ignore = ["models.py", "APT.py", "RPMng.py", "rpmtools.py",
-                  "Snapshots", "Reports", "Hostbase"]
+        ignore = []
+        for fname_list in django_checks.values() + no_checks.values():
+            ignore.extend(fname_list)
         return self._pylint_errors(["lib/Bcfg2"],
                                    extra_args=["--ignore", ",".join(ignore)])
 
-    @skipIf(not os.path.exists(srcpath), "%s does not exist" % srcpath)
-    @skipIf(not os.path.exists(rcfile), "%s does not exist" % rcfile)
     def _pylint_errors(self, paths, extra_args=None):
         """ test all files for fatals and errors """
         if extra_args is None:
@@ -121,16 +165,9 @@ class TestPylint(Bcfg2TestCase):
         args = self.pylint_cmd + extra_args + \
             ["-f", "parseable", "-d", "R0801,E1103"] + \
             [os.path.join(srcpath, p) for p in paths]
-        try:
-            pylint = Popen(args, stdout=PIPE, stderr=STDOUT)
-            output = pylint.communicate()[0]
-            rv = pylint.wait()
-        except OSError:
-            if can_skip:
-                return skip("pylint not found")
-            else:
-                print("pylint not found")
-                return
+        pylint = Popen(args, stdout=PIPE, stderr=STDOUT)
+        output = pylint.communicate()[0]
+        rv = pylint.wait()
 
         for line in output.splitlines():
             #print line
diff --git a/testsuite/pylintrc.conf b/testsuite/pylintrc.conf
index 014a94de5..2c56a3e81 100644
--- a/testsuite/pylintrc.conf
+++ b/testsuite/pylintrc.conf
@@ -169,14 +169,14 @@ variable-rgx=[a-z_][a-z0-9_]{2,30}$
 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
+good-names=_,rv,el,fd,ca,re
 
 # Bad variable names which should always be refused, separated by a comma
 bad-names=foo,bar,baz,toto,tutu,tata
 
 # Regular expression which should only match functions or classes name which do
 # not require a docstring
-no-docstring-rgx=__.*__
+no-docstring-rgx=__.*__|main
 
 
 [IMPORTS]