From ad570de497a4512a336eed141726af5b7b7e126b Mon Sep 17 00:00:00 2001
From: Travis Runner <degeneracypressure@gmail.com>
Date: Thu, 7 Dec 2023 17:10:17 -0500
Subject: [PATCH 1/3] Avoid unsafe assumptions about tempdir content in tests
test_new_should_raise_on_invalid_repo_location had previously used
tempfile.gettempdir() directly to get a path assumed not to be a
valid repository, assuming more than necessary about the directory
in which temporary files and directories are created. A newly
created temporary directory is now used for that check, instead.
test_new_should_raise_on_non_existent_path had assumed no
repos/foobar existed relative to the current directory. Typically
there would be no such directory, but is unnecessary for the test
to rely on this. A newly created temporary directory known to be
empty is now used in place of the "repos" component, for that test.
---
test/test_repo.py | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/test/test_repo.py b/test/test_repo.py
index 033deca1e..c68dd074c 100644
--- a/test/test_repo.py
+++ b/test/test_repo.py
@@ -71,15 +71,19 @@ def tearDown(self):
for lfp in glob.glob(_tc_lock_fpaths):
if osp.isfile(lfp):
raise AssertionError("Previous TC left hanging git-lock file: {}".format(lfp))
+
import gc
gc.collect()
def test_new_should_raise_on_invalid_repo_location(self):
- self.assertRaises(InvalidGitRepositoryError, Repo, tempfile.gettempdir())
+ with tempfile.TemporaryDirectory() as tdir:
+ self.assertRaises(InvalidGitRepositoryError, Repo, tdir)
def test_new_should_raise_on_non_existent_path(self):
- self.assertRaises(NoSuchPathError, Repo, "repos/foobar")
+ with tempfile.TemporaryDirectory() as tdir:
+ nonexistent = osp.join(tdir, "foobar")
+ self.assertRaises(NoSuchPathError, Repo, nonexistent)
@with_rw_repo("0.3.2.1")
def test_repo_creation_from_different_paths(self, rw_repo):
From 9277ff561bcc37c70b9e6e015f623af20216b9e0 Mon Sep 17 00:00:00 2001
From: Travis Runner <degeneracypressure@gmail.com>
Date: Thu, 7 Dec 2023 19:06:05 -0500
Subject: [PATCH 2/3] Avoid another tempdir content assumption in test
test_init was using tempfile.gettempdir() directly to get the
location where the hard-coded path repos/foo/bar.git would be used
to test repository creation with relative and absolute paths. That
reused the same location each time, and also assumed the directory
would be usable, which could fail due to previous runs or due to
the path being used separately from GitPython's tests. This commit
fixes that by using that path inside a temporary directory, known
at the start of the test to be empty.
Reorganizing the acquision and cleanup logic also has had the
effect of causing the test no longer to be skipped due to the logic
in git.util.rmtree due to the final cleanup attempt (after all
assertions). The directory is now successfully removed on Windows,
and the test passes on all platforms.
---
test/test_repo.py | 23 +++++++----------------
1 file changed, 7 insertions(+), 16 deletions(-)
diff --git a/test/test_repo.py b/test/test_repo.py
index c68dd074c..845fabf15 100644
--- a/test/test_repo.py
+++ b/test/test_repo.py
@@ -41,7 +41,7 @@
UnsafeProtocolError,
)
from git.repo.fun import touch
-from git.util import bin_to_hex, cygpath, join_path_native, rmfile, rmtree
+from git.util import bin_to_hex, cwd, cygpath, join_path_native, rmfile, rmtree
from test.lib import TestBase, fixture, with_rw_directory, with_rw_repo
@@ -511,13 +511,11 @@ def write(self, b):
repo.git.log(n=100, output_stream=TestOutputStream(io.DEFAULT_BUFFER_SIZE))
def test_init(self):
- prev_cwd = os.getcwd()
- os.chdir(tempfile.gettempdir())
- git_dir_rela = "repos/foo/bar.git"
- del_dir_abs = osp.abspath("repos")
- git_dir_abs = osp.abspath(git_dir_rela)
- try:
- # with specific path
+ with tempfile.TemporaryDirectory() as tdir, cwd(tdir):
+ git_dir_rela = "repos/foo/bar.git"
+ git_dir_abs = osp.abspath(git_dir_rela)
+
+ # With specific path
for path in (git_dir_rela, git_dir_abs):
r = Repo.init(path=path, bare=True)
self.assertIsInstance(r, Repo)
@@ -527,7 +525,7 @@ def test_init(self):
self._assert_empty_repo(r)
- # test clone
+ # Test clone
clone_path = path + "_clone"
rc = r.clone(clone_path)
self._assert_empty_repo(rc)
@@ -562,13 +560,6 @@ def test_init(self):
assert not r.has_separate_working_tree()
self._assert_empty_repo(r)
- finally:
- try:
- rmtree(del_dir_abs)
- except OSError:
- pass
- os.chdir(prev_cwd)
- # END restore previous state
def test_bare_property(self):
self.rorepo.bare
From c09ac1ab2c86a87080dcfb2fc0af64cbcc2bc9eb Mon Sep 17 00:00:00 2001
From: Travis Runner <degeneracypressure@gmail.com>
Date: Fri, 8 Dec 2023 01:18:54 -0500
Subject: [PATCH 3/3] Test InvalidGitRepositoryError in repo subdir
A Repo object can of course be constructed from a path to a
directory that is the root of an existing repository, and raises
InvalidGitRepositoryError on a directory that is outside of any
repository. Tests intended to show both conditions already exist.
This adds a test to verify that InvalidGitRepositoryError is also
raised when an attempt is made to construct a Repo object from a
path to a directory that is not the root of a repository but that
is known to be located inside an existing git repository.
---
test/test_repo.py | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/test/test_repo.py b/test/test_repo.py
index 845fabf15..836862b0c 100644
--- a/test/test_repo.py
+++ b/test/test_repo.py
@@ -77,9 +77,20 @@ def tearDown(self):
gc.collect()
def test_new_should_raise_on_invalid_repo_location(self):
+ # Ideally this tests a directory that is outside of any repository. In the rare
+ # case tempfile.gettempdir() is inside a repo, this still passes, but tests the
+ # same scenario as test_new_should_raise_on_invalid_repo_location_within_repo.
with tempfile.TemporaryDirectory() as tdir:
self.assertRaises(InvalidGitRepositoryError, Repo, tdir)
+ @with_rw_directory
+ def test_new_should_raise_on_invalid_repo_location_within_repo(self, rw_dir):
+ repo_dir = osp.join(rw_dir, "repo")
+ Repo.init(repo_dir)
+ subdir = osp.join(repo_dir, "subdir")
+ os.mkdir(subdir)
+ self.assertRaises(InvalidGitRepositoryError, Repo, subdir)
+
def test_new_should_raise_on_non_existent_path(self):
with tempfile.TemporaryDirectory() as tdir:
nonexistent = osp.join(tdir, "foobar")
@@ -122,7 +133,7 @@ def test_tree_from_revision(self):
self.assertEqual(tree.type, "tree")
self.assertEqual(self.rorepo.tree(tree), tree)
- # try from invalid revision that does not exist
+ # Try from an invalid revision that does not exist.
self.assertRaises(BadName, self.rorepo.tree, "hello world")
def test_pickleable(self):