Document dvc.scm.git::Git::_get_diff_trees and shorten commit hashes for dvc diff (#1906)

* Document dvc.scm.git::Git::_get_diff_trees and shorten commit hashes in dvc.scm.git::Git::get_diff_trees

* diff: Changes to dvc.scm.git so `dvc diff` outputs short SHAs

- Docstring for Git class constructor
- Renames self.git for self.repo throughout module (involves several test changes)
- Better doc and logical refactor in Git._get_diff_trees
- `dvc diff` now outputs short SHA strings (Git._get_diff_trees) - hard coded to 7 length str
  (involves some test changes in tests/func/test_diff.py)

For #1902

* Undoes "Renames self.git for self.repo throughout module" introduced in 3d2dc83.

* diff: `dvc diff` uses code for `git rev_parse --short` instead of hard `[:7]` (for short commit hashes).

- Tests updated too.

Per https://github.com/iterative/dvc/pull/1906#discussion_r278510340

* tests: Reformatted with `black tests/func/test_diff.py`

* tests: Remove uneccessary `str()` casting form `dvc diff` tests (and reformat)
This commit is contained in:
Jorge Orpinel
2019-05-10 15:30:36 +03:00
committed by Ruslan Kuprieiev
parent 7ceaf88c54
commit b09bd0426e
2 changed files with 52 additions and 29 deletions
+27 -16
View File
@@ -33,6 +33,9 @@ class Git(Base):
GIT_DIR = ".git"
def __init__(self, root_dir=os.curdir, repo=None):
"""Git class constructor.
Requires `Repo` class from `git` module (from gitpython package).
"""
super(Git, self).__init__(root_dir, repo=repo)
import git
@@ -248,22 +251,30 @@ class Git(Base):
return GitTree(self.git, rev)
def _get_diff_trees(self, a_ref, b_ref):
"""Private method for getting the trees and commit hashes of 2 git
references.
Requires `gitdb` module (from gitpython package).
Args:
a_ref(str) - git reference
b_ref(str) - second git reference. If None, uses HEAD
Returns:
tuple - tuple with elements: (trees, commits)
"""
from gitdb.exc import BadObject, BadName
trees = {DIFF_A_TREE: None, DIFF_B_TREE: None}
commits = []
if b_ref is None:
b_ref = self.git.head.commit
try:
if b_ref is not None:
a_commit = self.git.commit(a_ref)
b_commit = self.git.commit(b_ref)
commits.append(str(a_commit))
commits.append(str(b_commit))
else:
a_commit = self.git.commit(a_ref)
b_commit = self.git.head.commit
commits.append(str(a_commit))
commits.append(str(b_commit))
a_commit = self.git.git.rev_parse(a_ref, short=True)
b_commit = self.git.git.rev_parse(b_ref, short=True)
# See https://gitpython.readthedocs.io
# /en/2.1.11/reference.html#git.objects.base.Object.__str__
commits.append(a_commit)
commits.append(b_commit)
trees[DIFF_A_TREE] = self.get_tree(commits[0])
trees[DIFF_B_TREE] = self.get_tree(commits[1])
except (BadName, BadObject) as e:
@@ -271,8 +282,8 @@ class Git(Base):
return trees, commits
def get_diff_trees(self, a_ref, b_ref=None):
"""Method for getting two repo trees between two git tag commits
returns the dvc hash names of changed file/directory
"""Method for getting two repo trees between two git tag commits.
Returns the dvc hash names of changed file/directory
Args:
a_ref(str) - git reference
@@ -282,10 +293,10 @@ class Git(Base):
dict - dictionary with keys: (a_tree, b_tree, a_ref, b_ref, equal)
"""
diff_dct = {DIFF_EQUAL: False}
trees, commit_refs = self._get_diff_trees(a_ref, b_ref)
diff_dct[DIFF_A_REF] = commit_refs[0]
diff_dct[DIFF_B_REF] = commit_refs[1]
if commit_refs[0] == commit_refs[1]:
trees, commits = self._get_diff_trees(a_ref, b_ref)
diff_dct[DIFF_A_REF] = commits[0]
diff_dct[DIFF_B_REF] = commits[1]
if commits[0] == commits[1]:
diff_dct[DIFF_EQUAL] = True
return diff_dct
diff_dct[DIFF_A_TREE] = trees[DIFF_A_TREE]
+25 -13
View File
@@ -28,13 +28,15 @@ class TestDiff(TestDvc):
self.new_file = "new_test_file"
self.create(self.new_file, self.new_file)
self.dvc.add(self.new_file)
self.a_ref = self.git.head.commit
self.a_ref = self.git.git.rev_parse(self.git.head.commit, short=True)
self.new_checksum = _get_checksum(self.dvc, self.new_file)
self.git.index.add([self.new_file + ".dvc"])
self.git.index.commit("adds new_file")
self.test_dct = {
diff.DIFF_A_REF: str(self.a_ref),
diff.DIFF_B_REF: str(self.git.head.commit),
diff.DIFF_A_REF: self.a_ref,
diff.DIFF_B_REF: self.git.git.rev_parse(
self.git.head.commit, short=True
),
diff.DIFF_LIST: [
{
diff.DIFF_TARGET: self.new_file,
@@ -48,8 +50,11 @@ class TestDiff(TestDvc):
def test(self):
out = self.dvc.scm.get_diff_trees(self.a_ref)
self.assertFalse(out[diff.DIFF_EQUAL])
self.assertEqual(str(self.a_ref), out[diff.DIFF_A_REF])
self.assertEqual(str(self.git.head.commit), out[diff.DIFF_B_REF])
self.assertEqual(self.a_ref, out[diff.DIFF_A_REF])
self.assertEqual(
self.git.git.rev_parse(self.git.head.commit, short=True),
out[diff.DIFF_B_REF],
)
class TestDiffRepo(TestDiff):
@@ -63,7 +68,7 @@ class TestDiffCmdLine(TestDiff):
with patch("dvc.cli.diff.CmdDiff._show", autospec=True):
with patch("dvc.repo.Repo", config="testing") as MockRepo:
MockRepo.return_value.diff.return_value = "testing"
ret = main(["diff", "-t", self.new_file, str(self.a_ref)])
ret = main(["diff", "-t", self.new_file, self.a_ref])
self.assertEqual(ret, 0)
@@ -97,7 +102,9 @@ class TestDiffDir(TestDvc):
self.dvc.add(self.DATA_DIR)
self.git.index.add([self.DATA_DIR + ".dvc"])
self.git.index.commit("adds data_dir")
self.a_ref = str(self.dvc.scm.git.head.commit)
self.a_ref = self.git.git.rev_parse(
self.dvc.scm.git.head.commit, short=True
)
self.old_checksum = _get_checksum(self.dvc, self.DATA_DIR)
self.new_file = os.path.join(self.DATA_SUB_DIR, diff.DIFF_NEW_FILE)
self.create(self.new_file, self.new_file)
@@ -109,8 +116,11 @@ class TestDiffDir(TestDvc):
def test(self):
out = self.dvc.scm.get_diff_trees(self.a_ref)
self.assertFalse(out[diff.DIFF_EQUAL])
self.assertEqual(str(self.a_ref), out[diff.DIFF_A_REF])
self.assertEqual(str(self.git.head.commit), out[diff.DIFF_B_REF])
self.assertEqual(self.a_ref, out[diff.DIFF_A_REF])
self.assertEqual(
self.git.git.rev_parse(self.git.head.commit, short=True),
out[diff.DIFF_B_REF],
)
class TestDiffDirRepo(TestDiffDir):
@@ -119,8 +129,10 @@ class TestDiffDirRepo(TestDiffDir):
def test(self):
result = self.dvc.diff(self.a_ref, target=self.DATA_DIR)
test_dct = {
diff.DIFF_A_REF: str(self.a_ref),
diff.DIFF_B_REF: str(self.git.head.commit),
diff.DIFF_A_REF: self.git.git.rev_parse(self.a_ref, short=True),
diff.DIFF_B_REF: self.git.git.rev_parse(
self.git.head.commit, short=True
),
diff.DIFF_LIST: [
{
diff.DIFF_CHANGE: 0,
@@ -157,8 +169,8 @@ class TestDiffDirRepoDeletedFile(TestDiffDir):
self.a_ref, b_ref=self.b_ref, target=self.DATA_DIR
)
test_dct = {
diff.DIFF_A_REF: str(self.a_ref),
diff.DIFF_B_REF: str(self.b_ref),
diff.DIFF_A_REF: self.git.git.rev_parse(self.a_ref, short=True),
diff.DIFF_B_REF: self.git.git.rev_parse(self.b_ref, short=True),
diff.DIFF_LIST: [
{
diff.DIFF_CHANGE: 0,