Python 3 compatibility for all tests (#4358)

9 files changed, 42 insertions, 26 deletions

diff --git a/certbot-nginx/certbot_nginx/configurator.py b/certbot-nginx/certbot_nginx/configurator.py
index 46ce18ab2..e62194d4f 100644
--- a/certbot-nginx/certbot_nginx/configurator.py
+++ b/certbot-nginx/certbot_nginx/configurator.py
@@ -9,6 +9,7 @@ import tempfile
 import time
 
 import OpenSSL
+import six
 import zope.interface
 
 from acme import challenges
@@ -263,7 +264,7 @@ class NginxConfigurator(common.Plugin):
         """
         if not matches:
             return None
-        elif matches[0]['rank'] in xrange(2, 6):
+        elif matches[0]['rank'] in six.moves.range(2, 6):
             # Wildcard match - need to find the longest one
             rank = matches[0]['rank']
             wildcards = [x for x in matches if x['rank'] == rank]
diff --git a/certbot-nginx/certbot_nginx/obj.py b/certbot-nginx/certbot_nginx/obj.py
index 29fa976f3..849cefe1f 100644
--- a/certbot-nginx/certbot_nginx/obj.py
+++ b/certbot-nginx/certbot_nginx/obj.py
@@ -1,6 +1,8 @@
 """Module contains classes used by the Nginx Configurator."""
 import re
 
+import six
+
 from certbot.plugins import common
 
 REDIRECT_DIRECTIVES = ['return', 'rewrite']
@@ -97,6 +99,11 @@ class Addr(common.Addr):
     def __repr__(self):
         return "Addr(" + self.__str__() + ")"
 
+    def __hash__(self):
+        # Python 3 requires explicit overridden for __hash__
+        # See certbot-apache/certbot_apache/obj.py for more information
+        return super(Addr, self).__hash__()
+
     def super_eq(self, other):
         """Check ip/port equality, with IPv6 support.
         """
@@ -147,13 +154,15 @@ class VirtualHost(object):  # pylint: disable=too-few-public-methods
         self.path = path
 
     def __str__(self):
-        addr_str = ", ".join(str(addr) for addr in self.addrs)
+        addr_str = ", ".join(str(addr) for addr in sorted(self.addrs, key=str))
+        # names might be a set, and it has different representations in Python
+        # 2 and 3. Force it to be a list here for consistent outputs
         return ("file: %s\n"
                 "addrs: %s\n"
                 "names: %s\n"
                 "ssl: %s\n"
                 "enabled: %s" % (self.filep, addr_str,
-                                 self.names, self.ssl, self.enabled))
+                                 list(self.names), self.ssl, self.enabled))
 
     def __repr__(self):
         return "VirtualHost(" + self.__str__().replace("\n", ", ") + ")\n"
@@ -161,7 +170,7 @@ class VirtualHost(object):  # pylint: disable=too-few-public-methods
     def __eq__(self, other):
         if isinstance(other, self.__class__):
             return (self.filep == other.filep and
-                    list(self.addrs) == list(other.addrs) and
+                    sorted(self.addrs, key=str) == sorted(other.addrs, key=str) and
                     self.names == other.names and
                     self.ssl == other.ssl and
                     self.enabled == other.enabled and
@@ -181,7 +190,7 @@ class VirtualHost(object):  # pylint: disable=too-few-public-methods
     def contains_list(self, test):
         """Determine if raw server block contains test list at top level
         """
-        for i in xrange(0, len(self.raw) - len(test)):
+        for i in six.moves.range(0, len(self.raw) - len(test)):
             if self.raw[i:i + len(test)] == test:
                 return True
         return False
diff --git a/certbot-nginx/certbot_nginx/parser.py b/certbot-nginx/certbot_nginx/parser.py
index eddc7b9b0..fd4ea4f11 100644
--- a/certbot-nginx/certbot_nginx/parser.py
+++ b/certbot-nginx/certbot_nginx/parser.py
@@ -342,7 +342,7 @@ class NginxParser(object):
             vhost.names = parsed_server['names']
             vhost.raw = new_server
         except errors.MisconfigurationError as err:
-            raise errors.MisconfigurationError("Problem in %s: %s" % (filename, err.message))
+            raise errors.MisconfigurationError("Problem in %s: %s" % (filename, str(err)))
 
 
 def _do_for_subarray(entry, condition, func, path=None):
diff --git a/certbot-nginx/certbot_nginx/tests/configurator_test.py b/certbot-nginx/certbot_nginx/tests/configurator_test.py
index cc36aa0de..d491d2a15 100644
--- a/certbot-nginx/certbot_nginx/tests/configurator_test.py
+++ b/certbot-nginx/certbot_nginx/tests/configurator_test.py
@@ -27,12 +27,13 @@ class NginxConfiguratorTest(util.NginxTest):
         super(NginxConfiguratorTest, self).setUp()
 
         self.config = util.get_nginx_configurator(
-            self.config_path, self.config_dir, self.work_dir)
+            self.config_path, self.config_dir, self.work_dir, self.logs_dir)
 
     def tearDown(self):
         shutil.rmtree(self.temp_dir)
         shutil.rmtree(self.config_dir)
         shutil.rmtree(self.work_dir)
+        shutil.rmtree(self.logs_dir)
 
     @mock.patch("certbot_nginx.configurator.util.exe_exists")
     def test_prepare_no_install(self, mock_exe_exists):
@@ -261,13 +262,13 @@ class NginxConfiguratorTest(util.NginxTest):
         # Note: As more challenges are offered this will have to be expanded
         achall1 = achallenges.KeyAuthorizationAnnotatedChallenge(
             challb=messages.ChallengeBody(
-                chall=challenges.TLSSNI01(token="kNdwjwOeX0I_A8DXt9Msmg"),
+                chall=challenges.TLSSNI01(token=b"kNdwjwOeX0I_A8DXt9Msmg"),
                 uri="https://ca.org/chall0_uri",
                 status=messages.Status("pending"),
             ), domain="localhost", account_key=self.rsa512jwk)
         achall2 = achallenges.KeyAuthorizationAnnotatedChallenge(
             challb=messages.ChallengeBody(
-                chall=challenges.TLSSNI01(token="m8TdO1qik4JVFtgPPurJmg"),
+                chall=challenges.TLSSNI01(token=b"m8TdO1qik4JVFtgPPurJmg"),
                 uri="https://ca.org/chall1_uri",
                 status=messages.Status("pending"),
             ), domain="example.com", account_key=self.rsa512jwk)
diff --git a/certbot-nginx/certbot_nginx/tests/nginxparser_test.py b/certbot-nginx/certbot_nginx/tests/nginxparser_test.py
index e83b414cf..cf4bbde39 100644
--- a/certbot-nginx/certbot_nginx/tests/nginxparser_test.py
+++ b/certbot-nginx/certbot_nginx/tests/nginxparser_test.py
@@ -128,7 +128,7 @@ class TestRawNginxParser(unittest.TestCase):
                                  [['root', ' ', 'html'],
                                   ['index', ' ', 'index.html index.htm']]]]]))
 
-        with tempfile.TemporaryFile() as f:
+        with tempfile.TemporaryFile(mode='w+t') as f:
             dump(parsed, f)
             f.seek(0)
             parsed_new = load(f)
@@ -138,7 +138,7 @@ class TestRawNginxParser(unittest.TestCase):
         with open(util.get_data_filename('minimalistic_comments.conf')) as handle:
             parsed = load(handle)
 
-        with tempfile.TemporaryFile() as f:
+        with tempfile.TemporaryFile(mode='w+t') as f:
             dump(parsed, f)
             f.seek(0)
             parsed_new = load(f)
diff --git a/certbot-nginx/certbot_nginx/tests/obj_test.py b/certbot-nginx/certbot_nginx/tests/obj_test.py
index b0a2d5ad8..069f860d6 100644
--- a/certbot-nginx/certbot_nginx/tests/obj_test.py
+++ b/certbot-nginx/certbot_nginx/tests/obj_test.py
@@ -158,7 +158,7 @@ class VirtualHostTest(unittest.TestCase):
 
     def test_str(self):
         stringified = '\n'.join(['file: filep', 'addrs: localhost',
-                                 "names: set(['localhost'])", 'ssl: False',
+                                 "names: ['localhost']", 'ssl: False',
                                  'enabled: False'])
         self.assertEqual(stringified, str(self.vhost1))
 
diff --git a/certbot-nginx/certbot_nginx/tests/tls_sni_01_test.py b/certbot-nginx/certbot_nginx/tests/tls_sni_01_test.py
index e7dacb400..7a2de44a2 100644
--- a/certbot-nginx/certbot_nginx/tests/tls_sni_01_test.py
+++ b/certbot-nginx/certbot_nginx/tests/tls_sni_01_test.py
@@ -3,6 +3,7 @@ import unittest
 import shutil
 
 import mock
+import six
 
 from acme import challenges
 
@@ -23,25 +24,25 @@ class TlsSniPerformTest(util.NginxTest):
     achalls = [
         achallenges.KeyAuthorizationAnnotatedChallenge(
             challb=acme_util.chall_to_challb(
-                challenges.TLSSNI01(token="kNdwjwOeX0I_A8DXt9Msmg"), "pending"),
+                challenges.TLSSNI01(token=b"kNdwjwOeX0I_A8DXt9Msmg"), "pending"),
             domain="www.example.com", account_key=account_key),
         achallenges.KeyAuthorizationAnnotatedChallenge(
             challb=acme_util.chall_to_challb(
                 challenges.TLSSNI01(
-                    token="\xba\xa9\xda?<m\xaewmx\xea\xad\xadv\xf4\x02\xc9y"
-                          "\x80\xe2_X\t\xe7\xc7\xa4\t\xca\xf7&\x945"
+                    token=b"\xba\xa9\xda?<m\xaewmx\xea\xad\xadv\xf4\x02\xc9y"
+                          b"\x80\xe2_X\t\xe7\xc7\xa4\t\xca\xf7&\x945"
                 ), "pending"),
             domain="another.alias", account_key=account_key),
         achallenges.KeyAuthorizationAnnotatedChallenge(
             challb=acme_util.chall_to_challb(
                 challenges.TLSSNI01(
-                    token="\x8c\x8a\xbf_-f\\cw\xee\xd6\xf8/\xa5\xe3\xfd"
-                          "\xeb9\xf1\xf5\xb9\xefVM\xc9w\xa4u\x9c\xe1\x87\xb4"
+                    token=b"\x8c\x8a\xbf_-f\\cw\xee\xd6\xf8/\xa5\xe3\xfd"
+                          b"\xeb9\xf1\xf5\xb9\xefVM\xc9w\xa4u\x9c\xe1\x87\xb4"
                 ), "pending"),
             domain="www.example.org", account_key=account_key),
         achallenges.KeyAuthorizationAnnotatedChallenge(
             challb=acme_util.chall_to_challb(
-                challenges.TLSSNI01(token="kNdwjxOeX0I_A8DXt9Msmg"), "pending"),
+                challenges.TLSSNI01(token=b"kNdwjxOeX0I_A8DXt9Msmg"), "pending"),
             domain="sslon.com", account_key=account_key),
     ]
 
@@ -49,7 +50,7 @@ class TlsSniPerformTest(util.NginxTest):
         super(TlsSniPerformTest, self).setUp()
 
         config = util.get_nginx_configurator(
-            self.config_path, self.config_dir, self.work_dir)
+            self.config_path, self.config_dir, self.work_dir, self.logs_dir)
 
         from certbot_nginx import tls_sni_01
         self.sni = tls_sni_01.NginxTlsSni01(config)
@@ -117,7 +118,7 @@ class TlsSniPerformTest(util.NginxTest):
             util.contains_at_depth(http, ['server_name', 'another.alias'], 3))
 
         self.assertEqual(len(sni_responses), 4)
-        for i in xrange(4):
+        for i in six.moves.range(4):
             self.assertEqual(sni_responses[i], acme_responses[i])
 
     def test_mod_config(self):
@@ -148,7 +149,7 @@ class TlsSniPerformTest(util.NginxTest):
             else:
                 response = self.achalls[2].response(self.account_key)
                 self.assertEqual(vhost.addrs, set(v_addr2_print))
-            self.assertEqual(vhost.names, set([response.z_domain]))
+            self.assertEqual(vhost.names, set([response.z_domain.decode('ascii')]))
 
         self.assertEqual(len(vhs), 2)
 
diff --git a/certbot-nginx/certbot_nginx/tests/util.py b/certbot-nginx/certbot_nginx/tests/util.py
index 2fb866b77..259dc1f10 100644
--- a/certbot-nginx/certbot_nginx/tests/util.py
+++ b/certbot-nginx/certbot_nginx/tests/util.py
@@ -2,6 +2,7 @@
 import copy
 import os
 import pkg_resources
+import tempfile
 import unittest
 
 import mock
@@ -27,6 +28,7 @@ class NginxTest(unittest.TestCase):  # pylint: disable=too-few-public-methods
 
         self.temp_dir, self.config_dir, self.work_dir = common.dir_setup(
             "etc_nginx", "certbot_nginx.tests")
+        self.logs_dir = tempfile.mkdtemp('logs')
 
         self.ssl_options = common.setup_ssl_options(
             self.config_dir, constants.MOD_SSL_CONF_SRC,
@@ -46,7 +48,7 @@ def get_data_filename(filename):
 
 
 def get_nginx_configurator(
-        config_path, config_dir, work_dir, version=(1, 6, 2)):
+        config_path, config_dir, work_dir, logs_dir, version=(1, 6, 2)):
     """Create an Nginx Configurator with the specified options."""
 
     backups = os.path.join(work_dir, "backups")
@@ -62,6 +64,7 @@ def get_nginx_configurator(
                     le_vhost_ext="-le-ssl.conf",
                     config_dir=config_dir,
                     work_dir=work_dir,
+                    logs_dir=logs_dir,
                     backup_dir=backups,
                     temp_checkpoint_dir=os.path.join(work_dir, "temp_checkpoints"),
                     in_progress_dir=os.path.join(backups, "IN_PROGRESS"),
@@ -111,7 +114,7 @@ def contains_at_depth(haystack, needle, n):
     """
     # Specifically use hasattr rather than isinstance(..., collections.Iterable)
     # because we want to include lists but reject strings.
-    if not hasattr(haystack, '__iter__'):
+    if not hasattr(haystack, '__iter__') or hasattr(haystack, 'strip'):
         return False
     if n == 0:
         return needle in haystack
diff --git a/certbot-nginx/certbot_nginx/tls_sni_01.py b/certbot-nginx/certbot_nginx/tls_sni_01.py
index dec21e791..eedd97c03 100644
--- a/certbot-nginx/certbot_nginx/tls_sni_01.py
+++ b/certbot-nginx/certbot_nginx/tls_sni_01.py
@@ -1,9 +1,10 @@
 """A class that performs TLS-SNI-01 challenges for Nginx"""
 
-import itertools
 import logging
 import os
 
+import six
+
 from certbot import errors
 from certbot.plugins import common
 
@@ -113,7 +114,7 @@ class NginxTlsSni01(common.TLSSNI01):
                 'TLS-SNI-01 challenges in %s.' % root)
 
         config = [self._make_server_block(pair[0], pair[1])
-                  for pair in itertools.izip(self.achalls, ll_addrs)]
+                  for pair in six.moves.zip(self.achalls, ll_addrs)]
         config = nginxparser.UnspacedList(config)
 
         self.configurator.reverter.register_file_creation(
@@ -142,7 +143,7 @@ class NginxTlsSni01(common.TLSSNI01):
         block = [['listen', ' ', addr.to_string(include_default=False)] for addr in addrs]
 
         block.extend([['server_name', ' ',
-                       achall.response(achall.account_key).z_domain],
+                       achall.response(achall.account_key).z_domain.decode('ascii')],
                       # access and error logs necessary for
                       # integration testing (non-root)
                       ['access_log', ' ', os.path.join(