refine vhost display

letsencrypt_apache/configurator.py

@@ -929,7 +929,6 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         try:
             # Use check_output so the command will finish before reloading
             # TODO: a2enmod is debian specific...
-            self.
             subprocess.check_call([self.conf("enmod"), mod_name],
                                   stdout=open("/dev/null", "w"),
                                   stderr=open("/dev/null", "w"))

letsencrypt_apache/display_ops.py

@@ -1,3 +1,4 @@
+"""Contains UI methods for Apache operations."""
 import os
 import zope.component
 
@@ -38,26 +39,31 @@ def _vhost_menu(domain, vhosts):
     :rtype: `tuple`
 
     """
+    filename_size = 24
+    disp_name_size = 17
     choices = []
     for vhost in vhosts:
-        if vhost.names == 1:
+        if len(vhost.names) == 1:
             disp_name = next(iter(vhost.names))
-        elif vhost.names == 0:
+        elif len(vhost.names) == 0:
             disp_name = ""
         else:
             disp_name = "Multiple Names"
 
         choices.append(
-            "%s | %s | %s | %s" % (
-                os.path.basename(vhost.filep),
-                disp_name,
-                "HTTPS" if vhost.ssl,
-                "Enabled" if vhost.enabled)
+            "{0:{4}s} | {1:{5}s} | {2:5s} | {3:7s}".format(
+                os.path.basename(vhost.filep)[:filename_size],
+                disp_name[:disp_name_size],
+                "HTTPS" if vhost.ssl else "",
+                "Enabled" if vhost.enabled else "",
+                filename_size,
+                disp_name_size)
         )
 
     code, tag = zope.component.getUtility(interfaces.IDisplay).menu(
-        "We were unable to find a vhost with a Servername or Address of %s."
-        "Which virtual host would you like to choose?" % domain,
+        "We were unable to find a vhost with a ServerName or Address of {0}.{1}"
+        "Which virtual host would you like to choose?".format(
+            domain, os.linesep),
         choices, help_label="More Info", ok_label="Select")
 
     return code, tag
@@ -65,6 +71,6 @@ def _vhost_menu(domain, vhosts):
 
 def _more_info_vhost(vhost):
     zope.component.getUtility(interfaces.IDisplay).notification(
-        "Virtual Host Information:{0}{1}".format(
-            os.linesep, str(vhost)),
-        height=display_util.HEIGHT)
+        "Virtual Host Information:{0}{1}{0}{2}".format(
+            os.linesep, "-" * (display_util.WIDTH - 4), str(vhost)),
+        height=display_util.HEIGHT)

letsencrypt_apache/dvsni.py

@@ -1,5 +1,4 @@
 """ApacheDVSNI"""
-import logging
 import os
 
 from letsencrypt.plugins import common

letsencrypt_apache/obj.py

@@ -31,14 +31,19 @@ class VirtualHost(object):  # pylint: disable=too-few-public-methods
         self.names.add(name)
 
     def __str__(self):
-        addr_str = ", ".join(str(addr) for addr in self.addrs)
-        return ("File: %s\n"
-                "Vhost path: %s\n"
-                "Addresses: %s\n"
-                "Names: %s\n"
-                "TLS: %s\n"
-                "Enabled: %s" % (self.filep, self.path, addr_str,
-                                 self.names, self.ssl, self.enabled))
+        return (
+            "File: %s\n"
+            "Vhost path: %s\n"
+            "Addresses: %s\n"
+            "Names: %s\n"
+            "TLS Enabled: %s\n"
+            "Site Enabled: %s" % (
+                self.filep,
+                self.path,
+                ", ".join(str(addr) for addr in self.addrs),
+                ", ".join(name for name in self.names),
+                "Yes" if self.ssl else "No",
+                "Yes" if self.enabled else "No"))
 
     def __eq__(self, other):
         if isinstance(other, self.__class__):

letsencrypt_apache/tests/display_ops_test.py

@@ -1,7 +1,9 @@
 """Test letsencrypt_apache.display_ops."""
+import sys
 import unittest
 
 import mock
+import zope.component
 
 from letsencrypt_apache.tests import util
 
@@ -12,6 +14,7 @@ class SelectVhostTest(unittest.TestCase):
     """Tests for letsencrypt_apache.display_ops.select_vhost."""
 
     def setUp(self):
+        zope.component.provideUtility(display_util.FileDisplay(sys.stdout))
         self.base_dir = "/example_path"
         self.vhosts = util.get_vh_truth(
             self.base_dir, "debian_apache_2_4/two_vhost_80")
@@ -19,12 +22,12 @@ class SelectVhostTest(unittest.TestCase):
     @classmethod
     def _call(cls, vhosts):
         from letsencrypt_apache.display_ops import select_vhost
-        select_vhost("example.com", vhosts)
+        return select_vhost("example.com", vhosts)
 
     @mock.patch("letsencrypt_apache.display_ops.zope.component.getUtility")
     def test_successful_choice(self, mock_util):
-        mock_util().menu.return_value = (display_util.OK, 1)
-        self.assertEqual(self.vhosts[1], self._call(self.vhosts))
+        mock_util().menu.return_value = (display_util.OK, 3)
+        self.assertEqual(self.vhosts[3], self._call(self.vhosts))
 
     @mock.patch("letsencrypt_apache.display_ops.zope.component.getUtility")
     def test_more_info_cancel(self, mock_util):
@@ -34,8 +37,11 @@ class SelectVhostTest(unittest.TestCase):
             (display_util.CANCEL, -1),
         ]
 
-        self.assertEqual(None, self._call())
+        self.assertEqual(None, self._call(self.vhosts))
         self.assertEqual(mock_util().notification.call_count, 2)
 
     def test_no_vhosts(self):
-        self.assertEqual(self._call([]), None)
+        self.assertEqual(self._call([]), None)
+
+if __name__ == "__main__":
+    unittest.main()  # pragma: no cover