Make more tests run regardless of local plugin state

letsencrypt/cli.py

@@ -37,7 +37,7 @@ from letsencrypt import storage
 
 from letsencrypt.display import util as display_util
 from letsencrypt.display import ops as display_ops
-from letsencrypt.errors import Error, ConfiguratorError, CertStorageError
+from letsencrypt.errors import Error, PluginSelectionError, CertStorageError
 from letsencrypt.plugins import disco as plugins_disco
 
 
@@ -312,7 +312,7 @@ def set_configurator(previously, now):
     if previously:
         if previously != now:
             msg = "Too many flags setting configurators/installers/authenticators %s -> %s"
-            raise ConfiguratorError, msg % (`previously`, `now`)
+            raise PluginSelectionError, msg % (`previously`, `now`)
     return now
 
 def diagnose_configurator_problem(cfg_type, requested, plugins):
@@ -323,25 +323,25 @@ def diagnose_configurator_problem(cfg_type, requested, plugins):
     :param string requested: the plugin that was requested
     :param PluginRegistry plugins: available plugins
 
-    :raises error.ConfiguratorError: if there was a problem
+    :raises error.PluginSelectionError: if there was a problem
     """
 
     if requested:
         if requested not in plugins:
             msg = "The requested {0} plugin does not appear to be installed".format(requested)
-            raise ConfiguratorError, msg
+            raise PluginSelectionError, msg
         else:
             msg = ("The {0} plugin is not working; there may be problems with "
                    "your existing configuration").format(requested)
-            raise ConfiguratorError, msg
+            raise PluginSelectionError, msg
-    raise ConfiguratorError, "{0} could not be determined or is not installed".format(cfg_type)
+    raise PluginSelectionError, "{0} could not be determined or is not installed".format(cfg_type)
 
 
 def choose_configurator_plugins(args, config, plugins, verb):
     """
     Figure out which configurator we're going to use
 
-    :raises error.ConfiguratorError if there was a problem
+    :raises error.PluginSelectionError if there was a problem
     """
 
     # Which plugins do we need?
@@ -352,7 +352,7 @@ def choose_configurator_plugins(args, config, plugins, verb):
         need_inst = True
         if args.authenticator:
             msg = "Specifying an authenticator doesn't make sense in install mode"
-            raise ConfiguratorError, msg
+            raise PluginSelectionError, msg
 
     # Which plugins did the user request?
     req_inst = req_auth = args.configurator
@@ -393,8 +393,7 @@ def run(args, config, plugins):  # pylint: disable=too-many-branches,too-many-lo
     """Obtain a certificate and install."""
     try:
         installer, authenticator = choose_configurator_plugins(args, config, plugins, "run")
-    except ConfiguratorError, e:
+    except PluginSelectionError, e:
-        logger.warn("Exiting with message {0}".format(e.message))
         return e.message
 
     domains = _find_domains(args, installer)
@@ -426,7 +425,7 @@ def auth(args, config, plugins):
     try:
         # installers are used in auth mode to determine domain names
         installer, authenticator = choose_configurator_plugins(args, config, plugins, "auth")
-    except ConfiguratorError, e:
+    except PluginSelectionError, e:
         return e.message
 
     # TODO: Handle errors from _init_le_client?
@@ -450,7 +449,7 @@ def install(args, config, plugins):
 
     try:
         installer, _ = choose_configurator_plugins(args, config, plugins, "auth")
-    except ConfiguratorError, e:
+    except PluginSelectionError, e:
         return e.message
 
     if args.authenticator:
@@ -1007,6 +1006,8 @@ def _handle_exception(exc_type, exc_value, trace, args):
             traceback.format_exception(exc_type, exc_value, trace)))
 
 
+# this copy of plugins can be mocked out
+plugins_testable = plugins_disco.PluginsRegistry.find_all()
 def main(cli_args=sys.argv[1:]):
     """Command line argument parsing and main script execution."""
     sys.excepthook = functools.partial(_handle_exception, args=None)
@@ -1066,8 +1067,11 @@ def main(cli_args=sys.argv[1:]):
         #    "{0}Root is required to run letsencrypt.  Please use sudo.{0}"
         #    .format(os.linesep))
 
-    return args.func(args, config, plugins)
+    return args.func(args, config, plugins_testable)
 
 
 if __name__ == "__main__":
-    sys.exit(main())  # pragma: no cover
+    err_string = main()
+    if err_string:
+        logger.warn("Exiting with message %s", err_string)
+    sys.exit(err_string)  # pragma: no cover

letsencrypt/errors.py

@@ -24,9 +24,6 @@ class SubprocessError(Error):
 class CertStorageError(Error):
     """Generic `.CertStorage` error."""
 
-class ConfiguratorError(Error):
-    """A problem with plugin/configurator selection or setup"""
-
 # Auth Handler Errors
 class AuthorizationError(Error):
     """Authorization error."""
@@ -67,6 +64,8 @@ class DvsniError(DvAuthError):
 class PluginError(Error):
     """Let's Encrypt Plugin error."""
 
+class PluginSelectionError(Error):
+    """A problem with plugin/configurator selection or setup"""
 
 class NoInstallationError(PluginError):
     """Let's Encrypt No Installation error."""

letsencrypt/tests/cli_test.py

@@ -95,24 +95,26 @@ class CLITest(unittest.TestCase):
             self.assertTrue(cli.USAGE in out)
 
     def test_configurator_selection(self):
-        plugins = disco.PluginsRegistry.find_all()
+        real_plugins = disco.PluginsRegistry.find_all()
         args = ['--agree-eula', '--apache', '--authenticator', 'standalone']
-        ret, _, _, _ = self._call(args)
-        # TODO replace these cases with .mockery to test both paths regardless
-        # of what's actually installed
-        if "apache" in plugins:
-            self.assertTrue("Too many flags setting" in ret)
-        else:
-            self.assertTrue("The requested apache plugin does not appear" in ret)
 
+        with mock.patch('letsencrypt.cli.plugins_testable') as plugins:
+            plugins.return_value = {"apache": True, "nginx": True}
+            ret, _, _, _ = self._call(args)
+            self.assertTrue("Too many flags setting" in ret)
+
+        if "nginx" in real_plugins:
             # Sending nginx a non-existent conf dir will simulate misconfiguration
+            # (we can only do that if letsencrypt-nginx is actually present)
             args = ["install", "--nginx", "--cert-path", "/tmp/blah", "--key-path", "/tmp/blah",
                     "--nginx-server-root", "/nonexistent/thing"]
             ret, _, _, _ = self._call(args)
-
-        if "nginx" in plugins:
             self.assertTrue("The nginx plugin is not working" in ret)
-        else:
+
+        # But we can pretend that nginx is uninstalled, even if it is
+        with mock.patch('letsencrypt.cli.plugins_testable') as plugins:
+            plugins.return_value = {}
+            ret, _, _, _ = self._call(args)
             self.assertTrue("The requested nginx plugin does not appear" in ret)
 
     def test_rollback(self):