added PEP-8 style checks to test suite

12 files changed, 185 insertions, 113 deletions

diff --git a/src/lib/Bcfg2/Compat.py b/src/lib/Bcfg2/Compat.py
index b0f0ef5cf..4bcc76e8f 100644
--- a/src/lib/Bcfg2/Compat.py
+++ b/src/lib/Bcfg2/Compat.py
@@ -51,7 +51,8 @@ except ImportError:
 
 # xmlrpc imports
 try:
-    import xmlrpclib, SimpleXMLRPCServer
+    import xmlrpclib
+    import SimpleXMLRPCServer
 except ImportError:
     import xmlrpc.client as xmlrpclib
     import xmlrpc.server as SimpleXMLRPCServer
@@ -73,6 +74,7 @@ try:
 except NameError:
     unicode = str
 
+
 def u_str(string, encoding=None):
     """ print to file compatibility """
     if sys.hexversion >= 0x03000000:
diff --git a/src/lib/Bcfg2/Proxy.py b/src/lib/Bcfg2/Proxy.py
index 3b406c78e..3aefed5d1 100644
--- a/src/lib/Bcfg2/Proxy.py
+++ b/src/lib/Bcfg2/Proxy.py
@@ -51,13 +51,16 @@ class ProxyError(Exception):
             msg = str(err)
         Exception.__init__(self, msg)
 
+
 class CertificateError(Exception):
     def __init__(self, commonName):
         self.commonName = commonName
+
     def __str__(self):
         return ("Got unallowed commonName %s from server"
                 % self.commonName)
 
+
 _orig_Method = xmlrpclib._Method
 
 class RetryMethod(xmlrpclib._Method):
diff --git a/src/lib/Bcfg2/Server/Plugin/interfaces.py b/src/lib/Bcfg2/Server/Plugin/interfaces.py
index f42ada773..cb996b1ca 100644
--- a/src/lib/Bcfg2/Server/Plugin/interfaces.py
+++ b/src/lib/Bcfg2/Server/Plugin/interfaces.py
@@ -313,6 +313,7 @@ class Threaded(object):
         """
         raise NotImplementedError
 
+
 class ThreadedStatistics(Statistics, Threaded, threading.Thread):
     """ ThreadedStatistics plugins process client statistics in a
     separate thread. """
diff --git a/src/lib/Bcfg2/Server/Plugins/DBStats.py b/src/lib/Bcfg2/Server/Plugins/DBStats.py
index e0794f019..e6ef50fa1 100644
--- a/src/lib/Bcfg2/Server/Plugins/DBStats.py
+++ b/src/lib/Bcfg2/Server/Plugins/DBStats.py
@@ -9,7 +9,6 @@ class DBStats(Bcfg2.Server.Plugin.Plugin):
     def __init__(self, core, datastore):
         Bcfg2.Server.Plugin.Plugin.__init__(self, core, datastore)
         self.logger.error("DBStats has been replaced with Reporting")
-        self.logger.error("DBStats: Be sure to migrate your data "\
-            "before running the report collector")
+        self.logger.error("DBStats: Be sure to migrate your data "
+                          "before running the report collector")
         raise Bcfg2.Server.Plugin.PluginInitError
-
diff --git a/src/lib/Bcfg2/Server/Plugins/Packages/Apt.py b/src/lib/Bcfg2/Server/Plugins/Packages/Apt.py
index ec0d8e828..27f493677 100644
--- a/src/lib/Bcfg2/Server/Plugins/Packages/Apt.py
+++ b/src/lib/Bcfg2/Server/Plugins/Packages/Apt.py
@@ -35,7 +35,7 @@ class AptCollection(Collection):
 
         for source in self:
             if source.rawurl:
-                self.logger.info("Packages: Skipping rawurl %s" % 
+                self.logger.info("Packages: Skipping rawurl %s" %
                                  source.rawurl)
             else:
                 lines.append("deb %s %s %s" % (source.url, source.version,
diff --git a/src/sbin/bcfg2-admin b/src/sbin/bcfg2-admin
index fa4307702..31e49c00b 100755
--- a/src/sbin/bcfg2-admin
+++ b/src/sbin/bcfg2-admin
@@ -10,6 +10,7 @@ import Bcfg2.Options
 import Bcfg2.Server.Admin
 from Bcfg2.Compat import StringIO
 
+
 def mode_import(modename):
     """Load Bcfg2.Server.Admin.<mode>."""
     modname = modename.capitalize()
diff --git a/src/sbin/bcfg2-crypt b/src/sbin/bcfg2-crypt
index fde6af582..eae316da5 100755
--- a/src/sbin/bcfg2-crypt
+++ b/src/sbin/bcfg2-crypt
@@ -261,7 +261,7 @@ class Encryptor(object):
                                     (self.pname, pname))
         return (passphrase, pname)
 
-    def _get_passphrase(self, chunk): #  pylint: disable=W0613
+    def _get_passphrase(self, chunk):  # pylint: disable=W0613
         """ get the passphrase for a chunk of a file """
         return None
 
diff --git a/src/sbin/bcfg2-info b/src/sbin/bcfg2-info
index fa8c89b46..311784606 100755
--- a/src/sbin/bcfg2-info
+++ b/src/sbin/bcfg2-info
@@ -474,7 +474,7 @@ Bcfg2 client itself.""")
                 ('Password', self.setup['password']),
                 ('Server Metadata Connector', self.setup['mconnect']),
                 ('Filemonitor', self.setup['filemonitor']),
-                ('Server address',    self.setup['location']),
+                ('Server address', self.setup['location']),
                 ('Path to key', self.setup['key']),
                 ('Path to SSL certificate', self.setup['cert']),
                 ('Path to SSL CA certificate', self.setup['ca']),
@@ -727,7 +727,6 @@ Bcfg2 client itself.""")
         pass
 
 
-
 def build_usage():
     """ build usage message """
     cmd_blacklist = ["do_loop", "do_EOF"]
diff --git a/src/sbin/bcfg2-server b/src/sbin/bcfg2-server
index 8322edeaa..cdca71e74 100755
--- a/src/sbin/bcfg2-server
+++ b/src/sbin/bcfg2-server
@@ -11,6 +11,7 @@ from Bcfg2.Server.Core import CoreInitError
 
 LOGGER = logging.getLogger('bcfg2-server')
 
+
 def main():
     optinfo = dict()
     optinfo.update(Bcfg2.Options.CLI_COMMON_OPTIONS)
@@ -22,7 +23,7 @@ def main():
     if not os.path.exists(setup['configfile']):
         print("Could not read %s" % setup['configfile'])
         sys.exit(1)
-    
+
     if setup['backend'] not in ['best', 'cherrypy', 'builtin']:
         print("Unknown server backend %s, using 'best'" % setup['backend'])
         setup['backend'] = 'best'
diff --git a/src/sbin/bcfg2-test b/src/sbin/bcfg2-test
index 3dcad8689..4a57fa42f 100755
--- a/src/sbin/bcfg2-test
+++ b/src/sbin/bcfg2-test
@@ -5,7 +5,6 @@ without failures"""
 
 import os
 import sys
-import signal
 import fnmatch
 import logging
 import Bcfg2.Logger
@@ -26,7 +25,7 @@ except ImportError:
 class CapturingLogger(object):
     """ Fake logger that captures logging output so that errors are
     only displayed for clients that fail tests """
-    def __init__(self, *args, **kwargs):
+    def __init__(self, *args, **kwargs):  # pylint: disable=W0613
         self.output = []
 
     def error(self, msg):
@@ -46,6 +45,7 @@ class CapturingLogger(object):
         self.output.append(msg)
 
     def reset_output(self):
+        """ Reset the captured output """
         self.output = []
 
 
@@ -182,7 +182,8 @@ def run_child(setup, clients, queue):
     core.shutdown()
 
 
-def main():
+def parse_args():
+    """ Parse command line arguments. """
     optinfo = dict(Bcfg2.Options.TEST_COMMON_OPTIONS)
     optinfo.update(Bcfg2.Options.CLI_COMMON_OPTIONS)
     optinfo.update(Bcfg2.Options.SERVER_COMMON_OPTIONS)
@@ -230,7 +231,12 @@ def main():
                 pass
         if xunitfile and not setup['xunit']:
             setup['xunit'] = xunitfile
+    return setup
+
 
+def main():
+    setup = parse_args()
+    logger = logging.getLogger(sys.argv[0])
     core = get_core(setup)
 
     if setup['args']:
diff --git a/src/sbin/bcfg2-yum-helper b/src/sbin/bcfg2-yum-helper
index ba6f30406..7e5c03fd5 100755
--- a/src/sbin/bcfg2-yum-helper
+++ b/src/sbin/bcfg2-yum-helper
@@ -129,7 +129,7 @@ class DepSolver(object):
             err = sys.exc_info()[1]
             self.logger.warning(err)
             return []
-        
+
         if ptype == "default":
             return [p
                     for p, d in list(group.default_packages.items())
@@ -254,6 +254,6 @@ def main():
             rv[gdata['group']] = list(packages)
         print(json.dumps(rv))
 
-        
+
 if __name__ == '__main__':
     sys.exit(main())
diff --git a/testsuite/Testsrc/test_code_checks.py b/testsuite/Testsrc/test_code_checks.py
index 3404c19ce..2f49134fa 100644
--- a/testsuite/Testsrc/test_code_checks.py
+++ b/testsuite/Testsrc/test_code_checks.py
@@ -25,14 +25,6 @@ srcpath = os.path.abspath(os.path.join(testdir, "..", "src"))
 # path to pylint rc file
 rcfile = os.path.join(testdir, "pylintrc.conf")
 
-# test for pylint existence
-try:
-    Popen(['pylint'], stdout=PIPE, stderr=STDOUT).wait()
-    HAS_PYLINT = True
-except OSError:
-    HAS_PYLINT = False
-
-
 # perform checks on the listed files only if the module listed in the
 # keys can be imported
 contingent_checks = {
@@ -136,13 +128,8 @@ def blacklist_filter(filelist, blacklist):
     return rv
 
 
-class TestPylint(Bcfg2TestCase):
-    pylint_cmd = ["pylint", "--rcfile", rcfile, "--init-hook",
-                  "import sys;sys.path.append('%s')" %
-                  os.path.join(srcpath, "lib")]
-
-    # regex to find errors and fatal errors
-    error_re = re.compile(r':\d+:\s+\[[EF]\d{4}')
+class CodeTestCase(Bcfg2TestCase):
+    __test__ = False
 
     # build the blacklists
     blacklist = expand_path_dict(no_checks)
@@ -154,118 +141,183 @@ class TestPylint(Bcfg2TestCase):
     full_blacklist = expand_path_dict(error_checks) + contingent_blacklist + \
         blacklist
 
+    command = [None]
+
+    has_command = None
+
+    # extra arguments when running tests on sbin/*
+    sbin_args = []
+
+    # extra arguments when running tests on lib/*
+    lib_args = []
+
+    # extra arguments for full tests
+    full_args = []
+
+    # extra arguments for error tests
+    error_args = []
+
+    def has_exec(self):
+        if self.has_command is None:
+            try:
+                proc = Popen(self.command,
+                             stdin=PIPE, stdout=PIPE, stderr=STDOUT)
+                proc.communicate(input="\n")
+                proc.wait()
+                self.has_command = True
+            except OSError:
+                self.has_command = False
+        return self.has_command
+
     def get_env(self):
         env = copy.copy(os.environ)
-        if 'PYTHONPATH' in os.environ:
-            env['PYTHONPATH'] = '%s:%s' % (env['PYTHONPATH'], testdir)
-        else:
-            env['PYTHONPATH'] = testdir
+        env['PYTHONPATH'] = ':'.join([env.get("PYTHONPATH", ""),
+                                     testdir])
         return env
 
+    def _test_full(self, files, extra_args=None):
+        """ test select files for all problems """
+        if not len(files):
+            return
+        if extra_args is None:
+            extra_args = []
+        cmd = self.command + self.full_args + extra_args + \
+            [os.path.join(srcpath, f) for f in files]
+        proc = Popen(cmd, stdout=PIPE, stderr=STDOUT, env=self.get_env())
+        print(proc.communicate()[0])
+        self.assertEqual(proc.wait(), 0)
+
+    def _test_errors(self, files, extra_args=None):
+        """ test select files for errors """
+        if not len(files):
+            return
+        if extra_args is None:
+            extra_args = []
+        cmd = self.command + self.error_args + extra_args + \
+            [os.path.join(srcpath, f) for f in files]
+        proc = Popen(cmd, stdout=PIPE, stderr=STDOUT, env=self.get_env())
+        print(proc.communicate()[0])
+        self.assertEqual(proc.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_lib_full(self):
-        full_list = []
-        for root, _, files in os.walk(os.path.join(srcpath, "lib")):
-            full_list.extend(blacklist_filter([os.path.join(root, f)
-                                               for f in files
-                                               if f.endswith(".py")],
-                                              self.full_blacklist))
-        self._pylint_full(full_list)
+        @skipUnless(self.has_exec(),
+                    "%s not found, skipping" % self.command[0])
+        def inner():
+            full_list = []
+            for root, _, files in os.walk(os.path.join(srcpath, "lib")):
+                full_list.extend(blacklist_filter([os.path.join(root, f)
+                                                   for f in files
+                                                   if f.endswith(".py")],
+                                                  self.full_blacklist))
+            self._test_full(full_list, extra_args=self.lib_args)
+
+        inner()
 
     @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_contingent_full(self):
-        blacklist = set(expand_path_dict(error_checks) + self.blacklist)
-        for (mods, filedict) in contingent_checks.items():
-            if "django" in mods:
-                # there's some issue with running pylint on modules
-                # that use django in Travis CI (but not elsewhere), so
-                # skip these for now
-                continue
-            try:
-                for mod in mods:
-                    __import__(mod)
-            except ImportError:
-                continue
-            self._pylint_full(blacklist_filter(expand_path_dict(filedict),
-                                               blacklist))
+        @skipUnless(self.has_exec(),
+                    "%s not found, skipping" % self.command[0])
+        def inner():
+            filelist = []
+            blacklist = set(expand_path_dict(error_checks) + self.blacklist)
+            for (mods, filedict) in contingent_checks.items():
+                try:
+                    for mod in mods:
+                        __import__(mod)
+                except ImportError:
+                    continue
+                filelist.extend(expand_path_dict(filedict))
+            self._test_full(blacklist_filter(filelist, blacklist),
+                            extra_args=self.lib_args)
+
+        inner()
 
     @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):
-        all_sbin = [os.path.join(srcpath, "sbin", f)
-                    for f in glob.glob(os.path.join(srcpath, "sbin", "*"))]
-        sbin_list = blacklist_filter([f for f in all_sbin
-                                      if not os.path.islink(f)],
-                                     self.full_blacklist)
-        self._pylint_full(sbin_list,
-                          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 + \
-            [os.path.join(srcpath, p) for p in paths]
-        pylint = Popen(args, stdout=PIPE, stderr=STDOUT, env=self.get_env())
-        print(pylint.communicate()[0])
-        self.assertEqual(pylint.wait(), 0)
+    def test_sbin(self):
+        @skipUnless(self.has_exec(),
+                    "%s not found, skipping" % self.command[0])
+        def inner():
+            all_sbin = [os.path.join(srcpath, "sbin", f)
+                        for f in glob.glob(os.path.join(srcpath, "sbin", "*"))]
+            full_list = blacklist_filter([f for f in all_sbin
+                                          if not os.path.islink(f)],
+                                         self.full_blacklist)
+            self._test_full(full_list, extra_args=self.sbin_args)
+
+            errors_list = blacklist_filter([f for f in all_sbin
+                                            if not os.path.islink(f)],
+                                           self.contingent_blacklist)
+            self._test_errors(errors_list, extra_args=self.sbin_args)
+
+        inner()
 
     @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):
-        all_sbin = [os.path.join(srcpath, "sbin", f)
-                    for f in glob.glob(os.path.join(srcpath, "sbin", "*"))]
-        sbin_list = blacklist_filter([f for f in all_sbin
-                                      if not os.path.islink(f)],
-                                     self.contingent_blacklist)
-        self._pylint_errors(sbin_list,
-                            extra_args=["--module-rgx",
-                                        "[a-z_-][a-z0-9_-]*$"])
+    def test_contingent_errors(self):
+        @skipUnless(self.has_exec(),
+                    "%s not found, skipping" % self.command[0])
+        def inner():
+            filelist = []
+            whitelist = expand_path_dict(error_checks)
+            for (mods, filedict) in contingent_checks.items():
+                try:
+                    for mod in mods:
+                        __import__(mod)
+                except ImportError:
+                    continue
+                filelist.extend(expand_path_dict(filedict))
+            flist = blacklist_filter(whitelist_filter(filelist, whitelist),
+                                     self.blacklist)
+            self._test_errors(flist, extra_args=self.lib_args)
+
+        inner()
 
     @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_contingent_errors(self):
-        whitelist = expand_path_dict(error_checks)
-        for (mods, filedict) in contingent_checks.items():
+    def test_lib_errors(self):
+        @skipUnless(self.has_exec(),
+                    "%s not found, skipping" % self.command[0])
+        def inner():
+            filelist = blacklist_filter(expand_path_dict(error_checks),
+                                        self.contingent_blacklist)
+            return self._test_errors(filelist, extra_args=self.lib_args)
+
+        inner()
+
+
+class TestPylint(CodeTestCase):
+    __test__ = True
+    command = ["pylint", "--rcfile", rcfile, "--init-hook",
+               "import sys;sys.path.append('%s')" %
+               os.path.join(srcpath, "lib")]
+
+    sbin_args = ["--module-rgx", "[a-z_-][a-z0-9_-]*$"]
+    error_args = ["-f", "parseable", "-d", "R0801,E1103"]
+
+    # regex to find errors and fatal errors
+    error_re = re.compile(r':\d+:\s+\[[EF]\d{4}')
+
+    def __init__(self, *args, **kwargs):
+        CodeTestCase.__init__(self, *args, **kwargs)
+        for mods, filedict in contingent_checks.items():
             if "django" in mods:
                 # there's some issue with running pylint on modules
                 # that use django in Travis CI (but not elsewhere), so
                 # skip these for now
-                continue
-            try:
-                for mod in mods:
-                    __import__(mod)
-            except ImportError:
-                continue
-            flist = \
-                blacklist_filter(whitelist_filter(expand_path_dict(filedict),
-                                                  whitelist),
-                                     self.blacklist)
-            self._pylint_errors(flist)
-
-    @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):
-        filelist = blacklist_filter(expand_path_dict(error_checks),
-                                    self.contingent_blacklist)
-        return self._pylint_errors(filelist)
+                self.blacklist += expand_path_dict(filedict)
 
-    def _pylint_errors(self, paths, extra_args=None):
+    def _test_errors(self, files, extra_args=None):
         """ test all files for fatals and errors """
+        if not len(files):
+            return
         if extra_args is None:
             extra_args = []
-        args = self.pylint_cmd + extra_args + \
-            ["-f", "parseable", "-d", "R0801,E1103"] + \
-            [os.path.join(srcpath, p) for p in paths]
+        args = self.command + self.error_args + extra_args + \
+            [os.path.join(srcpath, p) for p in files]
         pylint = Popen(args, stdout=PIPE, stderr=STDOUT, env=self.get_env())
         output = pylint.communicate()[0]
         rv = pylint.wait()
@@ -276,3 +328,11 @@ class TestPylint(Bcfg2TestCase):
         # pylint returns a bitmask, where 1 means fatal errors
         # were encountered and 2 means errors were encountered.
         self.assertEqual(rv & 3, 0)
+
+
+class TestPEP8(CodeTestCase):
+    __test__ = True
+    command = ["pep8", "--ignore=E125,E501"]
+
+    def _test_errors(self, files, extra_args=None):
+        pass