Fix Mock 4.0.3 compatibility - Unpatch only once during teardown (#3541)

* #3535 - Unpatch only once during teardown

* EnvVar patching - Fix Python2 bug

* Allow latest version of mock-library

moto/core/models.py

@@ -35,6 +35,7 @@ RESPONSES_VERSION = pkg_resources.get_distribution("responses").version
 
 class BaseMockAWS(object):
     nested_count = 0
+    mocks_active = False
 
     def __init__(self, backends):
         from moto.instance_metadata import instance_metadata_backend
@@ -50,13 +51,12 @@ class BaseMockAWS(object):
         self.backends_for_urls.update(self.backends)
         self.backends_for_urls.update(default_backends)
 
-        # "Mock" the AWS credentials as they can't be mocked in Botocore currently
+        self.FAKE_KEYS = {
-        FAKE_KEYS = {
             "AWS_ACCESS_KEY_ID": "foobar_key",
             "AWS_SECRET_ACCESS_KEY": "foobar_secret",
         }
+        self.ORIG_KEYS = {}
         self.default_session_mock = mock.patch("boto3.DEFAULT_SESSION", None)
-        self.env_variables_mocks = mock.patch.dict(os.environ, FAKE_KEYS)
 
         if self.__class__.nested_count == 0:
             self.reset()
@@ -74,8 +74,10 @@ class BaseMockAWS(object):
         self.stop()
 
     def start(self, reset=True):
+        if not self.__class__.mocks_active:
             self.default_session_mock.start()
-        self.env_variables_mocks.start()
+            self.mock_env_variables()
+            self.__class__.mocks_active = True
 
         self.__class__.nested_count += 1
         if reset:
@@ -85,14 +87,16 @@ class BaseMockAWS(object):
         self.enable_patching()
 
     def stop(self):
-        self.default_session_mock.stop()
-        self.env_variables_mocks.stop()
         self.__class__.nested_count -= 1
 
         if self.__class__.nested_count < 0:
             raise RuntimeError("Called stop() before start().")
 
         if self.__class__.nested_count == 0:
+            if self.__class__.mocks_active:
+                self.default_session_mock.stop()
+                self.unmock_env_variables()
+                self.__class__.mocks_active = False
             self.disable_patching()
 
     def decorate_callable(self, func, reset):
@@ -139,6 +143,24 @@ class BaseMockAWS(object):
                 continue
         return klass
 
+    def mock_env_variables(self):
+        # "Mock" the AWS credentials as they can't be mocked in Botocore currently
+        # self.env_variables_mocks = mock.patch.dict(os.environ, FAKE_KEYS)
+        # self.env_variables_mocks.start()
+        for k, v in self.FAKE_KEYS.items():
+            self.ORIG_KEYS[k] = os.environ.get(k, None)
+            os.environ[k] = v
+
+    def unmock_env_variables(self):
+        # This doesn't work in Python2 - for some reason, unmocking clears the entire os.environ dict
+        # Obviously bad user experience, and also breaks pytest - as it uses PYTEST_CURRENT_TEST as an env var
+        # self.env_variables_mocks.stop()
+        for k, v in self.ORIG_KEYS.items():
+            if v:
+                os.environ[k] = v
+            else:
+                del os.environ[k]
+
 
 class HttprettyMockAWS(BaseMockAWS):
     def reset(self):

requirements-dev.txt

@@ -11,3 +11,4 @@ click==6.7
 inflection==0.3.1
 lxml==4.2.3
 beautifulsoup4==4.6.0
+

setup.py

@@ -60,7 +60,7 @@ install_requires += [
     "configparser<5.0; python_version < '3'",
     "Jinja2>=2.10.1",
     "Jinja2<3.0.0; python_version < '3'",
-    "mock<=4.0.2",
+    "mock",
     "mock<=3.0.5; python_version < '3'",
     "more-itertools",
     "more-itertools==5.0.0; python_version < '3'",

tests/test_core/test_environ_patching.py

@@ -0,0 +1,34 @@
+import os
+import sure  # noqa
+from moto import mock_ec2, mock_s3
+
+KEY = "AWS_ACCESS_KEY_ID"
+
+
+def test_aws_keys_are_patched():
+    with mock_ec2():
+        patched_value = os.environ[KEY]
+        patched_value.should.equal("foobar_key")
+
+
+def test_aws_keys_can_be_none():
+    """
+    Verify that the os.environ[KEY] can be None
+    Patching the None-value shouldn't be an issue
+    """
+    original = os.environ.get(KEY, "value-set-by-user")
+    # Delete the original value by the user
+    try:
+        del os.environ[KEY]
+    except KeyError:
+        pass  # Value might not be set on this system in the first place
+    try:
+        # Verify that the os.environ[KEY] is patched
+        with mock_s3():
+            patched_value = os.environ[KEY]
+            patched_value.should.equal("foobar_key")
+        # Verify that the os.environ[KEY] is unpatched, and reverts to None
+        assert os.environ.get(KEY) is None
+    finally:
+        # Reset the value original - don't want to change the users system
+        os.environ[KEY] = original

tests/test_s3/test_s3_classdecorator.py

@@ -0,0 +1,18 @@
+import unittest
+
+import boto3
+from moto import mock_s3
+
+
+@mock_s3
+class ClassDecoratorTest(unittest.TestCase):
+    """
+    https://github.com/spulec/moto/issues/3535
+    An update to the mock-package introduced a failure during teardown.
+    This test is in place to catch any similar failures with our mocking approach
+    """
+
+    def test_instantiation_succeeds(self):
+        s3 = boto3.client("s3", region_name="us-east-1")
+
+        assert s3 is not None