wrote unit tests for base client Tool classes

4 files changed, 98 insertions, 72 deletions

diff --git a/src/lib/Bcfg2/Client/Frame.py b/src/lib/Bcfg2/Client/Frame.py
index a95c0a7a6..637a916d6 100644
--- a/src/lib/Bcfg2/Client/Frame.py
+++ b/src/lib/Bcfg2/Client/Frame.py
@@ -1,14 +1,12 @@
 """ Frame is the Client Framework that verifies and installs entries,
 and generates statistics. """
 
-import os
-import sys
 import time
-import select
 import fnmatch
 import logging
 import Bcfg2.Client.Tools
-from Bcfg2.Compat import input, any, all  # pylint: disable=W0622
+from Bcfg2.Client import prompt
+from Bcfg2.Compat import any, all  # pylint: disable=W0622
 
 
 def cmpent(ent1, ent2):
@@ -154,7 +152,7 @@ class Frame(object):
             for entry in multi:
                 self.logger.debug(entry)
 
-    def promptFilter(self, prompt, entries):
+    def promptFilter(self, msg, entries):
         """Filter a supplied list based on user input."""
         ret = []
         entries.sort(cmpent)
@@ -165,20 +163,9 @@ class Frame(object):
             if 'qtext' in entry.attrib:
                 iprompt = entry.get('qtext')
             else:
-                iprompt = prompt % (entry.tag, entry.get('name'))
-            # flush input buffer
-            while len(select.select([sys.stdin.fileno()], [], [], 0.0)[0]) > 0:
-                os.read(sys.stdin.fileno(), 4096)
-            try:
-                ans = input(iprompt.encode(sys.stdout.encoding, 'replace'))
-                if ans in ['y', 'Y']:
-                    ret.append(entry)
-            except EOFError:
-                # python 2.4.3 on CentOS doesn't like ^C for some reason
-                break
-            except:
-                print("Error while reading input")
-                continue
+                iprompt = msg % (entry.tag, entry.get('name'))
+            if prompt(iprompt):
+                ret.append(entry)
         return ret
 
     def __getattr__(self, name):
@@ -281,7 +268,7 @@ class Frame(object):
 
     def Decide(self):  # pylint: disable=R0912
         """Set self.whitelist based on user interaction."""
-        prompt = "Install %s: %s? (y/N): "
+        iprompt = "Install %s: %s? (y/N): "
         rprompt = "Remove %s: %s? (y/N): "
         if self.setup['remove']:
             if self.setup['remove'] == 'all':
@@ -354,7 +341,7 @@ class Frame(object):
                            (bmodified or a.get('when') == 'always'))]
             # now we process all "always actions"
             if self.setup['interactive']:
-                self.promptFilter(prompt, actions)
+                self.promptFilter(iprompt, actions)
             self.DispatchInstallCalls(actions)
 
             # need to test to fail entries in whitelist
@@ -377,7 +364,7 @@ class Frame(object):
                                              if b.get("name")))
 
         if self.setup['interactive']:
-            self.whitelist = self.promptFilter(prompt, self.whitelist)
+            self.whitelist = self.promptFilter(iprompt, self.whitelist)
             self.removal = self.promptFilter(rprompt, self.removal)
 
         for entry in candidates:
diff --git a/src/lib/Bcfg2/Client/Tools/POSIX/Hardlink.py b/src/lib/Bcfg2/Client/Tools/POSIX/Hardlink.py
index 896ca5f49..64a0b1e15 100644
--- a/src/lib/Bcfg2/Client/Tools/POSIX/Hardlink.py
+++ b/src/lib/Bcfg2/Client/Tools/POSIX/Hardlink.py
@@ -12,5 +12,4 @@ class POSIXHardlink(POSIXLinkTool):
         return os.path.samefile(entry.get('name'), entry.get('to'))
 
     def _link(self, entry):
-        ## TODO: set permissions
         return os.link(entry.get('to'), entry.get('name'))
diff --git a/src/lib/Bcfg2/Client/Tools/__init__.py b/src/lib/Bcfg2/Client/Tools/__init__.py
index c0dd60c1e..08dc09294 100644
--- a/src/lib/Bcfg2/Client/Tools/__init__.py
+++ b/src/lib/Bcfg2/Client/Tools/__init__.py
@@ -3,10 +3,10 @@
 import os
 import sys
 import stat
-import select
 from subprocess import Popen, PIPE
+import Bcfg2.Client
 import Bcfg2.Client.XML
-from Bcfg2.Compat import input, walk_packages  # pylint: disable=W0622
+from Bcfg2.Compat import walk_packages  # pylint: disable=W0622
 
 __all__ = [m[1] for m in walk_packages(path=__path__)]
 
@@ -155,25 +155,34 @@ class Tool(object):
         #: A list of all entries handled by this tool
         self.handled = []
 
-        for struct in config:
+        self._analyze_config()
+        self._check_execs()
+
+    def _analyze_config(self):
+        """ Analyze the config at tool initialization-time for
+        important and handled entries """
+        for struct in self.config:
             for entry in struct:
                 if (entry.tag == 'Path' and
                     entry.get('important', 'false').lower() == 'true'):
                     self.__important__.append(entry.get('name'))
-                if self.handlesEntry(entry):
-                    self.handled.append(entry)
+        self.handled = self.getSupportedEntries()
+
+    def _check_execs(self):
+        """ Check all executables used by this tool to ensure that
+        they exist and are executable """
         for filename in self.__execs__:
             try:
                 mode = stat.S_IMODE(os.stat(filename)[stat.ST_MODE])
-                if mode & stat.S_IEXEC != stat.S_IEXEC:
-                    raise ToolInstantiationError("%s: %s not executable" %
-                                                 (self.name, filename))
             except OSError:
                 raise ToolInstantiationError(sys.exc_info()[1])
             except:
                 raise ToolInstantiationError("%s: Failed to stat %s" %
-                                             (self.name, filename),
-                                             exc_info=1)
+                                             (self.name, filename))
+            if not mode & stat.S_IEXEC:
+                raise ToolInstantiationError("%s: %s not executable" %
+                                             (self.name, filename))
+
 
     def BundleUpdated(self, bundle, states):  # pylint: disable=W0613
         """ Callback that is invoked when a bundle has been updated.
@@ -227,11 +236,13 @@ class Tool(object):
                 if self.canVerify(entry):
                     try:
                         func = getattr(self, "Verify%s" % entry.tag)
-                        states[entry] = func(entry, mods)
                     except AttributeError:
                         self.logger.error("%s: Cannot verify %s entries" %
                                           (self.name, entry.tag))
-                    except:
+                        continue
+                    try:
+                        states[entry] = func(entry, mods)
+                    except:  # pylint: disable=W0702
                         self.logger.error("%s: Unexpected failure verifying %s"
                                           % (self.name,
                                              self.primarykey(entry)),
@@ -255,14 +266,16 @@ class Tool(object):
         :returns: None """
         for entry in entries:
             try:
-                func = getattr(self, "Install%s" % (entry.tag))
-                states[entry] = func(entry)
-                if states[entry]:
-                    self.modified.append(entry)
+                func = getattr(self, "Install%s" % entry.tag)
             except AttributeError:
                 self.logger.error("%s: Cannot install %s entries" %
                                   (self.name, entry.tag))
-            except:
+                continue
+            try:
+                states[entry] = func(entry)
+                if states[entry]:
+                    self.modified.append(entry)
+            except:  # pylint: disable=W0702
                 self.logger.error("%s: Unexpected failure installing %s" %
                                   (self.name, self.primarykey(entry)),
                                   exc_info=1)
@@ -451,6 +464,19 @@ class PkgTool(Tool):
         """
         raise NotImplementedError
 
+    def _get_package_command(self, packages):
+        """ Get the command to install the given list of packages.
+
+        :param packages: The Package entries to install
+        :type packages: list of lxml.etree._Element
+        :returns: string - the command to run
+        """
+        pkgargs = " ".join(self.pkgtool[1][0] %
+                           tuple(pkg.get(field)
+                                 for field in self.pkgtool[1][1])
+                           for pkg in packages)
+        return self.pkgtool[0] % pkgargs
+
     def Install(self, packages, states):
         """ Run a one-pass install where all required packages are
         installed with a single command, followed by single package
@@ -464,14 +490,10 @@ class PkgTool(Tool):
         self.logger.info("Trying single pass package install for pkgtype %s" %
                          self.pkgtype)
 
-        data = [tuple([pkg.get(field) for field in self.pkgtool[1][1]])
-                for pkg in packages]
-        pkgargs = " ".join([self.pkgtool[1][0] % datum for datum in data])
-
-        self.logger.debug("Installing packages: %s" % pkgargs)
-        self.logger.debug("Running command: %s" % (self.pkgtool[0] % pkgargs))
+        pkgcmd = self._get_package_command(packages)
+        self.logger.debug("Running command: %s" % pkgcmd)
 
-        cmdrc = self.cmd.run(self.pkgtool[0] % pkgargs)[0]
+        cmdrc = self.cmd.run(pkgcmd)[0]
         if cmdrc == 0:
             self.logger.info("Single Pass Succeded")
             # set all package states to true and flush workqueues
@@ -481,7 +503,7 @@ class PkgTool(Tool):
                     and entry.get('type') == self.pkgtype
                     and entry.get('name') in pkgnames):
                     self.logger.debug('Setting state to true for pkg %s' %
-                                      (entry.get('name')))
+                                      entry.get('name'))
                     states[entry] = True
             self.RefreshPackages()
         else:
@@ -497,19 +519,13 @@ class PkgTool(Tool):
                 else:
                     self.logger.info("Installing pkg %s version %s" %
                                      (pkg.get('name'), pkg.get('version')))
-                    cmdrc = self.cmd.run(
-                        self.pkgtool[0] %
-                        (self.pkgtool[1][0] %
-                         tuple([pkg.get(field)
-                                for field in self.pkgtool[1][1]])))
-                    if cmdrc[0] == 0:
+                    if self.cmd.run(self._get_package_command([pkg]))[0] == 0:
                         states[pkg] = True
                     else:
                         self.logger.error("Failed to install package %s" %
-                                          (pkg.get('name')))
+                                          pkg.get('name'))
             self.RefreshPackages()
-        for entry in [ent for ent in packages if states[ent]]:
-            self.modified.append(entry)
+        self.modified.extend(entry for entry in packages if states[entry])
 
     def RefreshPackages(self):
         """ Refresh the internal representation of the package
@@ -603,11 +619,13 @@ class SvcTool(Tool):
         if self.setup['servicemode'] == 'disabled':
             return
 
-        for entry in [ent for ent in bundle if self.handlesEntry(ent)]:
-            restart = entry.get("restart", "true")
-            if (restart.lower() == "false" or
-                (restart.lower() == "interactive" and
-                 not self.setup['interactive'])):
+        for entry in bundle:
+            if not self.handlesEntry(entry):
+                continue
+
+            restart = entry.get("restart", "true").lower()
+            if (restart == "false" or
+                (restart == "interactive" and not self.setup['interactive'])):
                 continue
 
             rv = None
@@ -616,14 +634,8 @@ class SvcTool(Tool):
                     rv = self.stop_service(entry)
                 elif entry.get('name') not in self.restarted:
                     if self.setup['interactive']:
-                        prompt = ('Restart service %s?: (y/N): ' %
-                                  entry.get('name'))
-                        # flush input buffer
-                        while len(select.select([sys.stdin.fileno()], [], [],
-                                                0.0)[0]) > 0:
-                            os.read(sys.stdin.fileno(), 4096)
-                        ans = input(prompt)
-                        if ans not in ['y', 'Y']:
+                        if not Bcfg2.Client.prompt('Restart service %s? (y/N) '
+                                                   % entry.get('name')):
                             continue
                     rv = self.restart_service(entry)
                     if not rv:
@@ -639,8 +651,8 @@ class SvcTool(Tool):
         install_entries = []
         for entry in entries:
             if entry.get('install', 'true').lower() == 'false':
-                self.logger.info("Service %s installation is false. Skipping "
-                                 "installation." % (entry.get('name')))
+                self.logger.info("Installation is false for %s:%s, skipping" %
+                                 (entry.tag, entry.get('name')))
             else:
                 install_entries.append(entry)
         return Tool.Install(self, install_entries, states)
diff --git a/src/lib/Bcfg2/Client/__init__.py b/src/lib/Bcfg2/Client/__init__.py
index c03021f14..dd5ae1e83 100644
--- a/src/lib/Bcfg2/Client/__init__.py
+++ b/src/lib/Bcfg2/Client/__init__.py
@@ -1,3 +1,31 @@
 """This contains all Bcfg2 Client modules"""
 
 __all__ = ["Frame", "Tools", "XML", "Client"]
+
+import os
+import sys
+import select
+from Bcfg2.Compat import input  # pylint: disable=W0622
+
+
+def prompt(msg):
+    """ Helper to give a yes/no prompt to the user.  Flushes input
+    buffers, handles exceptions, etc.  Returns True if the user
+    answers in the affirmative, False otherwise.
+
+    :param msg: The message to show to the user.  The message is not
+                altered in any way for display; i.e., it should
+                contain "[y/N]" if desired, etc.
+    :type msg: string
+    :returns: bool - True if yes, False if no """
+    while len(select.select([sys.stdin.fileno()], [], [], 0.0)[0]) > 0:
+        os.read(sys.stdin.fileno(), 4096)
+    try:
+        ans = input(msg.encode(sys.stdout.encoding, 'replace'))
+        return ans in ['y', 'Y']
+    except EOFError:
+        # python 2.4.3 on CentOS doesn't like ^C for some reason
+        return False
+    except:
+        print("Error while reading input: %s" % sys.exc_info()[1])
+        return False