Jelajahi Sumber

Merge fix from kankri to reset environment variables during tests, to avoid
test isolation leaks reading ~/.gitconfig.

Jelmer Vernooij 13 tahun lalu
induk
melakukan
6b1e410377

+ 3 - 0
NEWS

@@ -11,6 +11,9 @@
   * Fix parsing of invalid timezone offsets with two minus signs.
     (Jason R. Coombs, #697828)
 
+  * Reset environment variables during tests, to avoid
+    test isolation leaks reading ~/.gitconfig. (Risto Kankkunen)
+
  TESTS
 
   * $HOME is now explicitly specified for tests that use it to read

+ 39 - 2
dulwich/tests/__init__.py

@@ -30,10 +30,47 @@ import tempfile
 if sys.version_info >= (2, 7):
     # If Python itself provides an exception, use that
     import unittest
-    from unittest import SkipTest, TestCase
+    from unittest import SkipTest, TestCase as _TestCase
 else:
     import unittest2 as unittest
-    from unittest2 import SkipTest, TestCase
+    from unittest2 import SkipTest, TestCase as _TestCase
+
+
+def get_safe_env(env=None):
+    """Returns the environment "env" (or a copy of "os.environ" by default)
+    modified to avoid side-effects caused by user's ~/.gitconfig"""
+
+    if env is None:
+        env = os.environ.copy()
+    # On Windows it's not enough to set "HOME" to a non-existing
+    # directory. Git.cmd takes the first existing directory out of
+    # "%HOME%", "%HOMEDRIVE%%HOMEPATH%" and "%USERPROFILE%".
+    for e in 'HOME', 'HOMEPATH', 'USERPROFILE':
+        env[e] = '/nosuchdir'
+    return env
+
+
+class TestCase(_TestCase):
+
+    def makeSafeEnv(self):
+        """Create environment with homedirectory-related variables stripped.
+
+        Modifies os.environ for the duration of a test case to avoid
+        side-effects caused by the user's ~/.gitconfig and other
+        files in their home directory.
+        """
+        old_env = os.environ
+        def restore():
+            os.environ = old_env
+        self.addCleanup(restore)
+        new_env = dict(os.environ)
+        for e in ['HOME', 'HOMEPATH', 'USERPROFILE']:
+            new_env[e] = '/nosuchdir'
+        os.environ = new_env
+
+    def setUp(self):
+        super(TestCase, self).setUp()
+        self.makeSafeEnv()
 
 
 class BlackboxTestCase(TestCase):

+ 2 - 1
dulwich/tests/compat/test_client.py

@@ -43,6 +43,7 @@ from dulwich import (
     repo,
     )
 from dulwich.tests import (
+    get_safe_env,
     SkipTest,
     )
 
@@ -255,7 +256,7 @@ class TestSSHVendor(object):
     def connect_ssh(host, command, username=None, port=None):
         cmd, path = command[0].replace("'", '').split(' ')
         cmd = cmd.split('-', 1)
-        p = subprocess.Popen(cmd + [path], stdin=subprocess.PIPE,
+        p = subprocess.Popen(cmd + [path], env=get_safe_env(), stdin=subprocess.PIPE,
                              stdout=subprocess.PIPE, stderr=subprocess.PIPE)
         return client.SubprocessWrapper(p)
 

+ 5 - 1
dulwich/tests/compat/utils.py

@@ -30,6 +30,7 @@ from dulwich.repo import Repo
 from dulwich.protocol import TCP_GIT_PORT
 
 from dulwich.tests import (
+    get_safe_env,
     SkipTest,
     TestCase,
     )
@@ -117,13 +118,16 @@ def run_git(args, git_path=_DEFAULT_GIT, input=None, capture_stdout=False,
         False, None will be returned as stdout contents.
     :raise OSError: if the git executable was not found.
     """
+
+    env = get_safe_env(popen_kwargs.pop('env', None))
+
     args = [git_path] + args
     popen_kwargs['stdin'] = subprocess.PIPE
     if capture_stdout:
         popen_kwargs['stdout'] = subprocess.PIPE
     else:
         popen_kwargs.pop('stdout', None)
-    p = subprocess.Popen(args, **popen_kwargs)
+    p = subprocess.Popen(args, env=env, **popen_kwargs)
     stdout, stderr = p.communicate(input=input)
     return (p.returncode, stdout)
 

+ 0 - 3
dulwich/tests/test_config.py

@@ -31,7 +31,6 @@ from dulwich.config import (
     _unescape_value,
     )
 from dulwich.tests import TestCase
-import os
 
 
 class ConfigFileTests(TestCase):
@@ -175,8 +174,6 @@ class ConfigDictTests(TestCase):
 class StackedConfigTests(TestCase):
 
     def test_default_backends(self):
-        self.addCleanup(os.environ.__setitem__, "HOME", os.environ["HOME"])
-        os.environ["HOME"] = "/nonexistant"
         StackedConfig.default_backends()
 
 

+ 0 - 4
dulwich/tests/test_repository.py

@@ -320,8 +320,6 @@ class RepositoryTests(TestCase):
         self.assertIsInstance(r.get_config(), Config)
 
     def test_get_config_stack(self):
-        self.addCleanup(os.environ.__setitem__, "HOME", os.environ["HOME"])
-        os.environ["HOME"] = "/nonexistant"
         r = self._repo = open_repo('ooo_merge.git')
         self.assertIsInstance(r.get_config_stack(), Config)
 
@@ -460,8 +458,6 @@ class BuildRepoTests(TestCase):
         self.assertEqual("iso8859-1", r[commit_sha].encoding)
 
     def test_commit_config_identity(self):
-        self.addCleanup(os.environ.__setitem__, "HOME", os.environ["HOME"])
-        os.environ["HOME"] = "/nonexistant"
         # commit falls back to the users' identity if it wasn't specified
         r = self._repo
         c = r.get_config()