Some corrections

certbot-ci/certbot_integration_tests/certbot_tests/assertions.py

@@ -42,7 +42,8 @@ def assert_equals_permissions(file1, file2, mask):
     Assert that permissions on two files are identical in respect to a given umask.
     :param file1: first file path to compare
     :param file2: second file path to compare
-    :param mask: umask to apply before comparing file modes
+    :param mask: 3-octal representation of a POSIX umask under which the two files mode
+                 should match (eg. 0o074 will test RWX on group and R on world)
     """
     mode_file1 = os.stat(file1).st_mode & mask
     mode_file2 = os.stat(file2).st_mode & mask

certbot-ci/certbot_integration_tests/certbot_tests/context.py

@@ -16,17 +16,20 @@ class IntegrationTestsContext(object):
 
         if hasattr(request.config, 'slaveinput'):  # Worker node
             self.worker_id = request.config.slaveinput['slaveid']
-            self.acme_xdist = request.config.slaveinput['acme_xdist']
+            acme_xdist = request.config.slaveinput['acme_xdist']
         else:  # Primary node
             self.worker_id = 'primary'
-            self.acme_xdist = request.config.acme_xdist
+            acme_xdist = request.config.acme_xdist
 
-        self.acme_server =self.acme_xdist['acme_server']
-        self.directory_url = self.acme_xdist['directory_url']
-        self.tls_alpn_01_port = self.acme_xdist['https_port'][self.worker_id]
-        self.http_01_port = self.acme_xdist['http_port'][self.worker_id]
-        self.challtestsrv_mgt_port = self.acme_xdist['challtestsrv_port']
+        self.acme_server =acme_xdist['acme_server']
+        self.directory_url = acme_xdist['directory_url']
+        self.tls_alpn_01_port = acme_xdist['https_port'][self.worker_id]
+        self.http_01_port = acme_xdist['http_port'][self.worker_id]
+        self.challtestsrv_mgt_port = acme_xdist['challtestsrv_port']
 
+        # Formally certbot version does not depend on the test context. But get its value requires
+        # to call certbot from a subprocess. Since it will be called a lot of time through
+        # _common_test_no_force_renew, we cache its value as a member of the fixture context.
         self.certbot_version = misc.get_certbot_version()
 
         self.workspace = tempfile.mkdtemp()
@@ -120,7 +123,7 @@ class IntegrationTestsContext(object):
         command.extend(args)
         return self.certbot_no_force_renew(command)
 
-    def domain(self, subdomain='le'):
+    def get_domain(self, subdomain='le'):
         """
         Generate a certificate domain name suitable for distributed certbot integration tests.
         This is a requirement to let the distribution know how to redirect the challenge check

certbot-ci/certbot_integration_tests/certbot_tests/test_main.py

@@ -25,7 +25,7 @@ def context(request):
 
 def test_manual_dns_auth(context):
     """Test the DNS-01 challenge using manual plugin."""
-    certname = context.domain('dns')
+    certname = context.get_domain('dns')
     context.certbot([
         '-a', 'manual', '-d', certname, '--preferred-challenges', 'dns',
         'run', '--cert-name', certname,
@@ -46,7 +46,7 @@ def test_renew(context):
     # First, we create a target certificate, with all hook dirs instantiated.
     # We should have a new certificate, with hooks executed.
     # Check also file permissions.
-    certname = context.domain('renew')
+    certname = context.get_domain('renew')
     context.certbot([
         'certonly', '-d', certname, '--rsa-key-size', '4096',
         '--preferred-challenges', 'http-01'

certbot-ci/certbot_integration_tests/utils/misc.py

@@ -66,6 +66,8 @@ def create_http_server(port):
     process = multiprocessing.Process(target=run)
 
     try:
+        # SimpleHTTPServer is designed to serve files from the current working directory at the
+        # time it starts. So we temporarily change the cwd to our crafted webroot before launch.
         try:
             os.chdir(webroot)
             process.start()
@@ -111,6 +113,10 @@ def generate_test_file_hooks(config_dir, hook_probe):
     renewal_hooks_dirs = list_renewal_hooks_dirs(config_dir)
 
     for hook_dir in renewal_hooks_dirs:
+        # We want a equivalent of bash `chmod -p $HOOK_DIR, that does not fail if one folder of
+        # the hierarchy already exists. It is not the case of os.makedirs. Python 3 has an
+        # optional parameter `exists_ok` to not fail on existing dir, but Python 2.7 does not.
+        # So we pass through a try except pass for it. To be removed with dropped support on py27.
         try:
             os.makedirs(hook_dir)
         except OSError as error:

certbot/plugins/dns_common_lexicon.py

@@ -83,7 +83,7 @@ class LexiconClient(object):
                     self.provider.options['domain'] = domain_name
                 else:
                     # For Lexicon 3.x
-                    self.provider.domain = domain_name
+                    self.provider.get_domain = domain_name
 
                 self.provider.authenticate()