Skip to content

Commit

Permalink
Keep temp files out of project dir and improve cleanup
Browse files Browse the repository at this point in the history
This fixes a test bug in TestTree.test_tree_modifier_ordering
where:

- A `tmp` subdirectory of the directory the tests are run from
  (almost always the root of the GitPython source tree) was
  created, instead of creating it in /tmp or elsewhere configured.

- That directory was never deleted afterward, creating new
  untracked files in GitPython's own working tree, and also making
  it so running the tests twice would always fail the second time,
  unless a manual intervening deletion was performed. (This had not
  broken CI, since CI checks clone the repository anew each time.)

- The directory was changed into to set up the test, but not
  deterministically changed back out of. It would typically be
  exited, but this was not guaranteed to occur if some subprocess
  commands were to fail unexpectedly.

In addition to fixing all three aspects of that bug, this also:

- Remains in the temporary directory only as long as necessary to
  execute the subprocesses in it that produce the expected value
  for the test (including not entering it until running them).

- Deletes the temporary directory immediately after exiting it,
  since it is only used to produce a file list to compare to.

- Avoids interleaving file creation and git subprocess commands,
  since doing so made it harder to understand what was being set up
  and since the difference is not relevant to the test.

- Does the work in that temporary directory before performing any
  operations with the code under test, because it is conceptually
  an "arrange" step, and because doing so makes its limited purpose
  clearer on reading the tests.

- Some other refactoring to support the fixes and accompanying
  changes, including extracting the temporary directory logic to a
  helper method.

This deliberately does not change the order in which any files are
created. It also keeps the approach of using subprocess functions
directly to operate on the temporary git repository (and changing
directory while doing so, keeping each of the commands the same),
since there might be good reasons to do this, such as to make very
clear that the file order being obtained to compare to is really
coming from git itself. Even if this is to be changed, it seems
outside the scope of this bugfix.

The test still fails if the fix to git/objects/tree.py from 365d44f
(#1799) is absent. This was verified by running:

    git revert --no-commit 365d44f
    pytest --no-cov -vv test/test_tree.py
    git revert --abort
  • Loading branch information
EliahKagan committed Feb 15, 2024
1 parent 7ba3fd2 commit dd42e38
Showing 1 changed file with 32 additions and 22 deletions.
54 changes: 32 additions & 22 deletions test/test_tree.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,15 @@
# 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/

from io import BytesIO
import os.path as osp
from pathlib import Path
import subprocess
import tempfile

from git.objects import Tree, Blob
from git.util import cwd
from test.lib import TestBase

import os
import os.path as osp
import subprocess


class TestTree(TestBase):
def test_serializable(self):
Expand Down Expand Up @@ -42,29 +43,41 @@ def test_serializable(self):
testtree._deserialize(stream)
# END for each item in tree

def test_tree_modifier_ordering(self):
def setup_git_repository_and_get_ordered_files():
os.mkdir("tmp")
os.chdir("tmp")
subprocess.run(["git", "init", "-q"], check=True)
os.mkdir("file")
for filename in [
@staticmethod
def _get_git_ordered_files():
"""Get files as git orders them, to compare in test_tree_modifier_ordering."""
with tempfile.TemporaryDirectory() as tdir:
# Create directory contents.
Path(tdir, "file").mkdir()
for filename in (
"bin",
"bin.d",
"file.to",
"file.toml",
"file.toml.bin",
"file0",
"file/a",
]:
open(filename, "a").close()

subprocess.run(["git", "add", "."], check=True)
subprocess.run(["git", "commit", "-m", "c1"], check=True)
tree_hash = subprocess.check_output(["git", "rev-parse", "HEAD^{tree}"]).decode().strip()
cat_file_output = subprocess.check_output(["git", "cat-file", "-p", tree_hash]).decode()
):
Path(tdir, filename).touch()
Path(tdir, "file", "a").touch()

with cwd(tdir):
# Prepare the repository.
subprocess.run(["git", "init", "-q"], check=True)
subprocess.run(["git", "add", "."], check=True)
subprocess.run(["git", "commit", "-m", "c1"], check=True)

# Get git output from which an ordered file list can be parsed.
rev_parse_command = ["git", "rev-parse", "HEAD^{tree}"]
tree_hash = subprocess.check_output(rev_parse_command).decode().strip()
cat_file_command = ["git", "cat-file", "-p", tree_hash]
cat_file_output = subprocess.check_output(cat_file_command).decode()

return [line.split()[-1] for line in cat_file_output.split("\n") if line]

def test_tree_modifier_ordering(self):
"""TreeModifier.set_done() sorts files in the same order git does."""
git_file_names_in_order = self._get_git_ordered_files()

hexsha = "6c1faef799095f3990e9970bc2cb10aa0221cf9c"
roottree = self.rorepo.tree(hexsha)
blob_mode = Tree.blob_id << 12
Expand Down Expand Up @@ -92,9 +105,6 @@ def names_in_mod_cache():
here = file_names_in_order()
return [e for e in a if e in here]

git_file_names_in_order = setup_git_repository_and_get_ordered_files()
os.chdir("..")

mod.set_done()
assert names_in_mod_cache() == git_file_names_in_order, "set_done() performs git-sorting"

Expand Down

0 comments on commit dd42e38

Please sign in to comment.