Revert "Call atexit handlers before test tearDown to remove errors on Windows (#6667)"

This reverts commit ca25d1b66a.

certbot/tests/lock_test.py

@@ -10,6 +10,7 @@ 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
@@ -24,6 +25,7 @@ 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
@@ -35,7 +37,6 @@ 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,
@@ -53,7 +54,6 @@ 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,7 +71,6 @@ 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
@@ -87,13 +86,11 @@ 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'

certbot/tests/util.py

@@ -3,15 +3,14 @@
 .. 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
@@ -329,22 +328,22 @@ class TempDirTestCase(unittest.TestCase):
 
     def tearDown(self):
         """Execute after test"""
-        # 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)
-
+        # 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)
 
 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

@@ -191,12 +191,7 @@ class CheckPermissionsTest(test_util.TempDirTestCase):
 
     def test_wrong_mode(self):
         os.chmod(self.tempdir, 0o400)
-        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)
+        self.assertFalse(self._call(0o600))
 
 
 class UniqueFileTest(test_util.TempDirTestCase):
@@ -282,9 +277,20 @@ class UniqueLineageNameTest(test_util.TempDirTestCase):
         for f, _ in items:
             f.close()
 
-    def test_failure(self):
-        with mock.patch("certbot.util.os.open", side_effect=OSError(errno.EIO)):
-            self.assertRaises(OSError, self._call, "wow")
+    @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")
 
 
 class SafelyRemoveTest(test_util.TempDirTestCase):

certbot/util.py

@@ -142,7 +142,6 @@ 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):
@@ -226,8 +225,9 @@ 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,)
-    fd = os.open(path, os.O_CREAT | os.O_EXCL | os.O_RDWR, *open_args)
-    return os.fdopen(fd, mode, *fdopen_args)
+    return os.fdopen(
+        os.open(path, os.O_CREAT | os.O_EXCL | os.O_RDWR, *open_args),
+        mode, *fdopen_args)
 
 
 def _unique_file(path, filename_pat, count, chmod, mode):