Always cleanup the changed environ

tests/integration/modules/test_pip.py

@@ -32,9 +32,9 @@ class PipModuleTest(ModuleCase):
         super(PipModuleTest, self).setUp()
 
         # Restore the environ
-        def cleanup_environ(envcopy):
+        def cleanup_environ(environ):
             os.environ.clear()
-            os.environ.update(envcopy)
+            os.environ.update(environ)
 
         self.addCleanup(cleanup_environ, os.environ.copy())
 

tests/integration/states/test_pip_state.py

@@ -123,37 +123,34 @@ class PipStateTest(ModuleCase, SaltReturnAssertsMixin):
         venv_dir = os.path.join(
             RUNTIME_VARS.TMP, 'pip-installed-errors'
         )
-        orig_shell = os.environ.get('SHELL')
-        try:
-            # Since we don't have the virtualenv created, pip.installed will
-            # throw an error.
-            # Example error strings:
-            #  * "Error installing 'pep8': /tmp/pip-installed-errors: not found"
-            #  * "Error installing 'pep8': /bin/sh: 1: /tmp/pip-installed-errors: not found"
-            #  * "Error installing 'pep8': /bin/bash: /tmp/pip-installed-errors: No such file or directory"
-            os.environ['SHELL'] = '/bin/sh'
-            ret = self.run_function('state.sls', mods='pip-installed-errors')
-            self.assertSaltFalseReturn(ret)
-            self.assertSaltCommentRegexpMatches(
-                ret,
-                'Error installing \'pep8\':'
-            )
 
-            # We now create the missing virtualenv
+        def cleanup_environ(environ):
-            ret = self._create_virtualenv(venv_dir)
+            os.environ.clear()
-            self.assertEqual(ret['retcode'], 0)
+            os.environ.update(environ)
 
-            # The state should not have any issues running now
+        self.addCleanup(cleanup_environ, os.environ.copy())
-            ret = self.run_function('state.sls', mods='pip-installed-errors')
+
-            self.assertSaltTrueReturn(ret)
+        # Since we don't have the virtualenv created, pip.installed will
-        finally:
+        # throw an error.
-            if orig_shell is None:
+        # Example error strings:
-                # Didn't exist before, don't leave it there. This should never
+        #  * "Error installing 'pep8': /tmp/pip-installed-errors: not found"
-                # happen, but if it does, we don't want this test to affect
+        #  * "Error installing 'pep8': /bin/sh: 1: /tmp/pip-installed-errors: not found"
-                # others elsewhere in the suite.
+        #  * "Error installing 'pep8': /bin/bash: /tmp/pip-installed-errors: No such file or directory"
-                os.environ.pop('SHELL')
+        os.environ['SHELL'] = '/bin/sh'
-            else:
+        ret = self.run_function('state.sls', mods='pip-installed-errors')
-                os.environ['SHELL'] = orig_shell
+        self.assertSaltFalseReturn(ret)
+        self.assertSaltCommentRegexpMatches(
+            ret,
+            'Error installing \'pep8\':'
+        )
+
+        # We now create the missing virtualenv
+        ret = self._create_virtualenv(venv_dir)
+        self.assertEqual(ret['retcode'], 0)
+
+        # The state should not have any issues running now
+        ret = self.run_function('state.sls', mods='pip-installed-errors')
+        self.assertSaltTrueReturn(ret)
 
     @skipIf(six.PY3, 'Issue is specific to carbon module, which is PY2-only')
     @skipIf(salt.utils.platform.is_windows(), "Carbon does not install in Windows")

tests/support/gitfs.py

@@ -70,7 +70,6 @@ _OPTS = {
     'git_pillar_includes': True,
 }
 PROC_TIMEOUT = 10
-NOTSET = object()
 
 
 class ProcessManager(object):
@@ -634,14 +633,15 @@ class GitPillarSSHTestBase(GitPillarTestBase, SSHDMixin):
         passphraselsess key is used to auth without needing to modify the root
         user's ssh config file.
         '''
-        orig_git_ssh = os.environ.pop('GIT_SSH', NOTSET)
+
+        def cleanup_environ(environ):
+            os.environ.clear()
+            os.environ.update(environ)
+
+        self.addCleanup(cleanup_environ, os.environ.copy())
+
         os.environ['GIT_SSH'] = self.git_ssh
-        try:
+        return super(GitPillarSSHTestBase, self).get_pillar(ext_pillar_conf)
-            return super(GitPillarSSHTestBase, self).get_pillar(ext_pillar_conf)
-        finally:
-            os.environ.pop('GIT_SSH', None)
-            if orig_git_ssh is not NOTSET:
-                os.environ['GIT_SSH'] = orig_git_ssh
 
 
 class GitPillarHTTPTestBase(GitPillarTestBase, WebserverMixin):

tests/unit/fileserver/test_gitfs.py

@@ -405,39 +405,40 @@ class GitFSTestBase(object):
 
         username_key = str('USERNAME')
         orig_username = os.environ.get(username_key)
-        try:
+        if username_key not in os.environ:
-            if username_key not in os.environ:
-                try:
-                    if salt.utils.platform.is_windows():
-                        os.environ[username_key] = \
-                            salt.utils.win_functions.get_current_user()
-                    else:
-                        os.environ[username_key] = \
-                            pwd.getpwuid(os.geteuid()).pw_name
-                except AttributeError:
-                    log.error(
-                        'Unable to get effective username, falling back to '
-                        '\'root\'.'
-                    )
-                    os.environ[username_key] = str('root')
 
-            repo.index.add([x for x in os.listdir(TMP_REPO_DIR)
+            def cleanup_environ(environ):
-                            if x != '.git'])
+                os.environ.clear()
-            repo.index.commit('Test')
+                os.environ.update(environ)
 
-            # Add another branch with unicode characters in the name
+            self.addCleanup(cleanup_environ, os.environ.copy())
-            repo.create_head(UNICODE_ENVNAME, 'HEAD')
 
-            # Add a tag
+            try:
-            repo.create_tag(TAG_NAME, 'HEAD')
+                if salt.utils.platform.is_windows():
-            # Older GitPython versions do not have a close method.
+                    os.environ[username_key] = \
-            if hasattr(repo, 'close'):
+                        salt.utils.win_functions.get_current_user()
-                repo.close()
+                else:
-        finally:
+                    os.environ[username_key] = \
-            if orig_username is not None:
+                        pwd.getpwuid(os.geteuid()).pw_name
-                os.environ[username_key] = orig_username
+            except AttributeError:
-            else:
+                log.error(
-                os.environ.pop(username_key, None)
+                    'Unable to get effective username, falling back to '
+                    '\'root\'.'
+                )
+                os.environ[username_key] = str('root')
+
+        repo.index.add([x for x in os.listdir(TMP_REPO_DIR)
+                        if x != '.git'])
+        repo.index.commit('Test')
+
+        # Add another branch with unicode characters in the name
+        repo.create_head(UNICODE_ENVNAME, 'HEAD')
+
+        # Add a tag
+        repo.create_tag(TAG_NAME, 'HEAD')
+        # Older GitPython versions do not have a close method.
+        if hasattr(repo, 'close'):
+            repo.close()
 
     @classmethod
     def tearDownClass(cls):