diff --git a/certbot/tests/lock_test.py b/certbot/tests/lock_test.py index aa82701f3..2ade47827 100644 --- a/certbot/tests/lock_test.py +++ b/certbot/tests/lock_test.py @@ -10,7 +10,6 @@ from certbot import errors from certbot.tests import util as test_util -@test_util.broken_on_windows class LockDirTest(test_util.TempDirTestCase): """Tests for certbot.lock.lock_dir.""" @classmethod @@ -25,7 +24,6 @@ class LockDirTest(test_util.TempDirTestCase): test_util.lock_and_call(assert_raises, lock_path) -@test_util.broken_on_windows class LockFileTest(test_util.TempDirTestCase): """Tests for certbot.lock.LockFile.""" @classmethod @@ -37,6 +35,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, @@ -54,6 +53,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) @@ -71,6 +71,7 @@ class LockFileTest(test_util.TempDirTestCase): self.assertTrue(lock_file.__class__.__name__ in lock_repr) self.assertTrue(self.lock_path in lock_repr) + @test_util.broken_on_windows def test_race(self): should_delete = [True, False] stat = os.stat @@ -86,11 +87,13 @@ 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() self.assertFalse(os.path.exists(self.lock_path)) + @test_util.broken_on_windows @mock.patch('certbot.compat.fcntl.lockf') def test_unexpected_lockf_err(self, mock_lockf): msg = 'hi there' diff --git a/certbot/tests/util.py b/certbot/tests/util.py index 8c5db2c2f..38a9075c1 100644 --- a/certbot/tests/util.py +++ b/certbot/tests/util.py @@ -3,14 +3,15 @@ .. warning:: This module is not part of the public API. """ +import logging import multiprocessing import os import pkg_resources import shutil +import stat import tempfile import unittest import sys -import warnings from cryptography.hazmat.backends import default_backend from cryptography.hazmat.primitives import serialization @@ -328,22 +329,22 @@ 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() + 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( diff --git a/certbot/tests/util_test.py b/certbot/tests/util_test.py index 6685b88c6..81d0629c8 100644 --- a/certbot/tests/util_test.py +++ b/certbot/tests/util_test.py @@ -191,7 +191,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): @@ -277,20 +282,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): diff --git a/certbot/util.py b/certbot/util.py index d7c542465..416075ce8 100644 --- a/certbot/util.py +++ b/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):