Increase coverage

- Improved coverage numbers by (1) adding and tweaking unit tests, (2)
  launching an additional scripts with coverage enabled, (3) commenting out
  some for-future-use code, and (4) marking various code blocks irrelevant for
  unittesting with the 'no cover' marker.
- Fixed up some comments and spelling mistakes.
- Added a mechanism to disable certain git UI feedback mechanisms during unit
  testing (previously skipped due to an oversight in a test case.

.travis.yml

@@ -70,6 +70,7 @@ script:
   - ksconf --version
   - coverage run -m unittest discover -s tests
 after_success:
+  - '[[ $BUILD_SDIST == "true" ]] && coverage run -a -m make_cli_docs'
   - codecov
   - coveralls
   - '[[ $BUILD_SDIST == "true" ]] && ./make_zipapp'

ksconf/commands/__init__.py

@@ -16,6 +16,13 @@
 from ksconf.consts import SMART_CREATE
 from ksconf.util import memoize, debug_traceback
 
+__all__ = [
+    "KsconfCmd",
+    "ConfDirProxy",
+    "ConfFileProxy",
+    "ConfFileType",
+    "dedent",
+]
 
 class ConfDirProxy(object):
     def __init__(self, name, mode, parse_profile=None):
@@ -46,7 +53,7 @@ def __init__(self, name, mode, stream=None, parse_profile=None, is_file=None):
     def is_file(self):
         return self._is_file
 
-    def _type(self):
+    def _type(self):    # pragma: no cover  (only used in exceptions)
         if self._is_file:
             return "file"
         else:
@@ -67,10 +74,7 @@ def reset(self):
             if self.is_file():
                 self.close()
             else:
-                try:
-                    self.stream.seek(0)
-                except:
-                    raise
+                self.stream.seek(0)
 
     def set_parser_option(self, **kwargs):
         """ Setting a key to None will remove that setting. """
@@ -100,7 +104,6 @@ def data(self):
         return self._data
 
     def load(self, profile=None):
-
         if "r" not in self._mode:
             # Q: Should we mimic the exception caused by doing a read() on a write-only file object?
             raise ValueError("Unable to load() from {} with mode '{}'".format(self._type(),
@@ -112,7 +115,7 @@ def load(self, profile=None):
         return data
 
     def dump(self, data):
-        if "+" not in self._mode and "w" not in self._mode:
+        if "+" not in self._mode and "w" not in self._mode:     # pragma: no cover
             raise ValueError("Unable to dump() to {} with mode '{}'".format(self._type(),
                                                                             self._mode))
         # Feels like the right thing to do????  OR self._data = data
@@ -130,6 +133,7 @@ def unlink(self):
         self.close()
         return os.unlink(self.name)
 
+    '''
     def backup(self, bkname=None):
         # One option:  Write this file directly to the git object store.  Just need to store some
         # kind of index to allow the users to pull it back.   (Sill, would need of fall-back
@@ -138,7 +142,7 @@ def backup(self, bkname=None):
 
     def checksum(self, hash="sha256"):
         raise NotImplementedError
-
+    '''
 
 class ConfFileType(object):
     """Factory for creating conf file object types;  returns a lazy-loader ConfFile proxy class
@@ -241,6 +245,7 @@ def __init__(self, name):
         self.stdout = sys.stdout
         self.stderr = sys.stderr
 
+    '''
     def redirect_io(self, stdin=None, stdout=None, stderr=None):
         if stdin is not None:
             self.stdin = stdin
@@ -253,6 +258,7 @@ def exit(self, exit_code):
         """ Allow overriding for unittesting or other high-level functionality, like an
         interactive interface. """
         sys.exit(exit_code)
+    '''
 
     def add_parser(self, subparser):
         # Passing in the object return by 'ArgumentParser.add_subparsers()'
@@ -266,7 +272,7 @@ def add_parser(self, subparser):
         self.parser.set_defaults(funct=self.launch)
         self.register_args(self.parser)
 
-    def register_args(self, parser):
+    def register_args(self, parser):        # pragma: no cover
         """ This function in passed the """
         raise NotImplementedError
 
@@ -278,7 +284,7 @@ def launch(self, args):
         exc = None
         try:
             return_code = self.run(args)
-        except:
+        except:     # pragma: no cover
             exc = sys.exc_info()
             raise
         finally:
@@ -290,13 +296,13 @@ def pre_run(self, args):
         """ Pre-run hook.  Any exceptions here prevent run() from being called. """
         pass
 
-    def run(self, args):
+    def run(self, args):    # pragma: no cover
         """ Actual works happens here.  Return code should be an EXIT_CODE_* from consts. """
         raise NotImplementedError
 
     def post_run(self, args, exec_info=None):
-        """ Any custom clean up work that needs done.  Allways called if run() was.  Presence of
-       exc_info indicates failure. """
+        """ Any custom clean up work that needs done.  Always called if run() was.  Presence of
+        exc_info indicates failure. """
         pass
 
 
@@ -346,7 +352,7 @@ def _get_fallback(group, name=None):
 # Removed _get_pkgresources_lib as middle option
 __get_entity_resolvers = [ _get_entrypoints_lib, _get_fallback ]
 
-if "ksconf_cmd" in os.environ.get("KSCONF_DISABLE_PLUGINS", ""):
+if "ksconf_cmd" in os.environ.get("KSCONF_DISABLE_PLUGINS", ""):    # pragma: no cover
     # Only use the fallback built in mechanism.  This is helpful when unittesting and building docs
     # as we don't want to accidentally document/test code from other packages.
     __get_entity_resolvers = [ _get_fallback ]
@@ -360,7 +366,7 @@ def get_entrypoints(group, name=None):
         results = None
         try:
             results = resolver(group, name=name)
-        except ImportError:
+        except ImportError as e:    # pragma: no cover
             __get_entity_resolvers.remove(resolver)
         if results:
             return results

ksconf/commands/unarchive.py

@@ -1,8 +1,8 @@
-""" SUBCOMMAND:  ksconf upgrade tarball
+""" SUBCOMMAND:  ksconf unarchive <tarball>
 
 Usage example:
 
-    ksconf upgrade tarball
+    ksconf unarchive splunk-add-on-for-amazon-web-services_111.tgz
 
 """
 

ksconf/util/__init__.py

@@ -1,3 +1,5 @@
+from __future__ import unicode_literals
+
 def _xargs(iterable, cmd_len=1024):
     fn_len = 0
     buf = []

ksconf/vc/git.py

@@ -1,13 +1,14 @@
-from __future__ import absolute_import
-from __future__ import unicode_literals
+from __future__ import absolute_import, unicode_literals
+
 from collections import namedtuple, Counter
-from subprocess import Popen, PIPE, list2cmdline
+from subprocess import Popen, PIPE, list2cmdline, call
 
 from ksconf.util import _xargs
 
 GIT_BIN = "git"
 GitCmdOutput = namedtuple("GitCmdOutput", ["cmd", "returncode", "stdout", "stderr", "lines"])
 
+unitesting = False
 
 def git_cmd(args, shell=False, cwd=None, capture_std=True, encoding="utf-8"):
     if isinstance(args, tuple):
@@ -93,9 +94,10 @@ def git_ls_files(path, *modifiers):
                            .format(proc.returncode))
     return proc.stdout.splitlines()
 
-
-def git_status_ui(path, *args):
-    from subprocess import call
+def git_status_ui(path, *args):  # pragma: no cover
+    # For unittesting purposes, this function is a nuisance
+    if unitesting:
+        return
     # Don't redirect the std* streams; let the output go straight to the console
     cmd = [GIT_BIN, "status", "."]
     cmd.extend(args)

tests/cli_helper.py

@@ -259,6 +259,10 @@ def read_file(self, rel_path, as_bytes=False):
             content = stream.read()
         return content
 
+    def remove_file(self, rel_path):
+        path = self.get_path(rel_path)
+        os.unlink(path)
+
     def write_conf(self, rel_path, conf):
         path = self.get_path(rel_path)
         self.makedir(None, path=os.path.dirname(path))

tests/test_cli.py

@@ -2,22 +2,54 @@
 from __future__ import absolute_import, print_function, unicode_literals
 import os
 import sys
-import unittest
 
 # Allow interactive execution from CLI,  cd tests; ./test_cli.py
 if __package__ is None:
     sys.path.append(os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
 
-from tests.cli_helper import ksconf_cli
+from tests.cli_helper import ksconf_cli, TestWorkDir, FakeStdin
+from ksconf.consts import EXIT_CODE_SUCCESS, EXIT_CODE_NO_SUCH_FILE, EXIT_CODE_USER_QUIT
+import unittest
 
 
 class CliSimpleTestCase(unittest.TestCase):
     """ Test some very simple CLI features. """
 
     def test_help(self):
         out = ksconf_cli("--help")
-        self.assertIn("Kintyre Splunk CONFig tool", out.stdout)
-        self.assertIn("usage: ", out.stdout)
+        with ksconf_cli:
+            self.assertIn("Kintyre Splunk CONFig tool", out.stdout)
+            self.assertIn("usage: ", out.stdout)
+            self.assertEqual(out.returncode, EXIT_CODE_SUCCESS)
+
+    def test_conffileproxy_invalid_arg(self):
+        bad_conf = """
+        [dangling stanza
+        attr = 1
+        bad file =  very true"""
+        twd = TestWorkDir()
+        badfile = twd.write_file("bad_conf.conf", bad_conf)
+        with ksconf_cli:
+            ko = ksconf_cli("merge", twd.get_path("a_non_existant_file.conf"))
+            self.assertIn(ko.returncode, (EXIT_CODE_USER_QUIT, EXIT_CODE_NO_SUCH_FILE))
+            self.assertRegex(ko.stderr, r".*\b(can't open '[^']+\.conf'|invalid ConfFileType).*")
+
+            ko = ksconf_cli("merge", badfile)
+            self.assertIn(ko.returncode, (EXIT_CODE_USER_QUIT, EXIT_CODE_NO_SUCH_FILE))
+            self.assertRegex(ko.stderr, ".*(failed to parse|invalid ConfFileType).*")
+
+            with FakeStdin(bad_conf):
+                ko = ksconf_cli("merge", "-")
+                self.assertIn(ko.returncode, (EXIT_CODE_USER_QUIT, EXIT_CODE_NO_SUCH_FILE))
+                self.assertRegex(ko.stderr, ".*(failed to parse|invalid ConfFileType).*")
+
+
+    def test_entrypoints(self):
+        from ksconf.commands import get_entrypoints, _get_fallback
+        get_entrypoints("ksconf_cmd", "sort")
+
+        # Just to exercise this (coverage and prevent regressions)
+        _get_fallback("ksconf_cmd")
 
 
 if __name__ == '__main__':  # pragma: no cover

tests/test_cli_unarchive.py

@@ -9,7 +9,7 @@
 if __package__ is None:
     sys.path.append(os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
 
-from ksconf.consts import EXIT_CODE_SUCCESS
+from ksconf.consts import EXIT_CODE_SUCCESS, EXIT_CODE_FAILED_SAFETY_CHECK
 from tests.cli_helper import TestWorkDir, static_data, ksconf_cli
 
 
@@ -20,6 +20,15 @@ def __init__(self, *args, **kwargs):
         self._modsec_workdir = TestWorkDir(git_repo=True)
     '''
 
+
+    @classmethod
+    def setUpClass(cls):
+        # Tell the VC/git module that unit testing is in progress and therefore don't run git
+        # command with the sole purpose of dumping junk to to the terminal.  Only run if unit
+        # testing is actually invoked.
+        import ksconf.vc.git
+        ksconf.vc.git.unitesting = True
+
     def setUp(self):
         # Setup environmental variables to avoid GIT commit errors regarding missing user.email, user.name configs
         env = os.environ
@@ -54,12 +63,21 @@ def _modsec01_install_11(self, twd):
         twd.write_file("Junk.bak", "# An ignored file.")
 
     def _modsec01_untracked_files(self, twd):
-        twd.write_file("untracked_file", "content")
+        twd.write_file("apps/Splunk_TA_modsecurity/untracked_file", "content")
+        twd.write_file("apps/Splunk_TA_modsecurity/ignored.bak", "Ignored file")
         with ksconf_cli:
             kco = ksconf_cli("unarchive", static_data("apps/modsecurity-add-on-for-splunk_12.tgz"),
                              "--dest", twd.get_path("apps"), "--git-sanity-check=ignored",
                              "--git-mode=commit", "--no-edit")
-
+            self.assertEqual(kco.returncode, EXIT_CODE_FAILED_SAFETY_CHECK)
+            # Rollback upgrade and try again
+            twd.git("reset", "--hard", "HEAD")
+            # Remove offending files
+            twd.remove_file("apps/Splunk_TA_modsecurity/untracked_file")
+            twd.remove_file("apps/Splunk_TA_modsecurity/ignored.bak")
+            kco = ksconf_cli("unarchive", static_data("apps/modsecurity-add-on-for-splunk_12.tgz"),
+                             "--dest", twd.get_path("apps"), "--git-sanity-check", "ignored",
+                             "--git-mode=commit", "--no-edit")
             self.assertEqual(kco.returncode, EXIT_CODE_SUCCESS)
 
     def _modsec01_upgrade(self, twd, app_tgz):