Merge branch 'master' into log-manual-hooks

CHANGELOG.md

@@ -16,6 +16,7 @@ Certbot adheres to [Semantic Versioning](https://semver.org/).
   command line defaults.
 * The running of manual plugin hooks is now always included in Certbot's log
   output.
+* Tests execution for certbot, certbot-apache and certbot-nginx packages now relies on pytest.
 
 ### Fixed
 
@@ -28,6 +29,7 @@ package with changes other than its version number was:
 * acme
 * certbot
 * certbot-apache
+* certbot-nginx
 
 More details about these changes can be found on our GitHub repo.
 

acme/setup.py

@@ -36,6 +36,7 @@ docs_extras = [
     'sphinx_rtd_theme',
 ]
 
+
 class PyTest(TestCommand):
     user_options = []
 
@@ -50,6 +51,7 @@ class PyTest(TestCommand):
         errno = pytest.main(shlex.split(self.pytest_args))
         sys.exit(errno)
 
+
 setup(
     name='acme',
     version=version,
@@ -82,7 +84,7 @@ setup(
         'dev': dev_extras,
         'docs': docs_extras,
     },
-    tests_require=["pytest"],
     test_suite='acme',
+    tests_require=["pytest"],
     cmdclass={"test": PyTest},
 )

certbot-apache/setup.py

@@ -1,5 +1,7 @@
 from setuptools import setup
 from setuptools import find_packages
+from setuptools.command.test import test as TestCommand
+import sys
 
 
 version = '0.32.0.dev0'
@@ -21,6 +23,22 @@ docs_extras = [
     'sphinx_rtd_theme',
 ]
 
+
+class PyTest(TestCommand):
+    user_options = []
+
+    def initialize_options(self):
+        TestCommand.initialize_options(self)
+        self.pytest_args = ''
+
+    def run_tests(self):
+        import shlex
+        # import here, cause outside the eggs aren't loaded
+        import pytest
+        errno = pytest.main(shlex.split(self.pytest_args))
+        sys.exit(errno)
+
+
 setup(
     name='certbot-apache',
     version=version,
@@ -64,4 +82,6 @@ setup(
         ],
     },
     test_suite='certbot_apache',
+    tests_require=["pytest"],
+    cmdclass={"test": PyTest},
 )

certbot-nginx/setup.py

@@ -1,5 +1,7 @@
 from setuptools import setup
 from setuptools import find_packages
+from setuptools.command.test import test as TestCommand
+import sys
 
 
 version = '0.32.0.dev0'
@@ -21,6 +23,22 @@ docs_extras = [
     'sphinx_rtd_theme',
 ]
 
+
+class PyTest(TestCommand):
+    user_options = []
+
+    def initialize_options(self):
+        TestCommand.initialize_options(self)
+        self.pytest_args = ''
+
+    def run_tests(self):
+        import shlex
+        # import here, cause outside the eggs aren't loaded
+        import pytest
+        errno = pytest.main(shlex.split(self.pytest_args))
+        sys.exit(errno)
+
+
 setup(
     name='certbot-nginx',
     version=version,
@@ -64,4 +82,6 @@ setup(
         ],
     },
     test_suite='certbot_nginx',
+    tests_require=["pytest"],
+    cmdclass={"test": PyTest},
 )

certbot/tests/lock_test.py

@@ -41,6 +41,7 @@ class LockFileTest(test_util.TempDirTestCase):
         super(LockFileTest, self).setUp()
         self.lock_path = os.path.join(self.tempdir, 'test.lock')
 
+    @test_util.broken_on_windows
     def test_acquire_without_deletion(self):
         # acquire the lock in another process but don't delete the file
         child = multiprocessing.Process(target=self._call,
@@ -58,6 +59,7 @@ class LockFileTest(test_util.TempDirTestCase):
             self.assertRaises, errors.LockError, self._call, self.lock_path)
         test_util.lock_and_call(assert_raises, self.lock_path)
 
+    @test_util.broken_on_windows
     def test_locked_repr(self):
         lock_file = self._call(self.lock_path)
         locked_repr = repr(lock_file)
@@ -92,6 +94,7 @@ class LockFileTest(test_util.TempDirTestCase):
             self._call(self.lock_path)
         self.assertFalse(should_delete)
 
+    @test_util.broken_on_windows
     def test_removed(self):
         lock_file = self._call(self.lock_path)
         lock_file.release()

certbot/tests/util.py

@@ -3,13 +3,14 @@
 .. warning:: This module is not part of the public API.
 
 """
+import logging
 import os
 import pkg_resources
 import shutil
+import stat
 import tempfile
 import unittest
 import sys
-import warnings
 from multiprocessing import Process, Event
 
 from cryptography.hazmat.backends import default_backend
@@ -329,23 +330,25 @@ class TempDirTestCase(unittest.TestCase):
 
     def tearDown(self):
         """Execute after test"""
-        # On Windows we have various files which are not correctly closed at the time of tearDown.
-        # For know, we log them until a proper file close handling is written.
-        # Useful for development only, so no warning when we are on a CI process.
-        def onerror_handler(_, path, excinfo):
-            """On error handler"""
-            if not os.environ.get('APPVEYOR'): # pragma: no cover
-                message = ('Following error occurred when deleting the tempdir {0}'
-                           ' for path {1} during tearDown process: {2}'
-                           .format(self.tempdir, path, str(excinfo)))
-                warnings.warn(message)
-        shutil.rmtree(self.tempdir, onerror=onerror_handler)
+        # Cleanup opened resources after a test. This is usually done through atexit handlers in
+        # Certbot, but during tests, atexit will not run registered functions before tearDown is
+        # called and instead will run them right before the entire test process exits.
+        # It is a problem on Windows, that does not accept to clean resources before closing them.
+        logging.shutdown()
+        # Remove logging handlers that have been closed so they won't be
+        # accidentally used in future tests.
+        logging.getLogger().handlers = []
+        util._release_locks()  # pylint: disable=protected-access
+
+        def handle_rw_files(_, path, __):
+            """Handle read-only files, that will fail to be removed on Windows."""
+            os.chmod(path, stat.S_IWRITE)
+            os.remove(path)
+        shutil.rmtree(self.tempdir, onerror=handle_rw_files)
 
 
 class ConfigTestCase(TempDirTestCase):
-    """Test class which sets up a NamespaceConfig object.
-
-    """
+    """Test class which sets up a NamespaceConfig object."""
     def setUp(self):
         super(ConfigTestCase, self).setUp()
         self.config = configuration.NamespaceConfig(

certbot/tests/util_test.py

@@ -193,7 +193,12 @@ class CheckPermissionsTest(test_util.TempDirTestCase):
 
     def test_wrong_mode(self):
         os.chmod(self.tempdir, 0o400)
-        self.assertFalse(self._call(0o600))
+        try:
+            self.assertFalse(self._call(0o600))
+        finally:
+            # Without proper write permissions, Windows is unable to delete a folder,
+            # even with admin permissions. Write access must be explicitly set first.
+            os.chmod(self.tempdir, 0o700)
 
 
 class UniqueFileTest(test_util.TempDirTestCase):
@@ -279,20 +284,9 @@ class UniqueLineageNameTest(test_util.TempDirTestCase):
         for f, _ in items:
             f.close()
 
-    @mock.patch("certbot.util.os.fdopen")
-    def test_failure(self, mock_fdopen):
-        err = OSError("whoops")
-        err.errno = errno.EIO
-        mock_fdopen.side_effect = err
-        self.assertRaises(OSError, self._call, "wow")
-
-    @mock.patch("certbot.util.os.fdopen")
-    def test_subsequent_failure(self, mock_fdopen):
-        self._call("wow")
-        err = OSError("whoops")
-        err.errno = errno.EIO
-        mock_fdopen.side_effect = err
-        self.assertRaises(OSError, self._call, "wow")
+    def test_failure(self):
+        with mock.patch("certbot.util.os.open", side_effect=OSError(errno.EIO)):
+            self.assertRaises(OSError, self._call, "wow")
 
 
 class SafelyRemoveTest(test_util.TempDirTestCase):

certbot/util.py

@@ -142,6 +142,7 @@ def _release_locks():
         except:  # pylint: disable=bare-except
             msg = 'Exception occurred releasing lock: {0!r}'.format(dir_lock)
             logger.debug(msg, exc_info=True)
+    _LOCKS.clear()
 
 
 def set_up_core_dir(directory, mode, uid, strict):
@@ -225,9 +226,8 @@ def safe_open(path, mode="w", chmod=None, buffering=None):
     fdopen_args = ()  # type: Union[Tuple[()], Tuple[int]]
     if buffering is not None:
         fdopen_args = (buffering,)
-    return os.fdopen(
-        os.open(path, os.O_CREAT | os.O_EXCL | os.O_RDWR, *open_args),
-        mode, *fdopen_args)
+    fd = os.open(path, os.O_CREAT | os.O_EXCL | os.O_RDWR, *open_args)
+    return os.fdopen(fd, mode, *fdopen_args)
 
 
 def _unique_file(path, filename_pat, count, chmod, mode):

setup.py

@@ -1,8 +1,10 @@
 import codecs
 import os
 import re
+import sys
 
 from setuptools import find_packages, setup
+from setuptools.command.test import test as TestCommand
 
 # Workaround for http://bugs.python.org/issue8876, see
 # http://bugs.python.org/issue8876#msg208792
@@ -77,6 +79,22 @@ docs_extras = [
     'sphinx_rtd_theme',
 ]
 
+
+class PyTest(TestCommand):
+    user_options = []
+
+    def initialize_options(self):
+        TestCommand.initialize_options(self)
+        self.pytest_args = ''
+
+    def run_tests(self):
+        import shlex
+        # import here, cause outside the eggs aren't loaded
+        import pytest
+        errno = pytest.main(shlex.split(self.pytest_args))
+        sys.exit(errno)
+
+
 setup(
     name='certbot',
     version=version,
@@ -123,6 +141,8 @@ setup(
     # to test all packages run "python setup.py test -s
     # {acme,certbot_apache,certbot_nginx}"
     test_suite='certbot',
+    tests_require=["pytest"],
+    cmdclass={"test": PyTest},
 
     entry_points={
         'console_scripts': [